Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Schimpe, Christina" <christina.schimpe@intel.com>
To: Guinevere Larsen <guinevere@redhat.com>,
	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: RE: [PATCH 2/2] gdb/record: add support for BMI2 instructions
Date: Thu, 9 Jul 2026 13:19:15 +0000	[thread overview]
Message-ID: <SN7PR11MB7638C4AA91F9A1A98368E6A6F9FE2@SN7PR11MB7638.namprd11.prod.outlook.com> (raw)
In-Reply-To: <fb75749d-26b8-465b-9c9a-f292a4f70230@redhat.com>

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

Christina

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

      reply	other threads:[~2026-07-09 13:19 UTC|newest]

Thread overview: 7+ 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 [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=SN7PR11MB7638C4AA91F9A1A98368E6A6F9FE2@SN7PR11MB7638.namprd11.prod.outlook.com \
    --to=christina.schimpe@intel.com \
    --cc=gdb-patches@sourceware.org \
    --cc=guinevere@redhat.com \
    /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