x86: Mark cvtpi2ps and cvtpi2pd as MMX

Message ID 20200219125819.89247-1-hjl.tools@gmail.com
State New
Headers show
Series
  • x86: Mark cvtpi2ps and cvtpi2pd as MMX
Related show

Commit Message

H.J. Lu Feb. 19, 2020, 12:58 p.m.
* config/tc-i386.c (output_insn): Mark cvtpi2ps and cvtpi2pd
	with GNU_PROPERTY_X86_FEATURE_2_MMX.
	* testsuite/gas/i386/i386.exp: Run property-3 and
	x86-64-property-3.
	* testsuite/gas/i386/property-3.d: New file.
	* testsuite/gas/i386/property-3.s: Likewise.
	* testsuite/gas/i386/x86-64-property-3.d: Likewise.
---
 gas/ChangeLog                              | 10 ++++++++++
 gas/config/tc-i386.c                       |  4 +++-
 gas/testsuite/gas/i386/i386.exp            |  2 ++
 gas/testsuite/gas/i386/property-3.d        |  9 +++++++++
 gas/testsuite/gas/i386/property-3.s        |  2 ++
 gas/testsuite/gas/i386/x86-64-property-3.d | 10 ++++++++++
 6 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 gas/testsuite/gas/i386/property-3.d
 create mode 100644 gas/testsuite/gas/i386/property-3.s
 create mode 100644 gas/testsuite/gas/i386/x86-64-property-3.d

-- 
2.24.1

Comments

Jan Beulich Feb. 19, 2020, 1:04 p.m. | #1
On 19.02.2020 13:58, H.J. Lu wrote:
> --- a/gas/config/tc-i386.c

> +++ b/gas/config/tc-i386.c

> @@ -8636,7 +8636,9 @@ output_insn (void)

>  	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_X87;

>        if (i.has_regmmx

>  	  || i.tm.base_opcode == 0xf77 /* emms */

> -	  || i.tm.base_opcode == 0xf0e /* femms */)

> +	  || i.tm.base_opcode == 0xf0e /* femms */

> +	  || i.tm.base_opcode == 0xf2a /* cvtpi2ps */

> +	  || i.tm.base_opcode == 0x660f2a /* cvtpi2pd */)


While for the former I agree, the latter - as pointed out
elsewhere - does explicitly _not_ switch into MMX mode when
the source operand is in memory.

> --- a/gas/testsuite/gas/i386/i386.exp

> +++ b/gas/testsuite/gas/i386/i386.exp

> @@ -601,6 +601,7 @@ if [expr ([istarget "i*86-*-*"] ||  [istarget "x86_64-*-*"]) && [gas_32_check]]

>  	run_dump_test "evex-no-scale-32"

>  	run_dump_test "property-1"

>  	run_dump_test "property-2"

> +	run_dump_test "property-3"

>  

>  	if {[istarget "*-*-linux*"]} then {

>  	    run_dump_test "align-branch-3"

> @@ -1166,6 +1167,7 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_64_check]] t

>  	run_dump_test "evex-no-scale-64"

>  	run_dump_test "x86-64-property-1"

>  	run_dump_test "x86-64-property-2"

> +	run_dump_test "x86-64-property-3"


While I appreciate you adding a test for this case, my
questions from several weeks back on the _intended_ behavior
of this tracking were because I found all of the to be
pretty broken. The brokenness would have become obvious (I
think) if proper testing of at least a fair part of cases
was actually done. Therefore I'm not convinced adding tests
of individual sub-sub-cases is actually going to be helpful.

Jan
H.J. Lu Feb. 19, 2020, 1:14 p.m. | #2
On Wed, Feb 19, 2020 at 5:04 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 19.02.2020 13:58, H.J. Lu wrote:

> > --- a/gas/config/tc-i386.c

> > +++ b/gas/config/tc-i386.c

> > @@ -8636,7 +8636,9 @@ output_insn (void)

> >       x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_X87;

> >        if (i.has_regmmx

> >         || i.tm.base_opcode == 0xf77 /* emms */

> > -       || i.tm.base_opcode == 0xf0e /* femms */)

> > +       || i.tm.base_opcode == 0xf0e /* femms */

> > +       || i.tm.base_opcode == 0xf2a /* cvtpi2ps */

