Re: [PATCH 2/2] gdb/record: add support for BMI2 instructions

[Guinevere Larsen <guinevere@redhat.com>]

On 7/7/26 6:18 AM, Schimpe, Christina wrote:
> Hi Guinevere,
>
> If I run this on an Intel cpu I see the following.
>
> ~~~
> Running /tmp/gdb.reverse/i386-avx-reverse.exp ...
> FAIL: gdb.reverse/i386-avx-reverse.exp: verify r8 before bzhi
> FAIL: gdb.reverse/i386-avx-reverse.exp: verify ecx before shrx
> FAIL: gdb.reverse/i386-avx-reverse.exp: verify eax before sarx
> FAIL: gdb.reverse/i386-avx-reverse.exp: verify ebx before rorx
> FAIL: gdb.reverse/i386-avx-reverse.exp: verify ebx before pdep
> FAIL: gdb.reverse/i386-avx-reverse.exp: verify ecx before mulx
> FAIL: gdb.reverse/i386-avx-reverse.exp: verify r8 before tzcnt
> FAIL: gdb.reverse/i386-avx-reverse.exp: verify ebx before tzcnt
> FAIL: gdb.reverse/i386-avx-reverse.exp: verify ecx before tzcnt
> FAIL: gdb.reverse/i386-avx-reverse.exp: verify ecx before blsr
> FAIL: gdb.reverse/i386-avx-reverse.exp: verify eax before blsmsk
> FAIL: gdb.reverse/i386-avx-reverse.exp: verify ecx before blsi
> FAIL: gdb.reverse/i386-avx-reverse.exp: verify eax before bextr
> ~~~
>
> Log:
> ~~~
> ecx            0xfb27ea2f          -81270225^M
> (gdb) FAIL: gdb.reverse/i386-avx-reverse.exp: verify ecx before shrx
> reverse-step^M
> 810       asm volatile ("shlx %eax, %ecx, %ebx");^M
> (gdb) PASS: gdb.reverse/i386-avx-reverse.exp: reverse-step from shlx to test register ebx
> info register ebx^M
> ebx            0x0                 0^M
> (gdb) PASS: gdb.reverse/i386-avx-reverse.exp: verify ebx before shlx
> reverse-step^M
> 809       asm volatile ("sarx %edx, %ebx, %eax");^M
> (gdb) PASS: gdb.reverse/i386-avx-reverse.exp: reverse-step from sarx to test register eax
> info register eax^M
> eax            0xffffffff          -1^M
> (gdb) FAIL: gdb.reverse/i386-avx-reverse.exp: verify eax before sarx
> reverse-step^M
> 808       asm volatile ("rorx $10, %r8, %rbx");^M
> (gdb) PASS: gdb.reverse/i386-avx-reverse.exp: reverse-step from rorx to test register ebx
> info register ebx^M
> ebx            0xd7                215^M
> (gdb) FAIL: gdb.reverse/i386-avx-reverse.exp: verify ebx before rorx
> [...]
> ~~~
>
> If I revert the commits
> - "gdb/record: add support for BMI2 instructions"
> - "gdb/record: Add support for recording BMI1 instructions"
> the fails disappear.

Well, they disappear because this commit is where those tests were 
introduced. This works fine inmy AMD machine, so the question is, is 
this an issue of recording or a difference in how the instructions work?
Could you run this function forward, setting the registers manually, and 
check if the test expects the right values in the registers?

