RE: [PATCH] [gdb/tdep] Add XOP support in amd64_get_insn_details

["Gerlicher, Klaus" <klaus.gerlicher@intel.com>]

Hi Tom,

Thanks for working on this, I was tempted to also add it also but since this is only supported on 10 year old AMD CPUs, so I didn't bother.

One inline comment/question. Seems fine to me.

Reviewed-By: Klaus Gerlicher<klaus.gerlicher.@intel.com>

Thanks
Klaus

> -----Original Message-----
> From: Tom de Vries <tdevries@suse.de>
> Sent: Tuesday, August 26, 2025 10:59 AM
> To: gdb-patches@sourceware.org
> Cc: Gerlicher, Klaus <klaus.gerlicher@intel.com>
> Subject: [PATCH] [gdb/tdep] Add XOP support in amd64_get_insn_details
> 
> Implement support for XOP instructions [1] in amd64_get_insn_details.
> 
> The encoding scheme is documented here [2].  Essentially it's a variant of the
> VEX3 encoding scheme, with:
> - 0x8f as the first byte instead of 0xc4, and
> - with an opcode map >= 8.
> 
> The changes are roughly the same as the XOP part of an earlier submission [3],
> hence the tag.
> 
> The only real difference is that that patch proposed to implement xop_prefix_p
> using:
> ...
>   return pfx[0] == 0x8f && (pfx[1] & 0x38);
> ...
> which is incorrect because the 0x38 mask selects bits bit 3-5, and bit 5 is
> not part of the opcode map in bits 0-4.
> 
> Instead, use:
> ...
>   gdb_byte m = pfx[1] & 0x1f;
>   return pfx[0] == 0x8f && m >= 8;

AMD SDM says

0x8 8 bit immediate
0x9  no immediate 
0xa 32-bit immediate.

Did you see any other maps somewhere? Or is this just to distinguish from POP?

> ...
> 
> Tested on x86_64-linux.
> 
> Co-Authored-By: Jan Beulich <jbeulich@suse.com>
> 
> [1] https://en.wikipedia.org/wiki/XOP_instruction_set
> [2] https://www.amd.com/content/dam/amd/en/documents/archived-tech-
> docs/programmer-references/43479.pdf
> [3] https://sourceware.org/pipermail/gdb-patches/2019-
> February/155347.html
> ---
>  gdb/amd64-tdep.c | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index d5ea4aff4cf..d20f490a0f4 100644
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -1181,6 +1181,15 @@ vex3_prefix_p (gdb_byte pfx)
>    return pfx == 0xc4;
>  }
> 
> +/* True if PFX is the start of an XOP prefix.  */
> +
> +static bool
> +xop_prefix_p (const gdb_byte *pfx)
> +{
> +  gdb_byte m = pfx[1] & 0x1f;
> +  return pfx[0] == 0x8f && m >= 8;
> +}
> +
>  /* Return true if PFX is the start of the 4-byte EVEX prefix.  */
> 
>  static bool
> @@ -1351,7 +1360,7 @@ amd64_get_insn_details (gdb_byte *insn, struct
> amd64_insn *details)
>        details->enc_prefix_offset = insn - start;
>        insn += 2;
>      }
> -  else if (vex3_prefix_p (*insn))
> +  else if (vex3_prefix_p (*insn) || xop_prefix_p (insn))
>      {
>        details->enc_prefix_offset = insn - start;
>        insn += 3;
> @@ -1438,6 +1447,11 @@ amd64_get_insn_details (gdb_byte *insn, struct
> amd64_insn *details)
>  	  return;
>  	}
>      }
> +  else if (prefix != nullptr && xop_prefix_p (prefix))
> +    {
> +      details->opcode_len = 1;
> +      need_modrm = 1;
> +    }
>    else if (*insn == TWO_BYTE_OPCODE_ESCAPE)
>      {
>        /* Two or three-byte opcode.  */
> @@ -1508,7 +1522,7 @@ fixup_riprel (const struct amd64_insn &details,
> gdb_byte *insn,
>  	{
>  	  /* VEX.!B is set implicitly.  */
>  	}
> -      else if (vex3_prefix_p (pfx[0]))
> +      else if (vex3_prefix_p (pfx[0]) || xop_prefix_p (pfx))
>  	pfx[1] |= VEX3_NOT_B;
>        else if (evex_prefix_p (pfx[0]))
>  	{
> @@ -3755,6 +3769,20 @@ test_amd64_get_insn_details (void)
>      = { 0x62, 0xf1, 0x7c, 0x48, 0x28, 0x81, 0x00, 0xfc, 0xff, 0xff };
>    fixup_riprel (details, insn.data (), ECX_REG_NUM);
>    SELF_CHECK (insn == updated_insn);
> +
> +  /* INSN: vpcomtrueuq 0x0(%rip),%xmm7,%xmm0, xop prefix.  */
> +  insn = { 0x8f, 0xe8, 0x40, 0xef, 0x05, 0x00, 0x00, 0x00, 0x00, 0x07 };
> +  amd64_get_insn_details (insn.data (), &details);
> +  SELF_CHECK (details.opcode_len == 1);
> +  SELF_CHECK (details.enc_prefix_offset == 0);
> +  SELF_CHECK (details.opcode_offset == 3);
> +  SELF_CHECK (details.modrm_offset == 4);
> +
> +  /* INSN: vpcomtrueuq 0x0(%ecx),%xmm7,%xmm0, xop prefix.  */
> +  fixup_riprel (details, insn.data (), ECX_REG_NUM);
> +  updated_insn
> +    = { 0x8f, 0xe8, 0x40, 0xef, 0x81, 0x00, 0x00, 0x00, 0x00, 0x07 };
> +  SELF_CHECK (insn == updated_insn);
>  }
> 
>  static void
> 
> base-commit: 5319c8dec64aa5e37c56da2b0cfe77a1886231ca
> --
> 2.43.0
> 

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928