[v2,3/4] x86: adjust segment override prefix emission

Message ID de0179de-c9f7-ad67-9e1f-3fd63ea6ecdc@suse.com
State New
Headers show
Series
  • x86: segment override handling adjustments
Related show

Commit Message

Jan Beulich Feb. 14, 2020, 11:44 a.m.
Since we already suppress the prefix altogether when it's the default
one for the chosen addressing mode, let's do so also when instruction
prefix and override specified with the memory operand match. (Note that
insn prefix specified segment overrides never get discarded.)

gas/
2020-02-XX  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386.c (process_operands): Also skip segment
	override prefix emission if it matches an already present one.
	* testsuite/gas/i386/prefix32.s: Add double segment override
	cases.
	* testsuite/gas/i386/prefix32.l: Adjust expectations.

Comments

H.J. Lu Feb. 14, 2020, 12:08 p.m. | #1
On Fri, Feb 14, 2020 at 3:44 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> Since we already suppress the prefix altogether when it's the default

> one for the chosen addressing mode, let's do so also when instruction

> prefix and override specified with the memory operand match. (Note that

> insn prefix specified segment overrides never get discarded.)

>

> gas/

> 2020-02-XX  Jan Beulich  <jbeulich@suse.com>

>

>         * config/tc-i386.c (process_operands): Also skip segment

>         override prefix emission if it matches an already present one.

>         * testsuite/gas/i386/prefix32.s: Add double segment override

>         cases.

>         * testsuite/gas/i386/prefix32.l: Adjust expectations.

>

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

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

> @@ -7208,11 +7208,12 @@ duplicate:

>      }

>

>    /* If a segment was explicitly specified, and the specified segment

> -     is not the default, use an opcode prefix to select it.  If we

> -     never figured out what the default segment is, then default_seg

> -     will be zero at this point, and the specified segment prefix will

> -     always be used.  */

> -  if ((i.seg[0]) && (i.seg[0] != default_seg))

> +     is neither the default nor the one already recorded from a prefix,

> +     use an opcode prefix to select it.  If we never figured out what

> +     the default segment is, then default_seg will be zero at this

> +     point, and the specified segment prefix will always be used.  */

> +  if ((i.seg[0]) && (i.seg[0] != default_seg)

> +      && (i.seg[0]->seg_prefix != i.prefix[SEG_PREFIX]))


Since you are changing the code, please write

if (i.seg[0]
   && i.seg[0] != default_seg
   && i.seg[0]->seg_prefix != i.prefix[SEG_PREFIX])

OK with this change.

Thanks.


H.J.
Jan Beulich Feb. 14, 2020, 12:17 p.m. | #2
On 14.02.2020 13:08, H.J. Lu wrote:
> On Fri, Feb 14, 2020 at 3:44 AM Jan Beulich <jbeulich@suse.com> wrote:

>>

>> Since we already suppress the prefix altogether when it's the default

>> one for the chosen addressing mode, let's do so also when instruction

>> prefix and override specified with the memory operand match. (Note that

>> insn prefix specified segment overrides never get discarded.)

>>

>> gas/

>> 2020-02-XX  Jan Beulich  <jbeulich@suse.com>

>>

>>         * config/tc-i386.c (process_operands): Also skip segment

>>         override prefix emission if it matches an already present one.

>>         * testsuite/gas/i386/prefix32.s: Add double segment override

>>         cases.

>>         * testsuite/gas/i386/prefix32.l: Adjust expectations.

>>

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

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

>> @@ -7208,11 +7208,12 @@ duplicate:

>>      }

>>

>>    /* If a segment was explicitly specified, and the specified segment

>> -     is not the default, use an opcode prefix to select it.  If we

>> -     never figured out what the default segment is, then default_seg

>> -     will be zero at this point, and the specified segment prefix will

>> -     always be used.  */

>> -  if ((i.seg[0]) && (i.seg[0] != default_seg))

>> +     is neither the default nor the one already recorded from a prefix,

>> +     use an opcode prefix to select it.  If we never figured out what

>> +     the default segment is, then default_seg will be zero at this

>> +     point, and the specified segment prefix will always be used.  */

>> +  if ((i.seg[0]) && (i.seg[0] != default_seg)

>> +      && (i.seg[0]->seg_prefix != i.prefix[SEG_PREFIX]))

> 

> Since you are changing the code, please write

> 

> if (i.seg[0]

>    && i.seg[0] != default_seg

>    && i.seg[0]->seg_prefix != i.prefix[SEG_PREFIX])


Sure, will do. I simply wasn't sure whether such secondary cleanup would
be welcome.

> OK with this change.


Thanks.

Jan

Patch

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -7208,11 +7208,12 @@  duplicate:
     }
 
   /* If a segment was explicitly specified, and the specified segment
-     is not the default, use an opcode prefix to select it.  If we
-     never figured out what the default segment is, then default_seg
-     will be zero at this point, and the specified segment prefix will
-     always be used.  */
-  if ((i.seg[0]) && (i.seg[0] != default_seg))
+     is neither the default nor the one already recorded from a prefix,
+     use an opcode prefix to select it.  If we never figured out what
+     the default segment is, then default_seg will be zero at this
+     point, and the specified segment prefix will always be used.  */
+  if ((i.seg[0]) && (i.seg[0] != default_seg)
+      && (i.seg[0]->seg_prefix != i.prefix[SEG_PREFIX]))
     {
       if (!add_prefix (i.seg[0]->seg_prefix))
 	return 0;
--- a/gas/testsuite/gas/i386/prefix32.l
+++ b/gas/testsuite/gas/i386/prefix32.l
@@ -8,6 +8,7 @@ 
 .*:19: Error: same type of prefix .*
 .*:20: Error: data size .* `vaddps'
 .*:21: Error: data size .* `vaddpd'
+.*:25: Error: same type of prefix .*
 GAS LISTING .*
 #...
 [ 	]*1[ 	]+\.text
@@ -33,4 +34,10 @@  GAS LISTING .*
 [ 	]*20[ 	]+data16 vaddps	%xmm0, %xmm0, %xmm0
 [ 	]*21[ 	]+data16 vaddpd	%xmm0, %xmm0, %xmm0
 [ 	]*22[ 	]*
-[ 	]*23[ 	]*[\?]+ 0+[ \t]+\.p2align	4,0
+[ 	]*23[ 	]+\.Lsegment:
+[ 	]*24[ 	]+\?\?\?\? 368B4500[ 	]+ss mov		%ss:\(%ebp\), %eax
+[ 	]*25[ 	]+ss mov		%ds:\(%ebp\), %eax
+[ 	]*26[ 	]+\?\?\?\? 3E8B4500[ 	]+ds mov		%ss:\(%ebp\), %eax
+[ 	]*27[ 	]+\?\?\?\? 3E8B4500[ 	]+ds mov		%ds:\(%ebp\), %eax
+[ 	]*28[ 	]*
+#pass
--- a/gas/testsuite/gas/i386/prefix32.s
+++ b/gas/testsuite/gas/i386/prefix32.s
@@ -20,4 +20,10 @@  prefix:
 	data16 vaddps	%xmm0, %xmm0, %xmm0
 	data16 vaddpd	%xmm0, %xmm0, %xmm0
 
+.Lsegment:
+	ss mov		%ss:(%ebp), %eax
+	ss mov		%ds:(%ebp), %eax
+	ds mov		%ss:(%ebp), %eax
+	ds mov		%ds:(%ebp), %eax
+
 	.p2align	4,0