> > +       || i.tm.base_opcode == 0x660f2a /* cvtpi2pd */)

>

> While for the former I agree, the latter - as pointed out

> elsewhere - does explicitly _not_ switch into MMX mode when

> the source operand is in memory.


They are still MMX instructions even with memory operand.

> > --- a/gas/testsuite/gas/i386/i386.exp

> > +++ b/gas/testsuite/gas/i386/i386.exp

> > @@ -601,6 +601,7 @@ if [expr ([istarget "i*86-*-*"] ||  [istarget "x86_64-*-*"]) && [gas_32_check]]

> >       run_dump_test "evex-no-scale-32"

> >       run_dump_test "property-1"

> >       run_dump_test "property-2"

> > +     run_dump_test "property-3"

> >

> >       if {[istarget "*-*-linux*"]} then {

> >           run_dump_test "align-branch-3"

> > @@ -1166,6 +1167,7 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_64_check]] t

> >       run_dump_test "evex-no-scale-64"

> >       run_dump_test "x86-64-property-1"

> >       run_dump_test "x86-64-property-2"

> > +     run_dump_test "x86-64-property-3"

>

> While I appreciate you adding a test for this case, my

> questions from several weeks back on the _intended_ behavior

> of this tracking were because I found all of the to be

> pretty broken. The brokenness would have become obvious (I

> think) if proper testing of at least a fair part of cases

> was actually done. Therefore I'm not convinced adding tests

> of individual sub-sub-cases is actually going to be helpful.

>


More tests are welcome.

-- 
H.J.
Jan Beulich Feb. 19, 2020, 1:40 p.m. | #3
On 19.02.2020 14:14, H.J. Lu wrote:
> On Wed, Feb 19, 2020 at 5:04 AM Jan Beulich <jbeulich@suse.com> wrote:

>> On 19.02.2020 13:58, H.J. Lu wrote:

>>> --- a/gas/config/tc-i386.c

>>> +++ b/gas/config/tc-i386.c

>>> @@ -8636,7 +8636,9 @@ output_insn (void)

>>>       x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_X87;

>>>        if (i.has_regmmx

>>>         || i.tm.base_opcode == 0xf77 /* emms */

>>> -       || i.tm.base_opcode == 0xf0e /* femms */)

>>> +       || i.tm.base_opcode == 0xf0e /* femms */

>>> +       || i.tm.base_opcode == 0xf2a /* cvtpi2ps */

>>> +       || i.tm.base_opcode == 0x660f2a /* cvtpi2pd */)

>>

>> While for the former I agree, the latter - as pointed out

>> elsewhere - does explicitly _not_ switch into MMX mode when

>> the source operand is in memory.

> 

> They are still MMX instructions even with memory operand.


Not exactly, see CVTPI2PD's description in the SDM.

>>> --- a/gas/testsuite/gas/i386/i386.exp

>>> +++ b/gas/testsuite/gas/i386/i386.exp

>>> @@ -601,6 +601,7 @@ if [expr ([istarget "i*86-*-*"] ||  [istarget "x86_64-*-*"]) && [gas_32_check]]

>>>       run_dump_test "evex-no-scale-32"

>>>       run_dump_test "property-1"

>>>       run_dump_test "property-2"

>>> +     run_dump_test "property-3"

>>>

>>>       if {[istarget "*-*-linux*"]} then {

>>>           run_dump_test "align-branch-3"

>>> @@ -1166,6 +1167,7 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_64_check]] t

>>>       run_dump_test "evex-no-scale-64"

>>>       run_dump_test "x86-64-property-1"

>>>       run_dump_test "x86-64-property-2"

>>> +     run_dump_test "x86-64-property-3"

>>

>> While I appreciate you adding a test for this case, my

>> questions from several weeks back on the _intended_ behavior

>> of this tracking were because I found all of the to be

>> pretty broken. The brokenness would have become obvious (I

>> think) if proper testing of at least a fair part of cases

>> was actually done. Therefore I'm not convinced adding tests

>> of individual sub-sub-cases is actually going to be helpful.

> 

> More tests are welcome.


Of course, but before adding tests I'd like to get things into
sane state, which is going to take time. I'm _not_ going to add
tests documenting the current, broken state. That's why I did
ask what the _intended_ behavior actually is (and to be honest
I'm still left guessing or taking my own decisions, as your
replies were only partly helpful; hence my shifting it down my
todo list).

