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)
prev parent 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