Re: [PATCH] x86: Encode 256-bit/512-bit VEX/EVEX insns with 128-bit VEX

["Jan Beulich" <JBeulich at suse dot com>]

>>> On 19.03.19 at 03:53, <hjl.tools@gmail.com> wrote:
> On Mon, Mar 18, 2019 at 7:42 PM Jan Beulich <JBeulich@suse.com> wrote:
>>
>> >>> On 17.03.19 at 11:08, <hjl.tools@gmail.com> wrote:
>> > --- a/gas/config/tc-i386.c
>> > +++ b/gas/config/tc-i386.c
>> > @@ -3977,8 +3977,7 @@ optimize_encoding (void)
>> >           }
>> >       }
>> >      }
>> > -  else if (optimize > 1
>> > -        && i.reg_operands == 3
>> > +  else if (i.reg_operands == 3
>> >          && i.op[0].regs == i.op[1].regs
>> >          && !i.types[2].bitfield.xmmword
>> >          && (i.tm.opcode_modifier.vex
>> > @@ -4009,15 +4008,15 @@ optimize_encoding (void)
>> >               || i.tm.base_opcode == 0x6647)
>> >              && i.tm.extension_opcode == None))
>> >      {
>> > -      /* Optimize: -O2:
>> > +      /* Optimize: -O1:
>> >          VOP, one of vandnps, vandnpd, vxorps, vxorpd, vpsubb, vpsubd,
>> >          vpsubq and vpsubw:
>> >            EVEX VOP %zmmM, %zmmM, %zmmN
>> >              -> VEX VOP %xmmM, %xmmM, %xmmN (M and N < 16)
>> > -            -> EVEX VOP %xmmM, %xmmM, %xmmN (M || N >= 16)
>> > +            -> EVEX VOP %xmmM, %xmmM, %xmmN (M || N >= 16) (-O2)
>> >            EVEX VOP %ymmM, %ymmM, %ymmN
>> >              -> VEX VOP %xmmM, %xmmM, %xmmN (M and N < 16)
>> > -            -> EVEX VOP %xmmM, %xmmM, %xmmN (M || N >= 16)
>> > +            -> EVEX VOP %xmmM, %xmmM, %xmmN (M || N >= 16) (-O2)
>> >            VEX VOP %ymmM, %ymmM, %ymmN
>> >              -> VEX VOP %xmmM, %xmmM, %xmmN
>> >          VOP, one of vpandn and vpxor:
>> > @@ -4026,17 +4025,17 @@ optimize_encoding (void)
>> >          VOP, one of vpandnd and vpandnq:
>> >            EVEX VOP %zmmM, %zmmM, %zmmN
>> >              -> VEX vpandn %xmmM, %xmmM, %xmmN (M and N < 16)
>> > -            -> EVEX VOP %xmmM, %xmmM, %xmmN (M || N >= 16)
>> > +            -> EVEX VOP %xmmM, %xmmM, %xmmN (M || N >= 16) (-O2)
>> >            EVEX VOP %ymmM, %ymmM, %ymmN
>> >              -> VEX vpandn %xmmM, %xmmM, %xmmN (M and N < 16)
>> > -            -> EVEX VOP %xmmM, %xmmM, %xmmN (M || N >= 16)
>> > +            -> EVEX VOP %xmmM, %xmmM, %xmmN (M || N >= 16) (-O2)
>> >          VOP, one of vpxord and vpxorq:
>> >            EVEX VOP %zmmM, %zmmM, %zmmN
>> >              -> VEX vpxor %xmmM, %xmmM, %xmmN (M and N < 16)
>> > -            -> EVEX VOP %xmmM, %xmmM, %xmmN (M || N >= 16)
>> > +            -> EVEX VOP %xmmM, %xmmM, %xmmN (M || N >= 16) (-O2)
>> >            EVEX VOP %ymmM, %ymmM, %ymmN
>> >              -> VEX vpxor %xmmM, %xmmM, %xmmN (M and N < 16)
>> > -            -> EVEX VOP %xmmM, %xmmM, %xmmN (M || N >= 16)
>> > +            -> EVEX VOP %xmmM, %xmmM, %xmmN (M || N >= 16) (-O2)
>> >          VOP, one of kxord and kxorq:
>> >            VEX VOP %kM, %kM, %kN
>> >              -> VEX kxorw %kM, %kM, %kN
>>
>> I disagree - as per my earlier reply to another patch there shouldn't
>> be any use of cpu_arch_flags here, and -O2 should not all of the
>> sudden imply AVX512VL to be available when it wasn't explicitly
>> enabled. Effectively this will make it impossible to add any other,
>> ISA-independent optimization to -O2 later on.
>>
>>
> 
> Does
> 
> https://sourceware.org/ml/binutils/2019-03/msg00111.html 
> 
> fix this problem?

I think so, yes, but I also think you've gone a little too far
with code you remove:

>--- a/gas/config/tc-i386.c
>+++ b/gas/config/tc-i386.c
>@@ -3985,9 +3985,6 @@ optimize_encoding (void)
> 		   && !i.rounding
> 		   && is_evex_encoding (&i.tm)
> 		   && (i.vec_encoding != vex_encoding_evex
>-		       || cpu_arch_flags.bitfield.cpuavx
>-		       || cpu_arch_isa_flags.bitfield.cpuavx
>-		       || cpu_arch_flags.bitfield.cpuavx512vl
> 		       || cpu_arch_isa_flags.bitfield.cpuavx512vl
> 		       || i.tm.cpu_flags.bitfield.cpuavx512vl
> 		       || (i.tm.operand_types[2].bitfield.zmmword

I agree here.

>@@ -4045,17 +4042,13 @@ optimize_encoding (void)
>        */
>       if (is_evex_encoding (&i.tm))
> 	{
>-	  if (i.vec_encoding != vex_encoding_evex
>-	      && (cpu_arch_flags.bitfield.cpuavx
>-		  || cpu_arch_isa_flags.bitfield.cpuavx))
>+	  if (i.vec_encoding != vex_encoding_evex)
> 	    {
> 	      i.tm.opcode_modifier.vex = VEX128;
> 	      i.tm.opcode_modifier.vexw = VEXW0;
> 	      i.tm.opcode_modifier.evex = 0;
> 	    }
>-	  else if (optimize > 1
>-		   && (cpu_arch_flags.bitfield.cpuavx512vl
>-		       || cpu_arch_isa_flags.bitfield.cpuavx512vl))
>+	  else if (optimize > 1)
> 	    i.tm.opcode_modifier.evex = EVEX128;

But don't you need to retain the cpu_arch_isa_flags check here?

Btw., I see you're removing two of the cpuavx checks here that
I've mentioned in the mail sent a few minutes ago - thanks.
There's a 3rd one though:

  else if ((cpu_arch_flags.bitfield.cpuavx
	    || cpu_arch_isa_flags.bitfield.cpuavx)
	   && i.vec_encoding != vex_encoding_evex
	   && !i.types[0].bitfield.zmmword
	   && !i.mask
	   && is_evex_encoding (&i.tm)
	   && (i.tm.base_opcode == 0x666f
	       || (i.tm.base_opcode ^ Opcode_SIMD_IntD) == 0x666f

Jan