[PATCH v2 0/4] x86: segment override handling adjustments

[PATCH v2 0/4] x86: segment override handling adjustments

[Jan Beulich]

1: extend LEA's segment override warning
2: optimize away pointless segment overrides
3: adjust segment override prefix emission
4: extend LEA's segment override warning to applicable MPX insns

Jan

[PATCH v2 1/4] x86: extend LEA's segment override warning

[Jan Beulich]

For one both possible forms should be warned about. And then, to guard
against future surprises, qualify the original opcode check by excluding
VEX/EVEX-like templates.

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

	* config/tc-i386.c (process_operands): Also check insn prefix
	for ineffectual segment override warning. Don't cover possible
	VEX/EVEX encoded insns there.
	* testsuite/gas/i386/lea.s, testsuite/gas/i386/lea.d,
	testsuite/gas/i386/lea.e: New.
	* testsuite/gas/i386/i386.exp: Run new test.
---
v2: Move controversial MPX part to separate patch.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -7194,9 +7194,10 @@ duplicate:
 	}
     }
 
-  if (i.tm.base_opcode == 0x8d /* lea */
-      && i.seg[0]
-      && !quiet_warnings)
+  if ((i.seg[0] || i.prefix[SEG_PREFIX])
+      && !quiet_warnings
+      && i.tm.base_opcode == 0x8d /* lea */
+      && !is_any_vex_encoding(&i.tm))
     as_warn (_("segment override on `%s' is ineffectual"), i.tm.name);
 
   /* If a segment was explicitly specified, and the specified segment
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -65,6 +65,7 @@ if [expr ([istarget "i*86-*-*"] ||  [ist
     run_dump_test "intelok"
     run_dump_test "prefix"
     run_list_test "prefix32" "-al"
+    run_dump_test "lea"
     run_dump_test "amd"
     run_dump_test "katmai"
     run_dump_test "jump"
--- /dev/null
+++ b/gas/testsuite/gas/i386/lea.d
@@ -0,0 +1,12 @@
+#objdump: -dw
+#name: i386 LEA-like warnings
+#warning_output: lea.e
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <start>:
+[ 	]*[0-9a-f]+:[ 	]+36 8d 00[ 	]+lea[ 	]+%ss:\(%eax\),%eax
+[ 	]*[0-9a-f]+:[ 	]+36 8d 00[ 	]+lea[ 	]+%ss:\(%eax\),%eax
+#pass
--- /dev/null
+++ b/gas/testsuite/gas/i386/lea.e
@@ -0,0 +1,3 @@
+.*: Assembler messages:
+.*:3: Warning: .* `lea' .*
+.*:4: Warning: .* `lea' .*
--- /dev/null
+++ b/gas/testsuite/gas/i386/lea.s
@@ -0,0 +1,4 @@
+	.text
+start:
+	lea	%ss:(%eax), %eax
+	ss lea	(%eax), %eax

[PATCH v2 2/4] x86: optimize away pointless segment overrides

[Jan Beulich]

When optimizing there's no point keeping the segment overrides when we
warn about their presence in the first place.

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

	* config/tc-i386.c (process_operands): Drop ineffectual segment
	overrides when optimizing.
	* testsuite/gas/i386/lea-optimize.d: New.
	* testsuite/gas/i386/i386.exp: Run new test.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -7195,10 +7195,17 @@ duplicate:
     }
 
   if ((i.seg[0] || i.prefix[SEG_PREFIX])
-      && !quiet_warnings
       && i.tm.base_opcode == 0x8d /* lea */
       && !is_any_vex_encoding(&i.tm))
