public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/3] x86: segment override handling adjustments
@ 2020-02-13 14:04 Jan Beulich
  2020-02-13 14:05 ` [PATCH 3/3] x86: adjust segment override prefix emission Jan Beulich
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Jan Beulich @ 2020-02-13 14:04 UTC (permalink / raw)
  To: binutils; +Cc: H.J. Lu

1: extend LEA's segment override warning
2: optimize away pointless segment overrides
3: adjust segment override prefix emission

Jan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/3] x86: adjust segment override prefix emission
  2020-02-13 14:04 [PATCH 0/3] x86: segment override handling adjustments Jan Beulich
@ 2020-02-13 14:05 ` Jan Beulich
  2020-02-13 14:05 ` [PATCH 2/3] x86: optimize away pointless segment overrides Jan Beulich
  2020-02-13 14:05 ` [PATCH 1/3] x86: extend LEA's segment override warning Jan Beulich
  2 siblings, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2020-02-13 14:05 UTC (permalink / raw)
  To: binutils; +Cc: H.J. Lu

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
@@ -7211,11 +7211,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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/3] x86: optimize away pointless segment overrides
  2020-02-13 14:04 [PATCH 0/3] x86: segment override handling adjustments Jan Beulich
  2020-02-13 14:05 ` [PATCH 3/3] x86: adjust segment override prefix emission Jan Beulich
@ 2020-02-13 14:05 ` Jan Beulich
  2020-02-13 14:05 ` [PATCH 1/3] x86: extend LEA's segment override warning Jan Beulich
  2 siblings, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2020-02-13 14:05 UTC (permalink / raw)
  To: binutils; +Cc: H.J. Lu

When optimizing there's no point keeping the segment overrides when
warning 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
@@ -7198,10 +7198,17 @@ duplicate:
     }
 
   if ((i.seg[0] || i.prefix[SEG_PREFIX])
-      && !quiet_warnings
       && ((i.tm.base_opcode == 0x8d && !is_any_vex_encoding(&i.tm)) /* lea */
           || ((i.tm.base_opcode | 0x010001) == 0xf30f1b) /* bnd{c[lnu],mk} */))
-    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
@@ -505,6 +505,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,21 @@
+#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
+[ 	]*[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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] x86: extend LEA's segment override warning
  2020-02-13 14:04 [PATCH 0/3] x86: segment override handling adjustments Jan Beulich
  2020-02-13 14:05 ` [PATCH 3/3] x86: adjust segment override prefix emission Jan Beulich
  2020-02-13 14:05 ` [PATCH 2/3] x86: optimize away pointless segment overrides Jan Beulich
@ 2020-02-13 14:05 ` Jan Beulich
  2020-02-13 14:12   ` H.J. Lu
  2 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2020-02-13 14:05 UTC (permalink / raw)
  To: binutils; +Cc: H.J. Lu

For one both possible forms should be warned about. And then there are
a couple of MPX insns behaving LEA-like, which should be warned about in
the same way. Finally, 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. 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
@@ -7197,9 +7197,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 && !is_any_vex_encoding(&i.tm)) /* lea */
+          || ((i.tm.base_opcode | 0x010001) == 0xf30f1b) /* bnd{c[lnu],mk} */))
     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,20 @@
+#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
+[ 	]*[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
--- /dev/null
+++ b/gas/testsuite/gas/i386/lea.e
@@ -0,0 +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' .*
--- /dev/null
+++ b/gas/testsuite/gas/i386/lea.s
@@ -0,0 +1,16 @@
+	.text
+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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] x86: extend LEA's segment override warning
  2020-02-13 14:05 ` [PATCH 1/3] x86: extend LEA's segment override warning Jan Beulich
@ 2020-02-13 14:12   ` H.J. Lu
  2020-02-13 14:48     ` Jan Beulich
  0 siblings, 1 reply; 11+ messages in thread
From: H.J. Lu @ 2020-02-13 14:12 UTC (permalink / raw)
  To: Jan Beulich; +Cc: binutils

On Thu, Feb 13, 2020 at 6:05 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> For one both possible forms should be warned about. And then there are
> a couple of MPX insns behaving LEA-like, which should be warned about in
> the same way. Finally, 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. 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.
>

Why should it be warning, not error?


-- 
H.J.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] x86: extend LEA's segment override warning
  2020-02-13 14:12   ` H.J. Lu
@ 2020-02-13 14:48     ` Jan Beulich
  2020-02-13 15:51       ` H.J. Lu
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2020-02-13 14:48 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On 13.02.2020 15:11, H.J. Lu wrote:
> On Thu, Feb 13, 2020 at 6:05 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> For one both possible forms should be warned about. And then there are
>> a couple of MPX insns behaving LEA-like, which should be warned about in
>> the same way. Finally, 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. 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.
>>
> 
> Why should it be warning, not error?

Because the code isn't wrong, just inefficient. I also don't think
converting from warning to error should be done in the same patch
as extending the coverage of what gets a diagnostic emitted.

Jan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] x86: extend LEA's segment override warning
  2020-02-13 14:48     ` Jan Beulich
@ 2020-02-13 15:51       ` H.J. Lu
  2020-02-13 16:00         ` Jan Beulich
  0 siblings, 1 reply; 11+ messages in thread
From: H.J. Lu @ 2020-02-13 15:51 UTC (permalink / raw)
  To: Jan Beulich; +Cc: binutils

On Thu, Feb 13, 2020 at 6:48 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 13.02.2020 15:11, H.J. Lu wrote:
> > On Thu, Feb 13, 2020 at 6:05 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> For one both possible forms should be warned about. And then there are
> >> a couple of MPX insns behaving LEA-like, which should be warned about in
> >> the same way. Finally, 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. 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.
> >>
> >
> > Why should it be warning, not error?
>
> Because the code isn't wrong, just inefficient. I also don't think
> converting from warning to error should be done in the same patch
> as extending the coverage of what gets a diagnostic emitted.
>

What do we gain to allow it?


-- 
H.J.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] x86: extend LEA's segment override warning
  2020-02-13 15:51       ` H.J. Lu
@ 2020-02-13 16:00         ` Jan Beulich
  2020-02-13 16:32           ` H.J. Lu
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2020-02-13 16:00 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On 13.02.2020 16:51, H.J. Lu wrote:
> On Thu, Feb 13, 2020 at 6:48 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 13.02.2020 15:11, H.J. Lu wrote:
>>> On Thu, Feb 13, 2020 at 6:05 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> For one both possible forms should be warned about. And then there are
>>>> a couple of MPX insns behaving LEA-like, which should be warned about in
>>>> the same way. Finally, 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. 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.
>>>>
>>>
>>> Why should it be warning, not error?
>>
>> Because the code isn't wrong, just inefficient. I also don't think
>> converting from warning to error should be done in the same patch
>> as extending the coverage of what gets a diagnostic emitted.
> 
> What do we gain to allow it?

It has always been allowed, and in one of the two cases even silently.
We're liable to break people's working code if we changed this.

Jan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] x86: extend LEA's segment override warning
  2020-02-13 16:00         ` Jan Beulich
@ 2020-02-13 16:32           ` H.J. Lu
  2020-02-13 16:38             ` Jan Beulich
  0 siblings, 1 reply; 11+ messages in thread
From: H.J. Lu @ 2020-02-13 16:32 UTC (permalink / raw)
  To: Jan Beulich; +Cc: binutils

On Thu, Feb 13, 2020 at 8:00 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 13.02.2020 16:51, H.J. Lu wrote:
> > On Thu, Feb 13, 2020 at 6:48 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 13.02.2020 15:11, H.J. Lu wrote:
> >>> On Thu, Feb 13, 2020 at 6:05 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> For one both possible forms should be warned about. And then there are
> >>>> a couple of MPX insns behaving LEA-like, which should be warned about in
> >>>> the same way. Finally, 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. 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.
> >>>>
> >>>
> >>> Why should it be warning, not error?
> >>
> >> Because the code isn't wrong, just inefficient. I also don't think
> >> converting from warning to error should be done in the same patch
> >> as extending the coverage of what gets a diagnostic emitted.
> >
> > What do we gain to allow it?
>
> It has always been allowed, and in one of the two cases even silently.
> We're liable to break people's working code if we changed this.
>

Given that MPX has been deprecated, MPX codes are very unlikely to
change.  Assembler shouldn't bother with this.

