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: Tue, 7 Jul 2026 09:18:47 +0000	[thread overview]
Message-ID: <SN7PR11MB763815D3F572F88117331605F9F02@SN7PR11MB7638.namprd11.prod.outlook.com> (raw)
In-Reply-To: <70b4fa57-d4c5-4c8c-95b0-0db51fe586ef@redhat.com>

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.

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

  reply	other threads:[~2026-07-07  9: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 [this message]
2026-07-07 13:57         ` Guinevere Larsen
2026-07-09 13:19           ` Schimpe, Christina

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=SN7PR11MB763815D3F572F88117331605F9F02@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