Jan
H.J. Lu Feb. 19, 2020, 1:46 p.m. | #4
On Wed, Feb 19, 2020 at 5:40 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 19.02.2020 14:14, H.J. Lu wrote:

> > On Wed, Feb 19, 2020 at 5:04 AM Jan Beulich <jbeulich@suse.com> wrote:

> >> On 19.02.2020 13:58, H.J. Lu wrote:

> >>> --- a/gas/config/tc-i386.c

> >>> +++ b/gas/config/tc-i386.c

> >>> @@ -8636,7 +8636,9 @@ output_insn (void)

> >>>       x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_X87;

> >>>        if (i.has_regmmx

> >>>         || i.tm.base_opcode == 0xf77 /* emms */

> >>> -       || i.tm.base_opcode == 0xf0e /* femms */)

> >>> +       || i.tm.base_opcode == 0xf0e /* femms */

> >>> +       || i.tm.base_opcode == 0xf2a /* cvtpi2ps */

> >>> +       || i.tm.base_opcode == 0x660f2a /* cvtpi2pd */)

> >>

> >> While for the former I agree, the latter - as pointed out

> >> elsewhere - does explicitly _not_ switch into MMX mode when

> >> the source operand is in memory.

> >

> > They are still MMX instructions even with memory operand.

>

> Not exactly, see CVTPI2PD's description in the SDM.


They are MMX in term of pure SSE.

> >>> --- a/gas/testsuite/gas/i386/i386.exp

> >>> +++ b/gas/testsuite/gas/i386/i386.exp

> >>> @@ -601,6 +601,7 @@ if [expr ([istarget "i*86-*-*"] ||  [istarget "x86_64-*-*"]) && [gas_32_check]]

> >>>       run_dump_test "evex-no-scale-32"

> >>>       run_dump_test "property-1"

> >>>       run_dump_test "property-2"

> >>> +     run_dump_test "property-3"

> >>>

> >>>       if {[istarget "*-*-linux*"]} then {

> >>>           run_dump_test "align-branch-3"

> >>> @@ -1166,6 +1167,7 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_64_check]] t

> >>>       run_dump_test "evex-no-scale-64"

> >>>       run_dump_test "x86-64-property-1"

> >>>       run_dump_test "x86-64-property-2"

> >>> +     run_dump_test "x86-64-property-3"

> >>

> >> While I appreciate you adding a test for this case, my

> >> questions from several weeks back on the _intended_ behavior

> >> of this tracking were because I found all of the to be

> >> pretty broken. The brokenness would have become obvious (I

> >> think) if proper testing of at least a fair part of cases

> >> was actually done. Therefore I'm not convinced adding tests

> >> of individual sub-sub-cases is actually going to be helpful.

> >

> > More tests are welcome.

>

> Of course, but before adding tests I'd like to get things into

> sane state, which is going to take time. I'm _not_ going to add

> tests documenting the current, broken state. That's why I did

> ask what the _intended_ behavior actually is (and to be honest

> I'm still left guessing or taking my own decisions, as your

> replies were only partly helpful; hence my shifting it down my

> todo list).

>

> Jan




-- 
H.J.
Jan Beulich Feb. 19, 2020, 1:53 p.m. | #5
On 19.02.2020 14:46, H.J. Lu wrote:
> On Wed, Feb 19, 2020 at 5:40 AM Jan Beulich <jbeulich@suse.com> wrote:

>>

>> On 19.02.2020 14:14, H.J. Lu wrote:

>>> On Wed, Feb 19, 2020 at 5:04 AM Jan Beulich <jbeulich@suse.com> wrote:

>>>> On 19.02.2020 13:58, H.J. Lu wrote:

>>>>> --- a/gas/config/tc-i386.c

>>>>> +++ b/gas/config/tc-i386.c

>>>>> @@ -8636,7 +8636,9 @@ output_insn (void)

>>>>>       x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_X87;

>>>>>        if (i.has_regmmx

>>>>>         || i.tm.base_opcode == 0xf77 /* emms */

>>>>> -       || i.tm.base_opcode == 0xf0e /* femms */)

>>>>> +       || i.tm.base_opcode == 0xf0e /* femms */

