[1/5] x86/SSE2AVX: respect prefixes

Message ID 663e2e0c-b098-476d-aac7-9bda63187af4@suse.com
State New
Headers
Series x86/APX: respect -msse2avx |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Testing passed

Commit Message

Jan Beulich March 22, 2024, 9:27 a.m. UTC
  1) Without -msse2avx we unconditionally honor REX.W. Hence we ought to
   also do so with -msse2avx, converting to VEX.W.

2) {rex} doesn't prevent conversion to VEX encodings. Thus {rex2}
   shouldn't either.
---
Further {evex} may better also be respected, but that'll be a bigger
change.
  

Comments

Cui, Lili March 27, 2024, 8:47 a.m. UTC | #1
> 1) Without -msse2avx we unconditionally honor REX.W. Hence we ought to
>    also do so with -msse2avx, converting to VEX.W.
> 
> 2) {rex} doesn't prevent conversion to VEX encodings. Thus {rex2}
>    shouldn't either.
> ---
> Further {evex} may better also be respected, but that'll be a bigger change.
> 
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -3885,7 +3885,7 @@ build_vex_prefix (const insn_template *t
>    /* Check the REX.W bit and VEXW.  */
>    if (i.tm.opcode_modifier.vexw == VEXWIG)
>      w = (vexwig == vexw1 || (i.rex & REX_W)) ? 1 : 0;
> -  else if (i.tm.opcode_modifier.vexw)
> +  else if (i.tm.opcode_modifier.vexw && !(i.rex & REX_W))
>      w = i.tm.opcode_modifier.vexw == VEXW1 ? 1 : 0;
>    else
>      w = (flag_code == CODE_64BIT ? i.rex & REX_W : vexwig == vexw1) ? 1 : 0;

Yes, we should also correct the operand size for AVX when rex.w appears.

Lili.
  
Jan Beulich March 27, 2024, 11:31 a.m. UTC | #2
On 27.03.2024 09:47, Cui, Lili wrote:
>> 1) Without -msse2avx we unconditionally honor REX.W. Hence we ought to
>>    also do so with -msse2avx, converting to VEX.W.
>>
>> 2) {rex} doesn't prevent conversion to VEX encodings. Thus {rex2}
>>    shouldn't either.
>> ---
>> Further {evex} may better also be respected, but that'll be a bigger change.
>>
>> --- a/gas/config/tc-i386.c
>> +++ b/gas/config/tc-i386.c
>> @@ -3885,7 +3885,7 @@ build_vex_prefix (const insn_template *t
>>    /* Check the REX.W bit and VEXW.  */
>>    if (i.tm.opcode_modifier.vexw == VEXWIG)
>>      w = (vexwig == vexw1 || (i.rex & REX_W)) ? 1 : 0;
>> -  else if (i.tm.opcode_modifier.vexw)
>> +  else if (i.tm.opcode_modifier.vexw && !(i.rex & REX_W))
>>      w = i.tm.opcode_modifier.vexw == VEXW1 ? 1 : 0;
>>    else
>>      w = (flag_code == CODE_64BIT ? i.rex & REX_W : vexwig == vexw1) ? 1 : 0;
> 
> Yes, we should also correct the operand size for AVX when rex.w appears.

That I did suggest before, with H.J. objecting (iirc).

Jan
  

Patch

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -3885,7 +3885,7 @@  build_vex_prefix (const insn_template *t
   /* Check the REX.W bit and VEXW.  */
   if (i.tm.opcode_modifier.vexw == VEXWIG)
     w = (vexwig == vexw1 || (i.rex & REX_W)) ? 1 : 0;
-  else if (i.tm.opcode_modifier.vexw)
+  else if (i.tm.opcode_modifier.vexw && !(i.rex & REX_W))
     w = i.tm.opcode_modifier.vexw == VEXW1 ? 1 : 0;
   else
     w = (flag_code == CODE_64BIT ? i.rex & REX_W : vexwig == vexw1) ? 1 : 0;
@@ -8413,7 +8413,7 @@  check_EgprOperands (const insn_template
     }
 
   /* Check if pseudo prefix {rex2} is valid.  */
-  if (i.rex2_encoding)
+  if (i.rex2_encoding && !t->opcode_modifier.sse2avx)
     {
       i.error = invalid_pseudo_prefix;
       return true;
@@ -10031,6 +10031,7 @@  process_operands (void)
       i.rex |= i.prefix[REX_PREFIX] & (REX_W | REX_R | REX_X | REX_B);
       i.prefix[REX_PREFIX] = 0;
       i.rex_encoding = 0;
+      i.rex2_encoding = 0;
     }
   /* ImmExt should be processed after SSE2AVX.  */
   else if (i.tm.opcode_modifier.immext)
--- a/gas/testsuite/gas/i386/x86-64-sse2avx.s
+++ b/gas/testsuite/gas/i386/x86-64-sse2avx.s
@@ -841,6 +841,15 @@  _start:
 	rex64 pcmpestri $0, %xmm0, %xmm0
 	rex64 pcmpestrm $0, %xmm0, %xmm0
 
+		movd	%xmm1, %eax
+	rex	movd	%xmm1, %eax
+	rex.b	movd	%xmm1, %eax
+	rex.r	movd	%xmm1, %eax
+	rex.x	movd	%xmm1, %eax
+	rex.w	movd	%xmm1, %eax
+	{rex}	movd	%xmm1, %eax
+	{rex2}	movd	%xmm1, %eax
+	{vex3}	movd	%xmm1, %eax
 
 	.intel_syntax noprefix
 # Tests for op mem64
--- a/gas/testsuite/gas/i386/x86-64-sse2avx.d
+++ b/gas/testsuite/gas/i386/x86-64-sse2avx.d
@@ -740,6 +740,15 @@  Disassembly of section .text:
 [ 	]*[a-f0-9]+:	c4 e1 fa 2a 00       	vcvtsi2ssq \(%rax\),%xmm0,%xmm0
 [ 	]*[a-f0-9]+:	c4 e3 f9 61 c0 00    	vpcmpestriq \$(0x)?0,%xmm0,%xmm0
 [ 	]*[a-f0-9]+:	c4 e3 f9 60 c0 00    	vpcmpestrmq \$(0x)?0,%xmm0,%xmm0
+[ 	]*[a-f0-9]+:	c5 f9 7e c8          	vmovd  %xmm1,%eax
+[ 	]*[a-f0-9]+:	c5 f9 7e c8          	vmovd  %xmm1,%eax
+[ 	]*[a-f0-9]+:	c4 c1 79 7e c8       	vmovd  %xmm1,%r8d
+[ 	]*[a-f0-9]+:	c5 79 7e c8          	vmovd  %xmm9,%eax
+[ 	]*[a-f0-9]+:	c4 a1 79 7e c8       	vmovd  %xmm1,%eax
+[ 	]*[a-f0-9]+:	c4 e1 f9 7e c8       	vmovq  %xmm1,%rax
+[ 	]*[a-f0-9]+:	c5 f9 7e c8          	vmovd  %xmm1,%eax
+[ 	]*[a-f0-9]+:	c5 f9 7e c8          	vmovd  %xmm1,%eax
+[ 	]*[a-f0-9]+:	c4 e1 79 7e c8       	vmovd  %xmm1,%eax
 [ 	]*[a-f0-9]+:	c5 f8 ae 11          	vldmxcsr \(%rcx\)
 [ 	]*[a-f0-9]+:	c5 f8 ae 19          	vstmxcsr \(%rcx\)
 [ 	]*[a-f0-9]+:	c5 f8 5b f4          	vcvtdq2ps %xmm4,%xmm6