[PATCH] x86: relax redundant REX prefix check

[H.J. Lu]

On Wed, May 30, 2018 at 12:42 AM, Jan Beulich <JBeulich@suse.com> wrote:
> All REX bits can be specified via individual prefixes. Redundancy should
> only be reported on a per-bit basis.
>
> Note that I originally had further checks added to the test case,
> checking the effect also on PDEP. I had to strip those, because my patch
> to correctly handle those
> (https://sourceware.org/ml/binutils/2017-02/msg00280.html) was rejected.
> I continue to think that there should not be any new prefix introduced
> to handle the VEX case - whether the encoding of an insn requires VEX et
> al should not be of immediate interest to the programmer.
>
> gas/
> 2018-05-30  Jan Beulich  <jbeulich@suse.com>
>
>         * config/tc-i386.c (add_prefix): Check REX bits individually.
>         * testsuite/gas/i386/rex.s: Add tests for overriding individual
>         REX bits, including when others are already set.
>         * testsuite/gas/i386/ilp32/rex.d, testsuite/gas/i386/rex.d:
>         Adjust expectations.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -2328,8 +2328,9 @@ add_prefix (unsigned int prefix)
>        && flag_code == CODE_64BIT)
>      {
>        if ((i.prefix[REX_PREFIX] & prefix & REX_W)
> -         || ((i.prefix[REX_PREFIX] & (REX_R | REX_X | REX_B))
> -             && (prefix & (REX_R | REX_X | REX_B))))
> +         || (i.prefix[REX_PREFIX] & prefix & REX_R)
> +         || (i.prefix[REX_PREFIX] & prefix & REX_X)
> +         || (i.prefix[REX_PREFIX] & prefix & REX_B))
>         ret = PREFIX_EXIST;
>        q = REX_PREFIX;
>      }
> --- a/gas/testsuite/gas/i386/ilp32/rex.d
> +++ b/gas/testsuite/gas/i386/ilp32/rex.d
> @@ -16,6 +16,14 @@ Disassembly of section .text:
>  [       ]*[0-9a-f]+:[   ]+4a 0f ae 04 05 00 00 00 00[   ]+fxsave64[     ]+(0x0)?\(,%r8(,1)?\)
>  [       ]*[0-9a-f]+:[   ]+43 0f ae 04 00[       ]+fxsave[       ]+\(%r8,%r8(,1)?\)
>  [       ]*[0-9a-f]+:[   ]+4b 0f ae 04 00[       ]+fxsave64[     ]+\(%r8,%r8(,1)?\)
> +[       ]*[0-9a-f]+:[   ]+48 03 04 00[  ]+add[  ]+\(%rax,%rax(,1)?\),%rax
> +[       ]*[0-9a-f]+:[   ]+44 03 04 00[  ]+add[  ]+\(%rax,%rax(,1)?\),%r8d
> +[       ]*[0-9a-f]+:[   ]+41 03 04 00[  ]+add[  ]+\(%r8,%rax(,1)?\),%eax
> +[       ]*[0-9a-f]+:[   ]+42 03 04 00[  ]+add[  ]+\(%rax,%r8(,1)?\),%eax
> +[       ]*[0-9a-f]+:[   ]+49 03 04 00[  ]+add[  ]+\(%r8,%rax(,1)?\),%rax
> +[       ]*[0-9a-f]+:[   ]+46 03 04 00[  ]+add[  ]+\(%rax,%r8(,1)?\),%r8d
> +[       ]*[0-9a-f]+:[   ]+45 03 04 00[  ]+add[  ]+\(%r8,%rax(,1)?\),%r8d
> +[       ]*[0-9a-f]+:[   ]+4a 03 04 00[  ]+add[  ]+\(%rax,%r8(,1)?\),%rax
>  [       ]*[0-9a-f]+:[   ]+41\s+rex\.B
>  [       ]*[0-9a-f]+:[   ]+9b dd 30\s+fsave\s+\(%rax\)
>  [       ]*[0-9a-f]+:[   ]+9b 41 dd 30\s+fsave\s+\(%r8\)
> --- a/gas/testsuite/gas/i386/rex.d
> +++ b/gas/testsuite/gas/i386/rex.d
> @@ -15,6 +15,14 @@ Disassembly of section .text:
>  [       ]*[0-9a-f]+:[   ]+4a 0f ae 04 05 00 00 00 00[   ]+fxsave64[     ]+(0x0)?\(,%r8(,1)?\)
>  [       ]*[0-9a-f]+:[   ]+43 0f ae 04 00[       ]+fxsave[       ]+\(%r8,%r8(,1)?\)
>  [       ]*[0-9a-f]+:[   ]+4b 0f ae 04 00[       ]+fxsave64[     ]+\(%r8,%r8(,1)?\)
> +[       ]*[0-9a-f]+:[   ]+48 03 04 00[  ]+add[  ]+\(%rax,%rax(,1)?\),%rax
> +[       ]*[0-9a-f]+:[   ]+44 03 04 00[  ]+add[  ]+\(%rax,%rax(,1)?\),%r8d
> +[       ]*[0-9a-f]+:[   ]+41 03 04 00[  ]+add[  ]+\(%r8,%rax(,1)?\),%eax
> +[       ]*[0-9a-f]+:[   ]+42 03 04 00[  ]+add[  ]+\(%rax,%r8(,1)?\),%eax
> +[       ]*[0-9a-f]+:[   ]+49 03 04 00[  ]+add[  ]+\(%r8,%rax(,1)?\),%rax
> +[       ]*[0-9a-f]+:[   ]+46 03 04 00[  ]+add[  ]+\(%rax,%r8(,1)?\),%r8d
> +[       ]*[0-9a-f]+:[   ]+45 03 04 00[  ]+add[  ]+\(%r8,%rax(,1)?\),%r8d
> +[       ]*[0-9a-f]+:[   ]+4a 03 04 00[  ]+add[  ]+\(%rax,%r8(,1)?\),%rax
>  [       ]*[0-9a-f]+:[   ]+41\s+rex\.B
>  [       ]*[0-9a-f]+:[   ]+9b dd 30\s+fsave\s+\(%rax\)
>  [       ]*[0-9a-f]+:[   ]+9b 41 dd 30\s+fsave\s+\(%r8\)
> --- a/gas/testsuite/gas/i386/rex.s
> +++ b/gas/testsuite/gas/i386/rex.s
> @@ -10,6 +10,16 @@ _start:
>         rex/fxsave (%r8,%r8)
>         rex64/fxsave (%r8,%r8)
>
> +       rex.w add       (%rax,%rax), %eax
> +       rex.r add       (%rax,%rax), %eax
> +       rex.b add       (%rax,%rax), %eax
> +       rex.x add       (%rax,%rax), %eax
> +
> +       rex.w add       (%r8,%rax), %eax
> +       rex.r add       (%rax,%r8), %eax
> +       rex.b add       (%rax,%rax), %r8d

Since R and B bits have been set in this case, shouldn't rex.r/rex.b
be the extra
REX prefix?

> +       rex.x add       (%rax,%rax), %rax
> +
>



-- 
H.J.