>
> I'd have given this a review (especially the x86 part) but didn't see this soon enough
> since I've been out for a couple of weeks.
>
> For now, I just run the test.
>
> Christina
>
>> -----Original Message-----
>> From: Guinevere Larsen <guinevere@redhat.com>
>> Sent: Freitag, 3. Juli 2026 20:22
>> To: gdb-patches@sourceware.org
>> Subject: Re: [PATCH 2/2] gdb/record: add support for BMI2 instructions
>>
>> I'm pushing this series with a trivial change to the test, fixing an oversight I had
>> when writing the original patch
>>
>> On 6/17/26 11:00 AM, Guinevere Larsen wrote:
>>> This commit adds support for recording all instructions in the Bit
>>> Manipulation Instruction set 2, for x86 cpus. The specific
>>> instructions
>>> are:
>>>
>>> * mulx
>>> * pdep
>>> * pext
>>> * rorx
>>> * sarx
>>> * sh[r|l]x
>>> * bzhi
>>>
>>> And adds them to the AVX test, for the same reason as the previous
>>> commit.
>>> ---
>>>    gdb/i386-tdep.c                                | 18 ++++++++++++++++--
>>>    gdb/testsuite/gdb.reverse/i386-avx-reverse.c   | 13 +++++++++++++
>>>    gdb/testsuite/gdb.reverse/i386-avx-reverse.exp | 14 ++++++++++++++
>>>    3 files changed, 43 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index
>>> 8b4dfb43f61..189a711fd81 100644
>>> --- a/gdb/i386-tdep.c
>>> +++ b/gdb/i386-tdep.c
>>> @@ -5128,7 +5128,7 @@ i386_record_vex (struct i386_record_s *ir,
>> uint8_t vex_w, uint8_t vex_r,
>>>        case 0xf2:	/* VPSLLD, dynamic shift and ANDN.  */
>>>        case 0xf3:	/* VPSLLQ, dynamic shift and BLSI, BLSR and BLSMSK.  */
>>>        case 0xf4:	/* VPMULUDQ  */
>>> -    case 0xf6:	/* VPSADBW.  */
>>> +    case 0xf6:	/* VPSADBW or MULX.  */
>>>        case 0xfc:	/* VPADDB  */
>>>        case 0xfd:	/* VPADDW  */
>>>        case 0xfe:	/* VPADDD  */
>>> @@ -5152,6 +5152,15 @@ i386_record_vex (struct i386_record_s *ir,
>> uint8_t vex_w, uint8_t vex_r,
>>>    	    record_full_arch_list_add_reg
>>>    	      (ir->regcache, ir->regmap[X86_RECORD_EFLAGS_REGNUM]);
>>>    	  }
>>> +	else if (opcode == 0xf6 && ir->map_select == 2)
>>> +	  {
>>> +	    record_full_arch_list_add_reg (ir->regcache,
>>> +					   ir-
>>> regmap[X86_RECORD_REAX_REGNUM
>>> +						      + ir->vvvv]);
>>> +	    record_full_arch_list_add_reg (ir->regcache,
>>> +					   ir-
>>> regmap[X86_RECORD_REAX_REGNUM
>>> +						      + reg_offset]);
>>> +	  }
>>>    	else
>>>    	  {
>>>    	    /* This set of instructions all share the same exact way to @@
>>> -5164,11 +5173,16 @@ i386_record_vex (struct i386_record_s *ir, uint8_t
>> vex_w, uint8_t vex_r,
>>>          }
>>>          break;
>>>
>>> -    case 0xf7:	/* BEXTR.  */
>>> +    case 0xf0:	/* RORX.  */
>>> +    case 0xf5:	/* PDEP or PEXT or BZHI.  */
>>> +    case 0xf7:	/* BEXTR or SARX or SHLX or SHRX.  */
>>>          i386_record_modrm (ir);
>>>          record_full_arch_list_add_reg (ir->regcache,
>>>    				     ir->regmap[X86_RECORD_REAX_REGNUM
>>>    						+ ir->reg + vex_r * 8]);
>>> +      if (opcode == 0xf5 && ir->pp == 0)
>>> +	record_full_arch_list_add_reg (ir->regcache,
>>> +				       ir-
>>> regmap[X86_RECORD_EFLAGS_REGNUM]);
>>>          break;
>>>
>>>        case 0x2e: /* VUCOMIS[S|D].  */
>>> diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
>>> b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
>>> index 190bc6f4a7a..4e4428f695a 100644
>>> --- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
>>> +++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
>>> @@ -799,6 +799,19 @@ bmi_test ()
>>>      asm volatile ("tzcnt %eax, %ebx");
>>>      asm volatile ("tzcnt %ax, %r8w");
>>>
>>> +  asm volatile ("mulx %eax, %ebx, %ecx");  asm volatile ("mulx %ebx,
>>> + %ecx, %ebx");
>>> +
>>> +  asm volatile ("pdep %r8, %rcx, %rbx");  asm volatile ("pext %edx,
>>> + %ecx, %ebx");  asm volatile ("rorx $10, %r8, %rbx");  asm volatile
>>> + ("sarx %edx, %ebx, %eax");  asm volatile ("shlx %eax, %ecx, %ebx");
>>> + asm volatile ("shrx %r8d, %eax, %ecx");
>>> +
>>> +  asm volatile ("bzhi %edx, %ebx, %r8d");  asm volatile ("bzhi %rax,
>>> + %rcx, %r8");
>>> +
>>>      return 0; /* end bmi_test  */
>>>    }
>>>
>>> diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
>>> b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
>>> index e1780267ad0..a482142edde 100644
>>> --- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
>>> +++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
>>> @@ -1206,9 +1206,23 @@ gdb_test_no_output \
>>>        "set \$eax = 0x1312" "set eax for bmi test"
>>>    gdb_test_no_output "set \$ebx = 0xdeadbeef" "set ebx for bmi test"
>>>    gdb_test_no_output "set \$ecx = 0xcafeface" "set ecx for bmi test"
>>> +gdb_test_no_output "set \$edx = 0x2337" "set edx for bmi test"
>>>
>>>    if {[record_full_function "bmi"] == true} {
>>>
>>> +    test_one_general_register "bzhi" "r8" "0xe884173"
>>> +    test_one_general_register "bzhi" "r8" "0x0"
>>> +
>>> +    test_one_general_register "shrx" "ecx" "0x0"
>> This was updated to look for 0xe884173
>>
>> --
>> Cheers,
>> Guinevere Larsen
>> it/its
>> she/her (deprecated)
>>
>>> +    test_one_general_register "shlx" "ebx" "0x0"
>>> +    test_one_general_register "sarx" "eax" "0x3"
>>> +    test_one_general_register "rorx" "ebx" "0x3b"
>>> +    test_one_general_register "pext" "ebx" "0x0"
>>> +    test_one_general_register "pdep" "ebx" "0x0"
>>> +
>>> +    test_one_general_register "mulx" "ecx" "0x0"
>>> +    test_one_general_register "mulx" "ebx" "0x0"
>>> +
>>>        test_one_general_register "tzcnt" "r8" "0x3e"
>>>        test_one_general_register "tzcnt" "ebx" "0xe"
>>>        test_one_general_register "tzcnt" "ecx" "0x1e"
> Intel Deutschland GmbH
>
> Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
> Tel: +49 89 991 430, www.intel.de
> Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Seat: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928
>

-- 
Cheers,
Guinevere Larsen
it/its
she/her (deprecated)