-    as_warn (_("segment override on `%s' is ineffectual"), i.tm.name);
+    {
+      if (!quiet_warnings)
+	as_warn (_("segment override on `%s' is ineffectual"), i.tm.name);
+      if (optimize)
+	{
+	  i.seg[0] = NULL;
+	  i.prefix[SEG_PREFIX] = 0;
+	}
+    }
 
   /* If a segment was explicitly specified, and the specified segment
      is not the default, use an opcode prefix to select it.  If we
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -506,6 +506,7 @@ if [expr ([istarget "i*86-*-*"] ||  [ist
     run_list_test "optimize-6a" "-I${srcdir}/$subdir -march=+noavx -al"
     run_dump_test "optimize-6b"
     run_list_test "optimize-7" "-I${srcdir}/$subdir -march=+noavx2 -al"
+    run_dump_test "lea-optimize"
     run_dump_test "align-branch-1a"
     run_dump_test "align-branch-1b"
     run_dump_test "align-branch-1c"
--- /dev/null
+++ b/gas/testsuite/gas/i386/lea-optimize.d
@@ -0,0 +1,13 @@
+#as: -O -q
+#objdump: -dw
+#name: i386 LEA-like segment overrride dropping
+#source: lea.s
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <start>:
+[ 	]*[0-9a-f]+:[ 	]+8d 00[ 	]+lea[ 	]+\(%eax\),%eax
+[ 	]*[0-9a-f]+:[ 	]+8d 00[ 	]+lea[ 	]+\(%eax\),%eax
+#pass

[PATCH v2 4/4] x86: extend LEA's segment override warning to applicable MPX insns

[Jan Beulich]

Besides LEA there are a couple of MPX insns behaving LEA-like, which
should be warned about in the same way.

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

	* config/tc-i386.c (process_operands): Also check insn prefix
	for ineffectual segment override warning. Also cover BNDC* and
	BNDMK there. Don't cover possible VEX/EVEX encoded insns there.
	* testsuite/gas/i386/lea.s, testsuite/gas/i386/lea.d,
	testsuite/gas/i386/lea.e: New.
	* testsuite/gas/i386/i386.exp: Run new test.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -7195,7 +7195,8 @@ duplicate:
     }
 
   if ((i.seg[0] || i.prefix[SEG_PREFIX])
-      && i.tm.base_opcode == 0x8d /* lea */
+      && (i.tm.base_opcode == 0x8d /* lea */
+          || ((i.tm.base_opcode | 0x010001) == 0xf30f1b) /* bnd{c[lnu],mk} */)
       && !is_any_vex_encoding(&i.tm))
     {
       if (!quiet_warnings)
--- a/gas/testsuite/gas/i386/lea-optimize.d
+++ b/gas/testsuite/gas/i386/lea-optimize.d
@@ -10,4 +10,12 @@ Disassembly of section .text:
 0+ <start>:
 [ 	]*[0-9a-f]+:[ 	]+8d 00[ 	]+lea[ 	]+\(%eax\),%eax
 [ 	]*[0-9a-f]+:[ 	]+8d 00[ 	]+lea[ 	]+\(%eax\),%eax
+[ 	]*[0-9a-f]+:[ 	]+f3 0f 1a 00[ 	]+bndcl[ 	]+\(%eax\),%bnd0
+[ 	]*[0-9a-f]+:[ 	]+f3 0f 1a 00[ 	]+bndcl[ 	]+\(%eax\),%bnd0
+[ 	]*[0-9a-f]+:[ 	]+f2 0f 1b 00[ 	]+bndcn[ 	]+\(%eax\),%bnd0
+[ 	]*[0-9a-f]+:[ 	]+f2 0f 1b 00[ 	]+bndcn[ 	]+\(%eax\),%bnd0
+[ 	]*[0-9a-f]+:[ 	]+f2 0f 1a 00[ 	]+bndcu[ 	]+\(%eax\),%bnd0
+[ 	]*[0-9a-f]+:[ 	]+f2 0f 1a 00[ 	]+bndcu[ 	]+\(%eax\),%bnd0
+[ 	]*[0-9a-f]+:[ 	]+f3 0f 1b 00[ 	]+bndmk[ 	]+\(%eax\),%bnd0
+[ 	]*[0-9a-f]+:[ 	]+f3 0f 1b 00[ 	]+bndmk[ 	]+\(%eax\),%bnd0
 #pass
--- a/gas/testsuite/gas/i386/lea.d
+++ b/gas/testsuite/gas/i386/lea.d
@@ -9,4 +9,12 @@ Disassembly of section .text:
 0+ <start>:
 [ 	]*[0-9a-f]+:[ 	]+36 8d 00[ 	]+lea[ 	]+%ss:\(%eax\),%eax
 [ 	]*[0-9a-f]+:[ 	]+36 8d 00[ 	]+lea[ 	]+%ss:\(%eax\),%eax
+[ 	]*[0-9a-f]+:[ 	]+36 f3 0f 1a 00[ 	]+bndcl[ 	]+%ss:\(%eax\),%bnd0
+[ 	]*[0-9a-f]+:[ 	]+36 f3 0f 1a 00[ 	]+bndcl[ 	]+%ss:\(%eax\),%bnd0
+[ 	]*[0-9a-f]+:[ 	]+36 f2 0f 1b 00[ 	]+bndcn[ 	]+%ss:\(%eax\),%bnd0
+[ 	]*[0-9a-f]+:[ 	]+36 f2 0f 1b 00[ 	]+bndcn[ 	]+%ss:\(%eax\),%bnd0
+[ 	]*[0-9a-f]+:[ 	]+36 f2 0f 1a 00[ 	]+bndcu[ 	]+%ss:\(%eax\),%bnd0
+[ 	]*[0-9a-f]+:[ 	]+36 f2 0f 1a 00[ 	]+bndcu[ 	]+%ss:\(%eax\),%bnd0
+[ 	]*[0-9a-f]+:[ 	]+36 f3 0f 1b 00[ 	]+bndmk[ 	]+%ss:\(%eax\),%bnd0
+[ 	]*[0-9a-f]+:[ 	]+36 f3 0f 1b 00[ 	]+bndmk[ 	]+%ss:\(%eax\),%bnd0
 #pass
--- a/gas/testsuite/gas/i386/lea.e
+++ b/gas/testsuite/gas/i386/lea.e
@@ -1,3 +1,11 @@
 .*: Assembler messages:
 .*:3: Warning: .* `lea' .*
 .*:4: Warning: .* `lea' .*
+.*:6: Warning: .* `bndcl' .*
+.*:7: Warning: .* `bndcl' .*
+.*:9: Warning: .* `bndcn' .*
+.*:10: Warning: .* `bndcn' .*
+.*:12: Warning: .* `bndcu' .*
+.*:13: Warning: .* `bndcu' .*
+.*:15: Warning: .* `bndmk' .*
+.*:16: Warning: .* `bndmk' .*
--- a/gas/testsuite/gas/i386/lea.s
+++ b/gas/testsuite/gas/i386/lea.s
@@ -2,3 +2,15 @@
 start:
 	lea	%ss:(%eax), %eax
 	ss lea	(%eax), %eax
+
+	bndcl	%ss:(%eax), %bnd0
+	ss bndcl (%eax), %bnd0
+
+	bndcn	%ss:(%eax), %bnd0
+	ss bndcn (%eax), %bnd0
+
+	bndcu	%ss:(%eax), %bnd0
+	ss bndcu (%eax), %bnd0
+
+	bndmk	%ss:(%eax), %bnd0
+	ss bndmk (%eax), %bnd0

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

[Jan Beulich]

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]))
     {
       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

Re: [PATCH v2 1/4] x86: extend LEA's segment override warning

[H.J. Lu]

On Fri, Feb 14, 2020 at 3:43 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> For one both possible forms should be warned about. And then, to guard
> against future surprises, qualify the original opcode check by excluding
> VEX/EVEX-like templates.
>
> gas/
> 2020-02-XX  Jan Beulich  <jbeulich@suse.com>
>
>         * config/tc-i386.c (process_operands): Also check insn prefix
>         for ineffectual segment override warning. Don't cover possible
>         VEX/EVEX encoded insns there.
>         * testsuite/gas/i386/lea.s, testsuite/gas/i386/lea.d,
>         testsuite/gas/i386/lea.e: New.
>         * testsuite/gas/i386/i386.exp: Run new test.
> ---
> v2: Move controversial MPX part to separate patch.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -7194,9 +7194,10 @@ duplicate:
>         }
>      }
>
> -  if (i.tm.base_opcode == 0x8d /* lea */
> -      && i.seg[0]
> -      && !quiet_warnings)
> +  if ((i.seg[0] || i.prefix[SEG_PREFIX])

Does your testcase verify that we need to check both
i.seg[0] and i.prefix[SEG_PREFIX]?  If yes, patch is OK.

Thanks.

> +      && !quiet_warnings
> +      && i.tm.base_opcode == 0x8d /* lea */
> +      && !is_any_vex_encoding(&i.tm))
>      as_warn (_("segment override on `%s' is ineffectual"), i.tm.name);
>