>>>>> +       || i.tm.base_opcode == 0xf2a /* cvtpi2ps */

>>>>> +       || i.tm.base_opcode == 0x660f2a /* cvtpi2pd */)

>>>>

>>>> While for the former I agree, the latter - as pointed out

>>>> elsewhere - does explicitly _not_ switch into MMX mode when

>>>> the source operand is in memory.

>>>

>>> They are still MMX instructions even with memory operand.

>>

>> Not exactly, see CVTPI2PD's description in the SDM.

> 

> They are MMX in term of pure SSE.


As per your suggested doc patch "pure SSE" means "not touching MMX
registers or state". This is the case for CVTPI2PD.

Jan
H.J. Lu Feb. 19, 2020, 2:09 p.m. | #6
On Wed, Feb 19, 2020 at 5:53 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 19.02.2020 14:46, H.J. Lu wrote:

> > On Wed, Feb 19, 2020 at 5:40 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>

> >> On 19.02.2020 14:14, H.J. Lu wrote:

> >>> On Wed, Feb 19, 2020 at 5:04 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>>> On 19.02.2020 13:58, H.J. Lu wrote:

> >>>>> --- a/gas/config/tc-i386.c

> >>>>> +++ b/gas/config/tc-i386.c

> >>>>> @@ -8636,7 +8636,9 @@ output_insn (void)

> >>>>>       x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_X87;

> >>>>>        if (i.has_regmmx

> >>>>>         || i.tm.base_opcode == 0xf77 /* emms */

> >>>>> -       || i.tm.base_opcode == 0xf0e /* femms */)

> >>>>> +       || i.tm.base_opcode == 0xf0e /* femms */

> >>>>> +       || i.tm.base_opcode == 0xf2a /* cvtpi2ps */

> >>>>> +       || i.tm.base_opcode == 0x660f2a /* cvtpi2pd */)

> >>>>

> >>>> While for the former I agree, the latter - as pointed out

> >>>> elsewhere - does explicitly _not_ switch into MMX mode when

> >>>> the source operand is in memory.

> >>>

> >>> They are still MMX instructions even with memory operand.

> >>

> >> Not exactly, see CVTPI2PD's description in the SDM.

> >

> > They are MMX in term of pure SSE.

>

> As per your suggested doc patch "pure SSE" means "not touching MMX

> registers or state". This is the case for CVTPI2PD.


I will exclude cvtpi2ps and cvtpi2pd explicitly.

-- 
H.J.
Jan Beulich Feb. 19, 2020, 2:19 p.m. | #7
On 19.02.2020 15:09, H.J. Lu wrote:
> On Wed, Feb 19, 2020 at 5:53 AM Jan Beulich <jbeulich@suse.com> wrote:

>>

>> On 19.02.2020 14:46, H.J. Lu wrote:

>>> On Wed, Feb 19, 2020 at 5:40 AM Jan Beulich <jbeulich@suse.com> wrote:

>>>>

>>>> On 19.02.2020 14:14, H.J. Lu wrote:

>>>>> On Wed, Feb 19, 2020 at 5:04 AM Jan Beulich <jbeulich@suse.com> wrote:

>>>>>> On 19.02.2020 13:58, H.J. Lu wrote:

>>>>>>> --- a/gas/config/tc-i386.c

>>>>>>> +++ b/gas/config/tc-i386.c

>>>>>>> @@ -8636,7 +8636,9 @@ output_insn (void)

>>>>>>>       x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_X87;

>>>>>>>        if (i.has_regmmx

>>>>>>>         || i.tm.base_opcode == 0xf77 /* emms */

>>>>>>> -       || i.tm.base_opcode == 0xf0e /* femms */)

>>>>>>> +       || i.tm.base_opcode == 0xf0e /* femms */

>>>>>>> +       || i.tm.base_opcode == 0xf2a /* cvtpi2ps */

>>>>>>> +       || i.tm.base_opcode == 0x660f2a /* cvtpi2pd */)

>>>>>>

>>>>>> While for the former I agree, the latter - as pointed out

>>>>>> elsewhere - does explicitly _not_ switch into MMX mode when

>>>>>> the source operand is in memory.

>>>>>

>>>>> They are still MMX instructions even with memory operand.

