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] gdb/record: avoid rax usage in i386-avx-record.exp
Date: Thu, 13 Aug 2026 10:04:57 -0300	[thread overview]
Message-ID: <0f068fec-3d51-4e58-beb8-f8293002bdff@redhat.com> (raw)
In-Reply-To: <SN7PR11MB76382D8FDE6D920988781EAB89DB2@SN7PR11MB7638.namprd11.prod.outlook.com>

On 8/13/26 3:17 AM, Joos, Christina wrote:
>> -----Original Message-----
>> From: Joos, Christina
>> Sent: Mittwoch, 12. August 2026 15:31
>> To: 'Guinevere Larsen' <guinevere@redhat.com>; gdb-
>> patches@sourceware.org
>> Subject: RE: [PATCH] gdb/record: avoid rax usage in i386-avx-record.exp
>>
>>> -----Original Message-----
>>> From: Guinevere Larsen <guinevere@redhat.com>
>>> Sent: Montag, 10. August 2026 21:33
>>> To: gdb-patches@sourceware.org
>>> Cc: Guinevere Larsen <guinevere@redhat.com>; Joos, Christina
>>> <christina.joos@intel.com>
>>> Subject: [PATCH] gdb/record: avoid rax usage in i386-avx-record.exp
>>>
>>> My recent commit, 1c04308f0958cbac83bad22703758139ea6969b6:
>>>
>>>      Author: Guinevere Larsen <guinevere@redhat.com>
>>>      Date:   Tue Jun 16 16:03:43 2026 -0300
>>>      gdb/record: add support for BMI2 instructions
>>>
>>> Introduced a few tests that rely on the rax register being set before
>>> entering the function. This isn't reliable, as eax is almost always
>>> set to 0 (except in my machine for some reason). To avoid ISAs getting
>>> in the way, this commit changes the test to use r12 (and r12w/r12d
>>> when appropriate), so that it won't be disturbed by the compiler.
>>>
>>> Reported-by: Christina Joos <christina.joos@intel.com>
>>> ---
>>>   gdb/testsuite/gdb.reverse/i386-avx-reverse.c  | 32 +++++++++----------
>>>   .../gdb.reverse/i386-avx-reverse.exp          |  8 ++---
>>>   2 files changed, 20 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
>>> b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
>>> index 88cd3fc7f34..4d45482c07a 100644
>>> --- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
>>> +++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
>>> @@ -770,48 +770,48 @@ bmi_test ()
>>>   {
>>>     /* start bmi_test.  */
>>>     /* Using GDB, load these values onto registers for testing.
>>> -     eax = 0x1312
>>> +     r12 = 0x1312
>>>        ebx = 0xdeadbeef
>>>        ecx = 0xcafeface
>>>        edx = 0x2337
>>>        r8  = 0
>>>        this way it's easy to confirm we're undoing things correctly.
>>> */
>>>
>>> -  asm volatile ("andn %rbx, %rax, %r8");
>>> -  asm volatile ("andn %ebx, %eax, %ecx");
>>> +  asm volatile ("andn %rbx, %r12, %r8");  asm volatile ("andn %ebx,
>>> + %r12d, %ecx");
>>>
>>>     asm volatile ("bextr %rcx, %r8, %rbx");
>>>     asm volatile ("bextr %ebx, %ecx, %r8d");
>>>
>>> -  asm volatile ("blsi %rax, %rcx");
>>> +  asm volatile ("blsi %r12, %rcx");
>>>     asm volatile ("blsi %ebx, %r8d");
>>>
>>>     asm volatile ("blsmsk %r8, %rbx");
>>> -  asm volatile ("blsmsk %eax, %eax");
>>> +  asm volatile ("blsmsk %r12d, %r12d");
>>>
>>>     asm volatile ("blsr %rcx, %rbx");
>>>     asm volatile ("blsr %r8d, %ecx");
>>>
>>> -  asm volatile ("lzcnt %rax, %r8");
>>> -  asm volatile ("lzcnt %eax, %ecx");
>>> -  asm volatile ("lzcnt %ax, %bx");
>>> +  asm volatile ("lzcnt %r12, %r8");
>>> +  asm volatile ("lzcnt %r12d, %ecx");  asm volatile ("lzcnt %r12w,
>>> + %bx");
>>>
>>> -  asm volatile ("tzcnt %rax, %rcx");
>>> -  asm volatile ("tzcnt %eax, %ebx");
>>> -  asm volatile ("tzcnt %ax, %r8w");
>>> +  asm volatile ("tzcnt %r12, %rcx");
>>> +  asm volatile ("tzcnt %r12d, %ebx");  asm volatile ("tzcnt %r12w,
>>> + %r8w");
>>>
>>> -  asm volatile ("mulx %eax, %ebx, %ecx");
>>> +  asm volatile ("mulx %r12d, %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 ("sarx %edx, %ebx, %r12d");  asm volatile ("shlx
>>> + %r12d, %ecx, %ebx");  asm volatile ("shrx %r8d, %r12d, %ecx");
>>>
>>>     asm volatile ("bzhi %edx, %ebx, %r8d");
>>> -  asm volatile ("bzhi %rax, %rcx, %r8");
>>> +  asm volatile ("bzhi %r12, %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 5d65245db39..a5b0e6612aa 100644
>>> --- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
>>> +++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
>>> @@ -1201,7 +1201,7 @@ gdb_test "finish" "Run till exit
>>> from.*convert_test.*" \ gdb_test_no_output \
>>>       "set \$r8 = 0" "set r8 for bmi test"
>>>   gdb_test_no_output \
>>> -    "set \$eax = 0x1312" "set eax for bmi test"
>>> +    "set \$r12 = 0x1312" "set r12 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"
>>> @@ -1213,7 +1213,7 @@ if {[record_full_function "bmi"] == true} {
>>>
>>>       test_one_general_register "shrx" "ecx" "0xe884173"
>>>       test_one_general_register "shlx" "ebx" "0x0"
>>> -    test_one_general_register "sarx" "eax" "0x3"
>>> +    test_one_general_register "sarx" "r12" "0x3"
>>>       test_one_general_register "rorx" "ebx" "0x3b"
>>>       test_one_general_register "pext" "ebx" "0x0"
>>>       test_one_general_register "pdep" "ebx" "0x0"
>>> @@ -1232,13 +1232,13 @@ if {[record_full_function "bmi"] == true} {
>>>       test_one_general_register "blsr" "ecx" "0x2"
>>>       test_one_general_register "blsr" "ebx" "0xffffffff"
>>>
>>> -    test_one_general_register "blsmsk" "eax" "0x1312"
>>> +    test_one_general_register "blsmsk" "r12" "0x1312"
>>>       test_one_general_register "blsmsk" "ebx" "0x0"
>>>
>>>       test_one_general_register "blsi" "r8" "0x0"
>>>       test_one_general_register "blsi" "ecx" "0xdeadaced"
>>>
>>> -    test_one_general_register "bextr" "eax" "0x1312"
>>> +    test_one_general_register "bextr" "r12" "0x1312"
>>>       test_one_general_register "bextr" "ebx" "0xdeadbeef"
>>>
>>>       test_one_general_register "andn" "ecx" "0xcafeface"
>>>
>>> base-commit: a80fede20bc1330eca5e419392c6595bb3a6ac1d
>>> --
>>> 2.55.0
>> Hi Guinevere,
>>
>> Thank you for this patch.
>> My machine has been updated recently to Ubuntu 26.04 and now the test
>> passes also for me (without your patch).
>> Reason is that I now use a newer gcc version (gcc-15) by default.
>>
>> Using gcc-14 it still fails, so maybe it would make sense to mention that in the
>> commit message.
>>
>> I still think using callee saved registers in the entire test would be better, also to
>> avoid future mistakes.
>> But since it's now passing again, I guess this is also fine.
>>
>> So with that fixed:
> This comment might not be so clear about "that", so to clarify:
>
> With a comment about the compiler in the commit message.
>
> Reviewed-by: Christina Joos <christina.joos@intel.com>

Oh, thanks for double checking the compiler version, and the quick review!

I'll send a v2 to the list soon.

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

> Kind Regards,
> 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.
>


      reply	other threads:[~2026-08-13 13:05 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-10 19:32 Guinevere Larsen
2026-08-12 13:31 ` Joos, Christina
2026-08-13  6:17   ` Joos, Christina
2026-08-13 13:04     ` 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=0f068fec-3d51-4e58-beb8-f8293002bdff@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