-- 
H.J.

Re: [PATCH v2 4/4] x86: extend LEA's segment override warning to applicable MPX insns

[H.J. Lu]

On Fri, Feb 14, 2020 at 3:44 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> Besides LEA there are a couple of MPX insns behaving LEA-like, which
> should be warned about in the same way.
>
> gas/
> 2020-02-XX  Jan Beulich  <jbeulich@suse.com>
>
>         * config/tc-i386.c (process_operands): Also check insn prefix
>         for ineffectual segment override warning. Also cover BNDC* and
>         BNDMK there. Don't cover possible VEX/EVEX encoded insns there.
>         * testsuite/gas/i386/lea.s, testsuite/gas/i386/lea.d,
>         testsuite/gas/i386/lea.e: New.
>         * testsuite/gas/i386/i386.exp: Run new test.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -7195,7 +7195,8 @@ duplicate:
>      }
>
>    if ((i.seg[0] || i.prefix[SEG_PREFIX])
> -      && i.tm.base_opcode == 0x8d /* lea */
> +      && (i.tm.base_opcode == 0x8d /* lea */
> +          || ((i.tm.base_opcode | 0x010001) == 0xf30f1b) /* bnd{c[lnu],mk} */)
>        && !is_any_vex_encoding(&i.tm))

There is no need for it.  Since MPX has been deprecated, I doubt anyone
will update MPX assembly codes even if they trigger this new warning.

