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

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

[Tom de Vries]

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;
...

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

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

[Gerlicher, Klaus]

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

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

[Tom de Vries]

On 8/26/25 15:20, Gerlicher, Klaus wrote:
> 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.
> 

Hi Klaus,

thanks for the review.

FWIW, I don't know how relevant this patch is, but new gdbs are used on 
both older and recent architectures, so it could be relevant for someone.

> 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.
>>

After reading this ( 
https://en.wikipedia.org/wiki/XOP_instruction_set#cite_note-5 ) I 
realized what this is trying to do: detect that it's not a POP.  I've 
updated the commit message accordingly.

>> 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?

I haven't looked for that, I've only looked at the specification of the 
prefix.  I'm fine with being "forward compatible" with any bitcode map 
with m >= 8.

The test m >=8 is part of the XOP specification:
...
Prefix Byte 0
Byte 0 of the XOP prefix is set to 8Fh. This signifies an XOP prefix 
only in conjunction with the mmmmm field of the following byte being 
greater than or equal to 8; if the mmmmm field is less than
8 then these two bytes are a form of the POP instruction rather than an 
XOP prefix.
...

I've pushed this.

Thanks,
- Tom

> 
>> ...
>>
>> 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
> 

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

[Simon Marchi]

On 8/26/25 3:42 PM, Tom de Vries wrote:
> On 8/26/25 15:20, Gerlicher, Klaus wrote:
>> 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.
>>
> 
> Hi Klaus,
> 
> thanks for the review.
> 
> FWIW, I don't know how relevant this patch is, but new gdbs are used on both older and recent architectures, so it could be relevant for someone.

OOC, do compilers actually produce these instructions by default?  I
would guess not.  Do they produce them if you select the right
-mcpu/-march?  Perhaps they appear in some ifunc specifically crafted
for these CPUs?

Simon

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

[Sam James]

Simon Marchi <simark@simark.ca> writes:

> On 8/26/25 3:42 PM, Tom de Vries wrote:
>> On 8/26/25 15:20, Gerlicher, Klaus wrote:
>>> 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.
>>>
>> 
>> Hi Klaus,
>> 
>> thanks for the review.
>> 
>> FWIW, I don't know how relevant this patch is, but new gdbs are used on both older and recent architectures, so it could be relevant for someone.
>
> OOC, do compilers actually produce these instructions by default?  I
> would guess not.  Do they produce them if you select the right
> -mcpu/-march?  Perhaps they appear in some ifunc specifically crafted
> for these CPUs?

Yeah, they appear with the right -march=. It came up recently with the
kernel even: https://lore.kernel.org/regressions/175386161199.564247.597496379413236944.stgit@devnote2/

sam