[PATCH] x86/Intel: SHLD/SHRD have dual meaning

[Hongtao Liu]

On Fri, Apr 19, 2024 at 5:29 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> Since we uniformly permit D suffixes in Intel mode whenever in AT&T mode
> an L suffix may be used, we need to be consistent with this.
I think we need to forbid the D suffix for APX NDD SHL/SHR under Intel
mode to avoid ambiguity.
Neither SHL (always SAL), nor SHR with a D suffix (Intel mode) is
generated by GCC.
>
> Take the easy route, despite that still leading to an anomaly which is
> also visible from the new testcase:
>
>         shld    eax, ecx, 1
>         shld    eax, ecx, cl
>
> can mean two things with APX: SHL with a D suffix in NDD EVEX encoding,
> or the traditional SHLD in legacy encoding.
> ---
> The alternative, more intrusive and more risky (in terms of perceived or
> even real regressions) route would be to mark the few insns which permit
> suffixes even in Intel syntax, and reject suffix uses when that
> indicator isn't set.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -5389,7 +5389,7 @@ static void init_globals (void)
>  }
>
>  /* Helper for md_assemble() to decide whether to prepare for a possible 2nd
> -   parsing pass. Instead of introducing a rarely use new insn attribute this
> +   parsing pass. Instead of introducing a rarely used new insn attribute this
>     utilizes a common pattern between affected templates. It is deemed
>     acceptable that this will lead to unnecessary pass 2 preparations in a
>     limited set of cases.  */
> @@ -5401,7 +5401,10 @@ static INLINE bool may_need_pass2 (const
>          : (t->opcode_space == SPACE_0F
>             && (t->base_opcode | 1) == 0xbf)
>            || (t->opcode_space == SPACE_BASE
> -              && t->base_opcode == 0x63);
> +              && t->base_opcode == 0x63)
> +          || (intel_syntax /* shld / shrd may mean suffixed shl / shr.  */
> +              && t->opcode_space == SPACE_EVEXMAP4
> +              && (t->base_opcode | 8) == 0x2c);
>  }
>
>  #if defined (OBJ_MAYBE_ELF) || defined (OBJ_ELF)
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/intel-suffix.d
> @@ -0,0 +1,34 @@
> +#objdump: -dw
> +#name: Intel syntax w/ suffixes
> +
> +.*: +file format .*
> +
> +Disassembly of section \.text:
> +0+0 <.*>:
> +[      ]*[a-f0-9]+:    0f a4 c8 01[    ]+shld   \$0x1,%ecx,%eax
> +[      ]*[a-f0-9]+:    0f a5 c8[       ]+shld   %cl,%ecx,%eax
> +[      ]*[a-f0-9]+:    d1 e1[  ]+shl    \$1,%ecx
> +[      ]*[a-f0-9]+:    d3 e1[  ]+shl    %cl,%ecx
> +[      ]*[a-f0-9]+:    62 f4 7c 18 d1 e1[      ]+shl    \$1,%ecx,%eax
> +[      ]*[a-f0-9]+:    62 f4 7c 18 d3 e1[      ]+shl    %cl,%ecx,%eax
> +[      ]*[a-f0-9]+:    d1 e1[  ]+shl    \$1,%ecx
> +[      ]*[a-f0-9]+:    d3 e1[  ]+shl    %cl,%ecx
> +[      ]*[a-f0-9]+:    62 f4 7c 18 d1 c1[      ]+rol    \$1,%ecx,%eax
> +[      ]*[a-f0-9]+:    62 f4 7c 18 d3 c1[      ]+rol    %cl,%ecx,%eax
> +[      ]*[a-f0-9]+:    d1 c1[  ]+rol    \$1,%ecx
> +[      ]*[a-f0-9]+:    d3 c1[  ]+rol    %cl,%ecx
> +
> +0+[0-9a-f]+ <.*>:
> +[      ]*[a-f0-9]+:    0f ac c8 01[    ]+shrd   \$0x1,%ecx,%eax
> +[      ]*[a-f0-9]+:    0f ad c8[       ]+shrd   %cl,%ecx,%eax
> +[      ]*[a-f0-9]+:    d1 e9[  ]+shr    \$1,%ecx
> +[      ]*[a-f0-9]+:    d3 e9[  ]+shr    %cl,%ecx
> +[      ]*[a-f0-9]+:    62 f4 7c 18 d1 f9[      ]+sar    \$1,%ecx,%eax
> +[      ]*[a-f0-9]+:    62 f4 7c 18 d3 f9[      ]+sar    %cl,%ecx,%eax
> +[      ]*[a-f0-9]+:    d1 f9[  ]+sar    \$1,%ecx
> +[      ]*[a-f0-9]+:    d3 f9[  ]+sar    %cl,%ecx
> +[      ]*[a-f0-9]+:    62 f4 7c 18 d1 c9[      ]+ror    \$1,%ecx,%eax
> +[      ]*[a-f0-9]+:    62 f4 7c 18 d3 c9[      ]+ror    %cl,%ecx,%eax
> +[      ]*[a-f0-9]+:    d1 c9[  ]+ror    \$1,%ecx
> +[      ]*[a-f0-9]+:    d3 c9[  ]+ror    %cl,%ecx
> +#pass
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/intel-suffix.s
> @@ -0,0 +1,39 @@
> +       .intel_syntax noprefix
> +       .text
> +left:
> +       shld    eax, ecx, 1
> +       shld    eax, ecx, cl
> +
> +       shld    ecx, 1
> +       shld    ecx, cl
> +
> +       sald    eax, ecx, 1
> +       sald    eax, ecx, cl
> +
> +       sald    ecx, 1
> +       sald    ecx, cl
> +
> +       rold    eax, ecx, 1
> +       rold    eax, ecx, cl
> +
> +       rold    ecx, 1
> +       rold    ecx, cl
> +
> +right:
> +       shrd    eax, ecx, 1
> +       shrd    eax, ecx, cl
> +
> +       shrd    ecx, 1
> +       shrd    ecx, cl
> +
> +       sard    eax, ecx, 1
> +       sard    eax, ecx, cl
> +
> +       sard    ecx, 1
> +       sard    ecx, cl
> +
> +       rord    eax, ecx, 1
> +       rord    eax, ecx, cl
> +
> +       rord    ecx, 1
> +       rord    ecx, cl
> --- a/gas/testsuite/gas/i386/x86-64.exp
> +++ b/gas/testsuite/gas/i386/x86-64.exp
> @@ -160,6 +160,7 @@ run_dump_test "x86-64-disp-intel"
>  run_list_test "disp-imm-64"
>  run_dump_test "intel-movs64"
>  run_dump_test "intel-cmps64"
> +run_dump_test "intel-suffix"
>  run_dump_test "x86-64-disp32"
>  run_dump_test "rexw"
>  run_list_test "x86-64-specific-reg"



-- 
BR,
Hongtao