Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH 1/1] [gdb/reverse] Fix failing test: i386-avx-reverse
@ 2025-07-19 20:00 Shiven Kashyap
  2025-07-20  2:48 ` Tom de Vries
  0 siblings, 1 reply; 3+ messages in thread
From: Shiven Kashyap @ 2025-07-19 20:00 UTC (permalink / raw)
  To: gdb-patches; +Cc: guinevere, Shiven Kashyap

Running the standalone test `gdb.reverse` leads to the following failure:
'''
FAIL: gdb.reverse/i386-avx-reverse.exp: verify ymm15 before vbroadcastsd
'''

This happens because the AVX broadcast instructions expect an actual value, but instead
it cast memory address of a buffer like so (see 'vpbroadcast_test' in 'i386-avx-reverse.c'):
'''
asm volatile ("vbroadcastss %0, %%xmm15": : "m" (dyn_buf0));
'''

which lead to the test failing for the next instruction (`vbroadcastsd`), which depended on the correct value being broadcast to the register.
Also, updated the corresponding expected output (gdb.reverse/i386-avx-reverse.exp) to match.

Tested on x86-64 Linux.

Signed-off-by: Shiven Kashyap <shivenkashyap24@gmail.com>
---
 gdb/testsuite/gdb.reverse/i386-avx-reverse.c   | 4 ++--
 gdb/testsuite/gdb.reverse/i386-avx-reverse.exp | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
index a3d64272f24..bf6cb77f313 100644
--- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
@@ -299,10 +299,10 @@ vpbroadcast_test ()
   asm volatile ("vbroadcastss %xmm1, %xmm0");
   asm volatile ("vbroadcastss %xmm1, %ymm15");
   asm volatile ("vbroadcastss %0, %%ymm0" : : "m" (global_buf0));
-  asm volatile ("vbroadcastss %0, %%xmm15": : "m" (dyn_buf0));
+  asm volatile ("vbroadcastss %0, %%xmm15": : "m" (*dyn_buf0));
   asm volatile ("vbroadcastsd %xmm1, %ymm0");
   asm volatile ("vbroadcastsd %0, %%ymm15": : "m" (global_buf0));
-  asm volatile ("vbroadcastf128 %0, %%ymm0" : : "m" (dyn_buf0));
+  asm volatile ("vbroadcastf128 %0, %%ymm0" : : "m" (*dyn_buf0));
 
   /* We have a return statement to deal with
      epilogue in different compilers.  */
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
index 7e75542720c..fb04260766e 100644
--- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
@@ -395,7 +395,7 @@ if {[record_full_function "vpbroadcast"] == true} {
     test_one_register "vbroadcastf128" "ymm0" \
 	"0x17161514131211101716151413121110, 0x17161514131211101716151413121110"
     test_one_register "vbroadcastsd" "ymm15" \
-	"0x404060004040600040406000404060, 0x0"
+	"0x23222120232221202322212023222120, 0x0"
     test_one_register "vbroadcastsd" "ymm0" \
 	"0x13121110131211101312111013121110, 0x13121110131211101312111013121110"
 
-- 
2.50.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] [gdb/reverse] Fix failing test: i386-avx-reverse
  2025-07-19 20:00 [PATCH 1/1] [gdb/reverse] Fix failing test: i386-avx-reverse Shiven Kashyap
@ 2025-07-20  2:48 ` Tom de Vries
  2025-07-21 19:14   ` Guinevere Larsen
  0 siblings, 1 reply; 3+ messages in thread
From: Tom de Vries @ 2025-07-20  2:48 UTC (permalink / raw)
  To: Shiven Kashyap, gdb-patches; +Cc: guinevere

On 7/19/25 22:00, Shiven Kashyap wrote:
> Running the standalone test `gdb.reverse` leads to the following failure:
> '''
> FAIL: gdb.reverse/i386-avx-reverse.exp: verify ymm15 before vbroadcastsd
> '''
> 

Hi,

thanks for the patch.

I first couldn't reproduce this, but then I managed using target board 
unix/-fPIE/-pie.

> This happens because the AVX broadcast instructions expect an actual value, but instead
> it cast memory address of a buffer like so (see 'vpbroadcast_test' in 'i386-avx-reverse.c'):
> '''
> asm volatile ("vbroadcastss %0, %%xmm15": : "m" (dyn_buf0));
> '''
> 

This is a bit misleading, because vbroadcastss can handle memory 
operands, not just "actual values".

After your fix (which looks correct to me), the generated instruction 
still accesses a memory address:
...
   400abd:       48 8b 05 04 26 00 00    mov    0x2604(%rip),%rax
                                           # 4030c8 <dyn_buf0>
   400ac4:       c4 62 79 18 38          vbroadcastss (%rax),%xmm15
...
but this time, the correct one.

Thanks,
- Tom

> which lead to the test failing for the next instruction (`vbroadcastsd`), which depended on the correct value being broadcast to the register.
> Also, updated the corresponding expected output (gdb.reverse/i386-avx-reverse.exp) to match.
> 
> Tested on x86-64 Linux.
> 
> Signed-off-by: Shiven Kashyap <shivenkashyap24@gmail.com>
> ---
>   gdb/testsuite/gdb.reverse/i386-avx-reverse.c   | 4 ++--
>   gdb/testsuite/gdb.reverse/i386-avx-reverse.exp | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
> index a3d64272f24..bf6cb77f313 100644
> --- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
> +++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
> @@ -299,10 +299,10 @@ vpbroadcast_test ()
>     asm volatile ("vbroadcastss %xmm1, %xmm0");
>     asm volatile ("vbroadcastss %xmm1, %ymm15");
>     asm volatile ("vbroadcastss %0, %%ymm0" : : "m" (global_buf0));
> -  asm volatile ("vbroadcastss %0, %%xmm15": : "m" (dyn_buf0));
> +  asm volatile ("vbroadcastss %0, %%xmm15": : "m" (*dyn_buf0));
>     asm volatile ("vbroadcastsd %xmm1, %ymm0");
>     asm volatile ("vbroadcastsd %0, %%ymm15": : "m" (global_buf0));
> -  asm volatile ("vbroadcastf128 %0, %%ymm0" : : "m" (dyn_buf0));
> +  asm volatile ("vbroadcastf128 %0, %%ymm0" : : "m" (*dyn_buf0));
>   
>     /* We have a return statement to deal with
>        epilogue in different compilers.  */
> diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
> index 7e75542720c..fb04260766e 100644
> --- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
> +++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
> @@ -395,7 +395,7 @@ if {[record_full_function "vpbroadcast"] == true} {
>       test_one_register "vbroadcastf128" "ymm0" \
>   	"0x17161514131211101716151413121110, 0x17161514131211101716151413121110"
>       test_one_register "vbroadcastsd" "ymm15" \
> -	"0x404060004040600040406000404060, 0x0"
> +	"0x23222120232221202322212023222120, 0x0"
>       test_one_register "vbroadcastsd" "ymm0" \
>   	"0x13121110131211101312111013121110, 0x13121110131211101312111013121110"
>   


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] [gdb/reverse] Fix failing test: i386-avx-reverse
  2025-07-20  2:48 ` Tom de Vries
