Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Guinevere Larsen <guinevere@redhat.com>
To: "Joos, Christina" <christina.joos@intel.com>,
	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH 2/2] gdb/record: add support for BMI2 instructions
Date: Mon, 10 Aug 2026 11:46:30 -0300	[thread overview]
Message-ID: <5d16c0a1-8a8e-44f5-8e29-c379dba1bde1@redhat.com> (raw)
In-Reply-To: <SN7PR11MB763808B15B99B378BF2A474289DE2@SN7PR11MB7638.namprd11.prod.outlook.com>

On 8/10/26 11:11 AM, Joos, Christina wrote:
>> -----Original Message-----
>> From: Schimpe, Christina
>> Sent: Donnerstag, 9. Juli 2026 15:19
>> To: Guinevere Larsen <guinevere@redhat.com>; gdb-patches@sourceware.org
>> Subject: RE: [PATCH 2/2] gdb/record: add support for BMI2 instructions
>>
>>> -----Original Message-----
>>> From: Guinevere Larsen <guinevere@redhat.com>
>>> Sent: Dienstag, 7. Juli 2026 15:57
>>> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
>>> patches@sourceware.org
>>> Subject: Re: [PATCH 2/2] gdb/record: add support for BMI2 instructions
>>>
>>> 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)
>> I think it's an issue with the test setup,  since eax in my setup is 0 at the
>> beginning of bmi_test function (which is expected since it's a caller saved
>> register). If I manually set it to 0x1312 (which is the value you initially set using
>> GDB) AFTER calling bmi_test but before executing the instructions, the test
>> passes again.
>> ~~~
>> (gdb) si
>> bmi_test () at /tmp/gdb.reverse/i386-avx-reverse.c:770
>> 770     {
>> (gdb) info register $eax
>> eax            0x0                 0
>> (gdb) set $eax = 0x1312
>> (gdb)
>>                  === gdb Summary ===
>>
>> # of expected passes            852
>> ~~~
>>
>> I think you even mention this in a comment:
>>
>> # Because of the infrastructure of the test, we can't set rax.
>> # However, it seems to always be set to 0, so this should be fine.
>>
>> In that specific bmi test you configure eax to a value =! 0, so I think that's the
>> reason.
>> I guess the safest would be to only use callee saved registers in the entire test.
>>
>> But it's interesting still that for your environment the test is passing, too.
>>
>> If I modify the test to use r12/r12d/r12w instead of rax/eax/ax it passes also on
>> my machine.
>>
>> What do you think?
> Hi Guinevere,
>
> Do you have an update on this ?

Hi!

Sorry, I completely missed your answer somehow. I'll give this a shot 
today and see how it goes. Should have the patch before long

>
> Thanks,
> Christina
> ________________________________________
> Intel Deutschland GmbH
>
> Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
>
> Tel: +49 (89) 99143-0
>
> www.intel.de
>
> Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman
>
> Chairperson of the Supervisory Board: Sonja Pierer
>
> Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>

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


      reply	other threads:[~2026-08-10 14:47 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-17 12:41 [PATCH 0/2] Add support for BMI1 and BMI2 recording Guinevere Larsen
2026-06-17 14:00 ` [PATCH 1/2] gdb/record: Add support for recording BMI1 instructions Guinevere Larsen
2026-06-17 14:00   ` [PATCH 2/2] gdb/record: add support for BMI2 instructions Guinevere Larsen
2026-07-03 18:22     ` Guinevere Larsen
2026-07-07  9:18       ` Schimpe, Christina
2026-07-07 13:57         ` Guinevere Larsen
2026-07-09 13:19           ` Schimpe, Christina
2026-08-10 14:11             ` Joos, Christina
2026-08-10 14:46               ` Guinevere Larsen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5d16c0a1-8a8e-44f5-8e29-c379dba1bde1@redhat.com \
    --to=guinevere@redhat.com \
    --cc=christina.joos@intel.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox