Re: [PATCH 2/2] gdb/record: add support for BMI2 instructions
Guinevere Larsen <[email protected]>
| Newsgroups | gmane.comp.gdb.patches |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]>; [email protected] >> Subject: RE: [PATCH 2/2] gdb/record: add support for BMI2 instructions >> >>> -----Original Message----- >>> From: Guinevere Larsen <[email protected]> >>> Sent: Dienstag, 7. Juli 2026 15:57 >>> To: Schimpe, Christina <[email protected]>; gdb- >>> [email protected] >>> 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 <[email protected]> >>>>> Sent: Freitag, 3. Juli 2026 20:22 >>>>> To: [email protected] >>>>> 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)