>>>>

>>>> Not exactly, see CVTPI2PD's description in the SDM.

>>>

>>> They are MMX in term of pure SSE.

>>

>> As per your suggested doc patch "pure SSE" means "not touching MMX

>> registers or state". This is the case for CVTPI2PD.

> 

> I will exclude cvtpi2ps and cvtpi2pd explicitly.


Okay, this would clarify what "pure SSE" means. My next objection
then is to make a connection between "pure SSE" and the feature
used property. There, as made pretty clear by my earlier inquiry
about the intentions of this feature tracking, things should be
really tied to hardware behavior. I.e. "not touching MMX registers
or state" ==> MMX feature not recorded as being used. No
exceptions for any insns.

Jan
H.J. Lu Feb. 19, 2020, 2:23 p.m. | #8
On Wed, Feb 19, 2020 at 6:19 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 19.02.2020 15:09, H.J. Lu wrote:

> > On Wed, Feb 19, 2020 at 5:53 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>

> >> On 19.02.2020 14:46, H.J. Lu wrote:

> >>> On Wed, Feb 19, 2020 at 5:40 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>>>

> >>>> On 19.02.2020 14:14, H.J. Lu wrote:

> >>>>> On Wed, Feb 19, 2020 at 5:04 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>>>>> On 19.02.2020 13:58, H.J. Lu wrote:

> >>>>>>> --- a/gas/config/tc-i386.c

> >>>>>>> +++ b/gas/config/tc-i386.c

> >>>>>>> @@ -8636,7 +8636,9 @@ output_insn (void)

> >>>>>>>       x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_X87;

> >>>>>>>        if (i.has_regmmx

> >>>>>>>         || i.tm.base_opcode == 0xf77 /* emms */

> >>>>>>> -       || i.tm.base_opcode == 0xf0e /* femms */)

> >>>>>>> +       || i.tm.base_opcode == 0xf0e /* femms */

> >>>>>>> +       || i.tm.base_opcode == 0xf2a /* cvtpi2ps */

> >>>>>>> +       || i.tm.base_opcode == 0x660f2a /* cvtpi2pd */)

> >>>>>>

> >>>>>> While for the former I agree, the latter - as pointed out

> >>>>>> elsewhere - does explicitly _not_ switch into MMX mode when

> >>>>>> the source operand is in memory.

> >>>>>

> >>>>> They are still MMX instructions even with memory operand.

> >>>>

> >>>> Not exactly, see CVTPI2PD's description in the SDM.

> >>>

> >>> They are MMX in term of pure SSE.

> >>

> >> As per your suggested doc patch "pure SSE" means "not touching MMX

> >> registers or state". This is the case for CVTPI2PD.

> >

> > I will exclude cvtpi2ps and cvtpi2pd explicitly.

>

> Okay, this would clarify what "pure SSE" means. My next objection

> then is to make a connection between "pure SSE" and the feature

> used property. There, as made pretty clear by my earlier inquiry

> about the intentions of this feature tracking, things should be

> really tied to hardware behavior. I.e. "not touching MMX registers

> or state" ==> MMX feature not recorded as being used. No

> exceptions for any insns.

>


Intention is to treat cvtpi2ps and cvtpi2pd as MMX.


-- 
H.J.
Jan Beulich Feb. 19, 2020, 2:36 p.m. | #9
On 19.02.2020 15:23, H.J. Lu wrote:
> On Wed, Feb 19, 2020 at 6:19 AM Jan Beulich <jbeulich@suse.com> wrote:

>> On 19.02.2020 15:09, H.J. Lu wrote:

>>> On Wed, Feb 19, 2020 at 5:53 AM Jan Beulich <jbeulich@suse.com> wrote:

>>>> As per your suggested doc patch "pure SSE" means "not touching MMX

>>>> registers or state". This is the case for CVTPI2PD.

>>>

>>> I will exclude cvtpi2ps and cvtpi2pd explicitly.

>>

>> Okay, this would clarify what "pure SSE" means. My next objection

>> then is to make a connection between "pure SSE" and the feature

>> used property. There, as made pretty clear by my earlier inquiry

>> about the intentions of this feature tracking, things should be

>> really tied to hardware behavior. I.e. "not touching MMX registers

>> or state" ==> MMX feature not recorded as being used. No

