diff mbox series

[v5,16/19] gdbserver: Clear upper ZMM registers in the right location.

Message ID 20230427210113.45380-17-jhb@FreeBSD.org
State New
Headers
Series Handle variable XSAVE layouts |

Commit Message

John Baldwin April 27, 2023, 9:01 p.m. UTC 
  This was previously clearing the upper 32 bytes of ZMM0-15 rather than
ZMM16-31.
---
 gdbserver/i387-fp.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Simon Marchi May 3, 2023, 5:49 p.m. UTC | #1 
On 4/27/23 17:01, John Baldwin wrote:
> This was previously clearing the upper 32 bytes of ZMM0-15 rather than
> ZMM16-31.
> ---
>  gdbserver/i387-fp.cc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdbserver/i387-fp.cc b/gdbserver/i387-fp.cc
> index e63eef60330..a122e2d860b 100644
> --- a/gdbserver/i387-fp.cc
> +++ b/gdbserver/i387-fp.cc
> @@ -306,7 +306,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
>        if ((clear_bv & X86_XSTATE_ZMM))
>  	{
>  	  for (i = 0; i < num_avx512_zmmh_high_registers; i++)
> -	    memset (((char *) &fp->zmmh_low_space[0]) + 32 + i * 64, 0, 32);
> +	    memset (((char *) &fp->zmmh_high_space[0]) + 32 + i * 64, 0, 32);

From what I understand, this is correct, so:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Unrelated to your patch, but I believe that zmmh_high_space should be
called zmm_high_space, since it doesn't contain only upper values like
zmmh_low_space does, it contains the full values for ZMM 16 through 31.

Simon
John Baldwin May 3, 2023, 11:47 p.m. UTC | #2 
On 5/3/23 10:49 AM, Simon Marchi wrote:
> On 4/27/23 17:01, John Baldwin wrote:
>> This was previously clearing the upper 32 bytes of ZMM0-15 rather than
>> ZMM16-31.
>> ---
>>   gdbserver/i387-fp.cc | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gdbserver/i387-fp.cc b/gdbserver/i387-fp.cc
>> index e63eef60330..a122e2d860b 100644
>> --- a/gdbserver/i387-fp.cc
>> +++ b/gdbserver/i387-fp.cc
>> @@ -306,7 +306,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
>>         if ((clear_bv & X86_XSTATE_ZMM))
>>   	{
>>   	  for (i = 0; i < num_avx512_zmmh_high_registers; i++)
>> -	    memset (((char *) &fp->zmmh_low_space[0]) + 32 + i * 64, 0, 32);
>> +	    memset (((char *) &fp->zmmh_high_space[0]) + 32 + i * 64, 0, 32);
> 
>  From what I understand, this is correct, so:
> 
> Approved-By: Simon Marchi <simon.marchi@efficios.com>
> 
> Unrelated to your patch, but I believe that zmmh_high_space should be
> called zmm_high_space, since it doesn't contain only upper values like
> zmmh_low_space does, it contains the full values for ZMM 16 through 31.

Yeah, I think I do rename it to zmm_space () in patch 17.
diff mbox series

Patch

diff --git a/gdbserver/i387-fp.cc b/gdbserver/i387-fp.cc
index e63eef60330..a122e2d860b 100644
--- a/gdbserver/i387-fp.cc
+++ b/gdbserver/i387-fp.cc
@@ -306,7 +306,7 @@  i387_cache_to_xsave (struct regcache *regcache, void *buf)
       if ((clear_bv & X86_XSTATE_ZMM))
 	{
 	  for (i = 0; i < num_avx512_zmmh_high_registers; i++)
-	    memset (((char *) &fp->zmmh_low_space[0]) + 32 + i * 64, 0, 32);
+	    memset (((char *) &fp->zmmh_high_space[0]) + 32 + i * 64, 0, 32);
 	  for (i = 0; i < num_avx512_xmm_registers; i++)
 	    memset (((char *) &fp->zmmh_high_space[0]) + i * 64, 0, 16);
 	  for (i = 0; i < num_avx512_ymmh_registers; i++)