* [PATCH v2 0/4] x86: segment override handling adjustments
@ 2020-02-14 11:41 Jan Beulich
2020-02-14 11:43 ` [PATCH v2 1/4] x86: extend LEA's segment override warning Jan Beulich
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Jan Beulich @ 2020-02-14 11:41 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
4: extend LEA's segment override warning to applicable MPX insns
Jan
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/4] x86: extend LEA's segment override warning
2020-02-14 11:41 [PATCH v2 0/4] x86: segment override handling adjustments Jan Beulich
@ 2020-02-14 11:43 ` Jan Beulich
2020-02-14 12:03 ` H.J. Lu
2020-02-14 11:43 ` [PATCH v2 2/4] x86: optimize away pointless segment overrides Jan Beulich
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2020-02-14 11:43 UTC (permalink / raw)
To: binutils; +Cc: H.J. Lu
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/4] x86: optimize away pointless segment overrides
2020-02-14 11:41 [PATCH v2 0/4] x86: segment override handling adjustments Jan Beulich
2020-02-14 11:43 ` [PATCH v2 1/4] x86: extend LEA's segment override warning Jan Beulich
@ 2020-02-14 11:43 ` Jan Beulich
2020-02-14 12:06 ` H.J. Lu
2020-02-14 11:44 ` [PATCH v2 4/4] x86: extend LEA's segment override warning to applicable MPX insns Jan Beulich
2020-02-14 11:44 ` [PATCH v2 3/4] x86: adjust segment override prefix emission Jan Beulich
3 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2020-02-14 11:43 UTC (permalink / raw)
To: binutils; +Cc: H.J. Lu
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 4/4] x86: extend LEA's segment override warning to applicable MPX insns
2020-02-14 11:41 [PATCH v2 0/4] x86: segment override handling adjustments Jan Beulich
2020-02-14 11:43 ` [PATCH v2 1/4] x86: extend LEA's segment override warning Jan Beulich
2020-02-14 11:43 ` [PATCH v2 2/4] x86: optimize away pointless segment overrides Jan Beulich
@ 2020-02-14 11:44 ` Jan Beulich
2020-02-14 12:05 ` H.J. Lu
2020-02-14 11:44 ` [PATCH v2 3/4] x86: adjust segment override prefix emission Jan Beulich
3 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2020-02-14 11:44 UTC (permalink / raw)
To: binutils; +Cc: H.J. Lu
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/4] x86: adjust segment override prefix emission
2020-02-14 11:41 [PATCH v2 0/4] x86: segment override handling adjustments Jan Beulich
` (2 preceding siblings ...)
2020-02-14 11:44 ` [PATCH v2 4/4] x86: extend LEA's segment override warning to applicable MPX insns Jan Beulich
@ 2020-02-14 11:44 ` Jan Beulich
2020-02-14 12:09 ` H.J. Lu
3 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2020-02-14 11:44 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
@@ -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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] x86: extend LEA's segment override warning
2020-02-14 11:43 ` [PATCH v2 1/4] x86: extend LEA's segment override warning Jan Beulich
@ 2020-02-14 12:03 ` H.J. Lu
2020-02-14 12:06 ` Jan Beulich
0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2020-02-14 12:03 UTC (permalink / raw)
To: Jan Beulich; +Cc: binutils
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] x86: extend LEA's segment override warning to applicable MPX insns
2020-02-14 11:44 ` [PATCH v2 4/4] x86: extend LEA's segment override warning to applicable MPX insns Jan Beulich
@ 2020-02-14 12:05 ` H.J. Lu
2020-02-14 12:08 ` Jan Beulich
0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2020-02-14 12:05 UTC (permalink / raw)
To: Jan Beulich; +Cc: binutils
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] x86: optimize away pointless segment overrides
2020-02-14 11:43 ` [PATCH v2 2/4] x86: optimize away pointless segment overrides Jan Beulich
@ 2020-02-14 12:06 ` H.J. Lu
0 siblings, 0 replies; 12+ messages in thread
From: H.J. Lu @ 2020-02-14 12:06 UTC (permalink / raw)
To: Jan Beulich; +Cc: binutils
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] x86: extend LEA's segment override warning
2020-02-14 12:03 ` H.J. Lu
@ 2020-02-14 12:06 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2020-02-14 12:06 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] x86: extend LEA's segment override warning to applicable MPX insns
2020-02-14 12:05 ` H.J. Lu
@ 2020-02-14 12:08 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2020-02-14 12:08 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/4] x86: adjust segment override prefix emission
2020-02-14 11:44 ` [PATCH v2 3/4] x86: adjust segment override prefix emission Jan Beulich
@ 2020-02-14 12:09 ` H.J. Lu
2020-02-14 12:17 ` Jan Beulich
0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2020-02-14 12:09 UTC (permalink / raw)
To: Jan Beulich; +Cc: binutils
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/4] x86: adjust segment override prefix emission
2020-02-14 12:09 ` H.J. Lu
@ 2020-02-14 12:17 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2020-02-14 12:17 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-02-14 12:17 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14 11:41 [PATCH v2 0/4] x86: segment override handling adjustments Jan Beulich
2020-02-14 11:43 ` [PATCH v2 1/4] x86: extend LEA's segment override warning Jan Beulich
2020-02-14 12:03 ` H.J. Lu
2020-02-14 12:06 ` Jan Beulich
2020-02-14 11:43 ` [PATCH v2 2/4] x86: optimize away pointless segment overrides Jan Beulich
2020-02-14 12:06 ` H.J. Lu
2020-02-14 11:44 ` [PATCH v2 4/4] x86: extend LEA's segment override warning to applicable MPX insns Jan Beulich
2020-02-14 12:05 ` H.J. Lu
2020-02-14 12:08 ` Jan Beulich
2020-02-14 11:44 ` [PATCH v2 3/4] x86: adjust segment override prefix emission Jan Beulich
2020-02-14 12:09 ` H.J. Lu
2020-02-14 12:17 ` Jan Beulich
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).