-- 
H.J.

Re: [PATCH v2 2/4] x86: optimize away pointless segment overrides

[H.J. Lu]

On Fri, Feb 14, 2020 at 3:43 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> When optimizing there's no point keeping the segment overrides when we
> warn about their presence in the first place.
>
> gas/
> 2020-02-XX  Jan Beulich  <jbeulich@suse.com>
>
>         * config/tc-i386.c (process_operands): Drop ineffectual segment
>         overrides when optimizing.
>         * testsuite/gas/i386/lea-optimize.d: New.
>         * testsuite/gas/i386/i386.exp: Run new test.
>

OK.   Thanks.

-- 
H.J.

Re: [PATCH v2 1/4] x86: extend LEA's segment override warning

[Jan Beulich]

On 14.02.2020 13:02, H.J. Lu wrote:
> On Fri, Feb 14, 2020 at 3:43 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> For one both possible forms should be warned about. And then, to guard
>> against future surprises, qualify the original opcode check by excluding
>> VEX/EVEX-like templates.
>>
>> gas/
>> 2020-02-XX  Jan Beulich  <jbeulich@suse.com>
>>
>>         * config/tc-i386.c (process_operands): Also check insn prefix
>>         for ineffectual segment override warning. Don't cover possible
>>         VEX/EVEX encoded insns there.
>>         * testsuite/gas/i386/lea.s, testsuite/gas/i386/lea.d,
>>         testsuite/gas/i386/lea.e: New.
>>         * testsuite/gas/i386/i386.exp: Run new test.
>> ---
>> v2: Move controversial MPX part to separate patch.
>>
>> --- a/gas/config/tc-i386.c
>> +++ b/gas/config/tc-i386.c
>> @@ -7194,9 +7194,10 @@ duplicate:
>>         }
>>      }
>>
>> -  if (i.tm.base_opcode == 0x8d /* lea */
>> -      && i.seg[0]
>> -      && !quiet_warnings)
>> +  if ((i.seg[0] || i.prefix[SEG_PREFIX])
> 
> Does your testcase verify that we need to check both
> i.seg[0] and i.prefix[SEG_PREFIX]?

Yes it does - that's why there are two different forms of LEA.
It was actually me not seeing the warning on one of the forms
that prompted me to see what's wrong with the condition.

>  If yes, patch is OK.

Thanks.

Jan

Re: [PATCH v2 4/4] x86: extend LEA's segment override warning to applicable MPX insns

[Jan Beulich]

On 14.02.2020 13:04, H.J. Lu wrote:
> On Fri, Feb 14, 2020 at 3:44 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> Besides LEA there are a couple of MPX insns behaving LEA-like, which
>> should be warned about in the same way.
>>
>> gas/
>> 2020-02-XX  Jan Beulich  <jbeulich@suse.com>
>>
>>         * config/tc-i386.c (process_operands): Also check insn prefix
>>         for ineffectual segment override warning. Also cover BNDC* and
>>         BNDMK there. Don't cover possible VEX/EVEX encoded insns there.
>>         * testsuite/gas/i386/lea.s, testsuite/gas/i386/lea.d,
>>         testsuite/gas/i386/lea.e: New.
>>         * testsuite/gas/i386/i386.exp: Run new test.
>>
>> --- a/gas/config/tc-i386.c
>> +++ b/gas/config/tc-i386.c
>> @@ -7195,7 +7195,8 @@ duplicate:
>>      }
>>
>>    if ((i.seg[0] || i.prefix[SEG_PREFIX])
>> -      && i.tm.base_opcode == 0x8d /* lea */
>> +      && (i.tm.base_opcode == 0x8d /* lea */
>> +          || ((i.tm.base_opcode | 0x010001) == 0xf30f1b) /* bnd{c[lnu],mk} */)
>>        && !is_any_vex_encoding(&i.tm))
> 
> There is no need for it.  Since MPX has been deprecated, I doubt anyone
> will update MPX assembly codes even if they trigger this new warning.

There's perhaps as much doubt about this as there is on there being
people actually using segment overrides with LEA and alike. If it
was code I'm the maintainer for, I'd be glad if such issues were
pointed out to me. But anyway - you've made your position clear
before, and I've added this as the last patch here just for
completeness (as I'll want to have this in my local trees down the
road anyway).

Jan

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

[H.J. Lu]

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.

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

[Jan Beulich]

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