-- 
H.J.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] x86: extend LEA's segment override warning
  2020-02-13 16:32           ` H.J. Lu
@ 2020-02-13 16:38             ` Jan Beulich
  2020-02-13 16:45               ` H.J. Lu
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2020-02-13 16:38 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On 13.02.2020 17:31, H.J. Lu wrote:
> On Thu, Feb 13, 2020 at 8:00 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 13.02.2020 16:51, H.J. Lu wrote:
>>> On Thu, Feb 13, 2020 at 6:48 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 13.02.2020 15:11, H.J. Lu wrote:
>>>>> On Thu, Feb 13, 2020 at 6:05 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>
>>>>>> For one both possible forms should be warned about. And then there are
>>>>>> a couple of MPX insns behaving LEA-like, which should be warned about in
>>>>>> the same way. Finally, 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. 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.
>>>>>>
>>>>>
>>>>> Why should it be warning, not error?
>>>>
>>>> Because the code isn't wrong, just inefficient. I also don't think
>>>> converting from warning to error should be done in the same patch
>>>> as extending the coverage of what gets a diagnostic emitted.
>>>
>>> What do we gain to allow it?
>>
>> It has always been allowed, and in one of the two cases even silently.
>> We're liable to break people's working code if we changed this.
> 
> Given that MPX has been deprecated, MPX codes are very unlikely to
> change.  Assembler shouldn't bother with this.

What a strange position to take. Anyway, are you saying the change is
going to be okay if I remove the MPX aspects from it? (It is only
this 3rd reply of yours where you mention MPX, so I'm a little
puzzled.)

Jan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] x86: extend LEA's segment override warning
  2020-02-13 16:38             ` Jan Beulich
@ 2020-02-13 16:45               ` H.J. Lu
  0 siblings, 0 replies; 11+ messages in thread
From: H.J. Lu @ 2020-02-13 16:45 UTC (permalink / raw)
  To: Jan Beulich; +Cc: binutils

On Thu, Feb 13, 2020 at 8:38 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 13.02.2020 17:31, H.J. Lu wrote:
> > On Thu, Feb 13, 2020 at 8:00 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 13.02.2020 16:51, H.J. Lu wrote:
> >>> On Thu, Feb 13, 2020 at 6:48 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 13.02.2020 15:11, H.J. Lu wrote:
> >>>>> On Thu, Feb 13, 2020 at 6:05 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>
> >>>>>> For one both possible forms should be warned about. And then there are
> >>>>>> a couple of MPX insns behaving LEA-like, which should be warned about in
> >>>>>> the same way. Finally, 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. 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.
> >>>>>>
> >>>>>
> >>>>> Why should it be warning, not error?
> >>>>
> >>>> Because the code isn't wrong, just inefficient. I also don't think
> >>>> converting from warning to error should be done in the same patch
> >>>> as extending the coverage of what gets a diagnostic emitted.
> >>>
> >>> What do we gain to allow it?
> >>
> >> It has always been allowed, and in one of the two cases even silently.
> >> We're liable to break people's working code if we changed this.
> >
> > Given that MPX has been deprecated, MPX codes are very unlikely to
> > change.  Assembler shouldn't bother with this.
>
> What a strange position to take. Anyway, are you saying the change is
> going to be okay if I remove the MPX aspects from it? (It is only
> this 3rd reply of yours where you mention MPX, so I'm a little
> puzzled.)
>

Yes.

-- 
H.J.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-02-13 16:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-13 14:04 [PATCH 0/3] x86: segment override handling adjustments Jan Beulich
2020-02-13 14:05 ` [PATCH 3/3] x86: adjust segment override prefix emission Jan Beulich
2020-02-13 14:05 ` [PATCH 2/3] x86: optimize away pointless segment overrides Jan Beulich
2020-02-13 14:05 ` [PATCH 1/3] x86: extend LEA's segment override warning Jan Beulich
2020-02-13 14:12   ` H.J. Lu
2020-02-13 14:48     ` Jan Beulich
2020-02-13 15:51       ` H.J. Lu
2020-02-13 16:00         ` Jan Beulich
2020-02-13 16:32           ` H.J. Lu
2020-02-13 16:38             ` Jan Beulich
2020-02-13 16:45               ` H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).