>> exceptions for any insns.

> 

> Intention is to treat cvtpi2ps and cvtpi2pd as MMX.


Well, I understand you want to special case these. But the model
behind the feature recording shouldn't require any (such) special
cases. To possible consumers of this information it doesn't matter
what exact insns are being used. Instead, affected machine state
is what counts. Hence the individual flags should be tied to the
machine state individual insns consume or touch, and at that point
no special cases like the ones you suggest are going to be needed
(special casing may still be needed when possibly the
consumed/touched machine state can't be inferred from other
information available, most notably the insn template and its
actual operands).

Jan
H.J. Lu Feb. 19, 2020, 2:46 p.m. | #10
On Wed, Feb 19, 2020 at 6:36 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 19.02.2020 15:23, H.J. Lu wrote:

> > On Wed, Feb 19, 2020 at 6:19 AM Jan Beulich <jbeulich@suse.com> wrote:

> >> On 19.02.2020 15:09, H.J. Lu wrote:

> >>> On Wed, Feb 19, 2020 at 5:53 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>>> As per your suggested doc patch "pure SSE" means "not touching MMX

> >>>> registers or state". This is the case for CVTPI2PD.

> >>>

> >>> I will exclude cvtpi2ps and cvtpi2pd explicitly.

> >>

> >> Okay, this would clarify what "pure SSE" means. My next objection

> >> then is to make a connection between "pure SSE" and the feature

> >> used property. There, as made pretty clear by my earlier inquiry

> >> about the intentions of this feature tracking, things should be

> >> really tied to hardware behavior. I.e. "not touching MMX registers

> >> or state" ==> MMX feature not recorded as being used. No

> >> exceptions for any insns.

> >

> > Intention is to treat cvtpi2ps and cvtpi2pd as MMX.

>

> Well, I understand you want to special case these. But the model

> behind the feature recording shouldn't require any (such) special

> cases. To possible consumers of this information it doesn't matter

> what exact insns are being used. Instead, affected machine state

> is what counts. Hence the individual flags should be tied to the

> machine state individual insns consume or touch, and at that point

> no special cases like the ones you suggest are going to be needed

> (special casing may still be needed when possibly the

> consumed/touched machine state can't be inferred from other

> information available, most notably the insn template and its

> actual operands).


Intention is to mark any instructions whose form include MMX register
or state as MMX.

-- 
H.J.
Jan Beulich Feb. 19, 2020, 2:58 p.m. | #11
On 19.02.2020 15:46, H.J. Lu wrote:
> On Wed, Feb 19, 2020 at 6:36 AM Jan Beulich <jbeulich@suse.com> wrote:

>>

>> On 19.02.2020 15:23, H.J. Lu wrote:

>>> On Wed, Feb 19, 2020 at 6:19 AM Jan Beulich <jbeulich@suse.com> wrote:

>>>> On 19.02.2020 15:09, H.J. Lu wrote:

>>>>> On Wed, Feb 19, 2020 at 5:53 AM Jan Beulich <jbeulich@suse.com> wrote:

>>>>>> As per your suggested doc patch "pure SSE" means "not touching MMX

>>>>>> registers or state". This is the case for CVTPI2PD.

>>>>>

>>>>> I will exclude cvtpi2ps and cvtpi2pd explicitly.

>>>>

>>>> Okay, this would clarify what "pure SSE" means. My next objection

>>>> then is to make a connection between "pure SSE" and the feature

>>>> used property. There, as made pretty clear by my earlier inquiry

>>>> about the intentions of this feature tracking, things should be

>>>> really tied to hardware behavior. I.e. "not touching MMX registers

>>>> or state" ==> MMX feature not recorded as being used. No

>>>> exceptions for any insns.

>>>

>>> Intention is to treat cvtpi2ps and cvtpi2pd as MMX.

>>

>> Well, I understand you want to special case these. But the model

>> behind the feature recording shouldn't require any (such) special

>> cases. To possible consumers of this information it doesn't matter

>> what exact insns are being used. Instead, affected machine state

>> is what counts. Hence the individual flags should be tied to the

>> machine state individual insns consume or touch, and at that point

>> no special cases like the ones you suggest are going to be needed

>> (special casing may still be needed when possibly the

>> consumed/touched machine state can't be inferred from other

>> information available, most notably the insn template and its

>> actual operands).