@ 2025-07-21 19:14   ` Guinevere Larsen
  0 siblings, 0 replies; 3+ messages in thread
From: Guinevere Larsen @ 2025-07-21 19:14 UTC (permalink / raw)
  To: Tom de Vries, Shiven Kashyap, gdb-patches

Hi! Thank you for this patch!

I have a comments, slightly expanding Tom's reply. A v2 with updated 
commit wording should be ready to push :)
On 7/19/25 11:48 PM, Tom de Vries wrote:
> On 7/19/25 22:00, Shiven Kashyap wrote:
>> Running the standalone test `gdb.reverse` leads to the following 
>> failure:
>> '''
>> FAIL: gdb.reverse/i386-avx-reverse.exp: verify ymm15 before vbroadcastsd
>> '''
>>
>
> Hi,
>
> thanks for the patch.
>
> I first couldn't reproduce this, but then I managed using target board 
> unix/-fPIE/-pie.
>
>> This happens because the AVX broadcast instructions expect an actual 
>> value, but instead
>> it cast memory address of a buffer like so (see 'vpbroadcast_test' in 
>> 'i386-avx-reverse.c'):
>> '''
>> asm volatile ("vbroadcastss %0, %%xmm15": : "m" (dyn_buf0));
>> '''
>>
>
> This is a bit misleading, because vbroadcastss can handle memory 
> operands, not just "actual values".

I second this, the instruction doesn't expect anything, but the test 
itself expects to have the contents in the address, not the address. I 
think the issue is better described as the test expecting the stored 
value, as those are fully under our control.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> After your fix (which looks correct to me), the generated instruction 
> still accesses a memory address:
> ...
>   400abd:       48 8b 05 04 26 00 00    mov    0x2604(%rip),%rax
>                                           # 4030c8 <dyn_buf0>
>   400ac4:       c4 62 79 18 38          vbroadcastss (%rax),%xmm15
> ...
> but this time, the correct one.
>
> Thanks,
> - Tom
>
>> which lead to the test failing for the next instruction 
>> (`vbroadcastsd`), which depended on the correct value being broadcast 
>> to the register.
>> Also, updated the corresponding expected output 
>> (gdb.reverse/i386-avx-reverse.exp) to match.
>>
>> Tested on x86-64 Linux.
>>
>> Signed-off-by: Shiven Kashyap <shivenkashyap24@gmail.com>
>> ---
>>   gdb/testsuite/gdb.reverse/i386-avx-reverse.c   | 4 ++--
>>   gdb/testsuite/gdb.reverse/i386-avx-reverse.exp | 2 +-
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c 
>> b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
>> index a3d64272f24..bf6cb77f313 100644
>> --- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
>> +++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
>> @@ -299,10 +299,10 @@ vpbroadcast_test ()
>>     asm volatile ("vbroadcastss %xmm1, %xmm0");
>>     asm volatile ("vbroadcastss %xmm1, %ymm15");
>>     asm volatile ("vbroadcastss %0, %%ymm0" : : "m" (global_buf0));
>> -  asm volatile ("vbroadcastss %0, %%xmm15": : "m" (dyn_buf0));
>> +  asm volatile ("vbroadcastss %0, %%xmm15": : "m" (*dyn_buf0));
>>     asm volatile ("vbroadcastsd %xmm1, %ymm0");
>>     asm volatile ("vbroadcastsd %0, %%ymm15": : "m" (global_buf0));
>> -  asm volatile ("vbroadcastf128 %0, %%ymm0" : : "m" (dyn_buf0));
>> +  asm volatile ("vbroadcastf128 %0, %%ymm0" : : "m" (*dyn_buf0));
>>       /* We have a return statement to deal with
>>        epilogue in different compilers.  */
>> diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp 
>> b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
>> index 7e75542720c..fb04260766e 100644
>> --- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
>> +++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
>> @@ -395,7 +395,7 @@ if {[record_full_function "vpbroadcast"] == true} {
>>       test_one_register "vbroadcastf128" "ymm0" \
>>       "0x17161514131211101716151413121110, 
>> 0x17161514131211101716151413121110"
>>       test_one_register "vbroadcastsd" "ymm15" \
>> -    "0x404060004040600040406000404060, 0x0"
>> +    "0x23222120232221202322212023222120, 0x0"
>>       test_one_register "vbroadcastsd" "ymm0" \
>>       "0x13121110131211101312111013121110, 
>> 0x13121110131211101312111013121110"
>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-21 19:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-19 20:00 [PATCH 1/1] [gdb/reverse] Fix failing test: i386-avx-reverse Shiven Kashyap
2025-07-20  2:48 ` Tom de Vries
2025-07-21 19:14   ` Guinevere Larsen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox