Re: [PATCH 8/8] Support APX JMPABS

[Jan Beulich <jbeulich@suse.com>]

On 19.09.2023 17:25, Cui, Lili wrote:
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -3880,6 +3880,7 @@ is_any_apx_encoding (void)
>      || i.tm.opcode_space == SPACE_EVEXMAP4
>      || i.has_nf
>      || i.has_zero_upper
> +    || i.tm.mnem_off == MN_jmpabs
>      || (i.vex.register_specifier
>  	&& i.vex.register_specifier->reg_flags & RegRex2);
>  }
> @@ -3887,7 +3888,8 @@ is_any_apx_encoding (void)
>  static INLINE bool
>  is_any_apx_rex2_encoding (void)
>  {
> -  return (i.rex2 && i.vex.length == 2) || i.rex2_encoding;
> +  return (i.rex2 && i.vex.length == 2) || i.rex2_encoding
> +	  || i.tm.mnem_off == MN_jmpabs;
>  }

These are getting both too complex for my taste, when they - iirc - sit
on a relatively common (and hence hot) path.

> @@ -7703,7 +7705,7 @@ match_template (char mnem_suffix)
>    if (!quiet_warnings)
>      {
>        if (!intel_syntax
> -	  && (i.jumpabsolute != (t->opcode_modifier.jump == JUMP_ABSOLUTE)))
> +	  && (i.jumpabsolute != (t->opcode_modifier.jump == JUMP_ABSOLUTE) && t->mnem_off != MN_jmpabs))

Nit: Too long line. But - does this need changing? JMPABS isn't an
indirect jump.

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/apx-mov-inval.l
> @@ -0,0 +1,2 @@
> +.* Assembler messages:
> +.*:5: Error: unsupport rex2 pseudo prefix for `mov'

What is this new file doing here, entirely in isolation?

> @@ -2057,7 +2061,7 @@ static const struct dis386 dis386[] = {
>    { "lahf",		{ XX }, 0 },
>    /* a0 */
>    { "mov%LB",		{ AL, Ob }, 0 },
> -  { "mov%LS",		{ eAX, Ov }, 0 },
> +  { "mov%LS",		{ { JMPABS_Fixup, eAX_reg }, { JMPABS_Fixup, v_mode } }, 0 },
>    { "mov%LB",		{ Ob, AL }, 0 },
>    { "mov%LS",		{ Ov, eAX }, 0 },
>    { "movs{b|}",		{ Ybr, Xb }, 0 },
> @@ -9690,6 +9694,15 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
>        goto out;
>      }
>  
> +  if (ins.jmpabs
> +      && ((ins.prefixes & (PREFIX_OPCODE | PREFIX_ADDR)) != 0x0
> +	  || (ins.rex2_payload & 0x8) != 0x0))
> +    {
> +      i386_dis_printf (info, dis_style_text, "(bad)");
> +      ret = ins.end_codep - priv.the_buffer;
> +      goto out;
> +    }

Why can't this be dealt with in JMPABS_Fixup()?

> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -161,7 +161,7 @@
>  
>  // Move instructions.
>  mov, 0xa0, No64, D|W|CheckOperandSize|No_sSuf|No_qSuf, { Disp16|Disp32|Unspecified|Byte|Word|Dword, Acc|Byte|Word|Dword }
> -mov, 0xa0, x64, D|W|CheckOperandSize|No_sSuf, { Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }
> +mov, 0xa0, x64, D|W|CheckOperandSize|No_sSuf|No_egpr, { Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }

Isn't this (and perhaps more) needed already earlier? And can't
that be generalized to treating egpr as invalid for everything that
doesn't have Modrm?

>  movabs, 0xa0, x64, D|W|CheckOperandSize|No_sSuf, { Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }

Plus, why would this one not need a similar adjustment?

Jan