> 

> Intention is to mark any instructions whose form include MMX register

> or state as MMX.


Exactly. And hence CVTPI2PD should not get such marking when
having a memory operand.

Jan

Patch

diff --git a/gas/ChangeLog b/gas/ChangeLog
index 095e457822..56262742f0 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,13 @@ 
+2020-02-19  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* config/tc-i386.c (output_insn): Mark cvtpi2ps and cvtpi2pd
+	with GNU_PROPERTY_X86_FEATURE_2_MMX.
+	* testsuite/gas/i386/i386.exp: Run property-3 and
+	x86-64-property-3.
+	* testsuite/gas/i386/property-3.d: New file.
+	* testsuite/gas/i386/property-3.s: Likewise.
+	* testsuite/gas/i386/x86-64-property-3.d: Likewise.
+
 2020-02-17  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* config/tc-i386.c (cpu_arch): Add .popcnt.
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index f559ad4103..d118fdd567 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -8636,7 +8636,9 @@  output_insn (void)
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_X87;
       if (i.has_regmmx
 	  || i.tm.base_opcode == 0xf77 /* emms */
-	  || i.tm.base_opcode == 0xf0e /* femms */)
+	  || i.tm.base_opcode == 0xf0e /* femms */
+	  || i.tm.base_opcode == 0xf2a /* cvtpi2ps */
+	  || i.tm.base_opcode == 0x660f2a /* cvtpi2pd */)
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_MMX;
       if (i.has_regxmm)
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_XMM;
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index d884f89364..685e62ea72 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -601,6 +601,7 @@  if [expr ([istarget "i*86-*-*"] ||  [istarget "x86_64-*-*"]) && [gas_32_check]]
 	run_dump_test "evex-no-scale-32"
 	run_dump_test "property-1"
 	run_dump_test "property-2"
+	run_dump_test "property-3"
 
 	if {[istarget "*-*-linux*"]} then {
 	    run_dump_test "align-branch-3"
@@ -1166,6 +1167,7 @@  if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_64_check]] t
 	run_dump_test "evex-no-scale-64"
 	run_dump_test "x86-64-property-1"
 	run_dump_test "x86-64-property-2"
+	run_dump_test "x86-64-property-3"
 
 	if {[istarget "*-*-linux*"]} then {
 	    run_dump_test "x86-64-align-branch-3"
diff --git a/gas/testsuite/gas/i386/property-3.d b/gas/testsuite/gas/i386/property-3.d
new file mode 100644
index 0000000000..36d215584e
--- /dev/null
+++ b/gas/testsuite/gas/i386/property-3.d
@@ -0,0 +1,9 @@ 
+#name: i386 property 3
+#as: -mx86-used-note=yes --generate-missing-build-notes=no
+#readelf: -n
+
+Displaying notes found in: .note.gnu.property
+[ 	]+Owner[ 	]+Data size[ 	]+Description
+  GNU                  0x[0-9a-f]+	NT_GNU_PROPERTY_TYPE_0
+      Properties: x86 ISA used: SSE
+	x86 feature used: x86, MMX, XMM
diff --git a/gas/testsuite/gas/i386/property-3.s b/gas/testsuite/gas/i386/property-3.s
new file mode 100644
index 0000000000..c42bdcbcdc
--- /dev/null
+++ b/gas/testsuite/gas/i386/property-3.s
@@ -0,0 +1,2 @@ 
+	.text
+	cvtpi2ps (%eax), %xmm0
diff --git a/gas/testsuite/gas/i386/x86-64-property-3.d b/gas/testsuite/gas/i386/x86-64-property-3.d
new file mode 100644
index 0000000000..aa116e0fbc
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-property-3.d
@@ -0,0 +1,10 @@ 
+#name: x86-64 property 3
+#source: property-3.s
+#as: -mx86-used-note=yes --generate-missing-build-notes=no
+#readelf: -n
+
+Displaying notes found in: .note.gnu.property
+[ 	]+Owner[ 	]+Data size[ 	]+Description
+  GNU                  0x[0-9a-f]+	NT_GNU_PROPERTY_TYPE_0
+      Properties: x86 ISA used: SSE
+	x86 feature used: x86, MMX, XMM