RE: [PATCH v3 8/8] Windows gdb: Implement AVX-512 register support

["Rohr, Stephan" <stephan.rohr@intel.com>]

Hi Hannes,

please see some inline feedback below.
Some of the feedback provided for 

  [PATCH v3 7/8] Windows gdb: Implement AVX register support

applies here as well.

Thanks

Stephan

> -----Original Message-----
> From: Hannes Domani <ssbssa@yahoo.de>
> Sent: Saturday, 29 August 2026 16:49
> To: gdb-patches@sourceware.org
> Subject: [PATCH v3 8/8] Windows gdb: Implement AVX-512 register support
> 
> This adds support for the Intel AVX-512 registers on Windows.
> It enables accessing registers $ymm0 - $ymm31, $zmm0 - $zmm31, and
> $k0 - $k7 where they are available.
> 
> After this patch gdb.arch/i386-avx512.exp passes on windows.
> ---
> v3:
>   - merged gdb+gdbserver parts, and split again AVX/AVX-512 parts
> ---
>  gdb/NEWS                    |  2 ++
>  gdb/nat/windows-nat.c       |  2 +-
>  gdb/x86-windows-nat.c       | 35 +++++++++++++++++++++++++++
>  gdbserver/win32-i386-low.cc | 48
> ++++++++++++++++++++++++++++++++++++-
>  4 files changed, 85 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index f7effc822e9..d3db6dd167e 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -120,6 +120,8 @@
> 
>  * Support for Intel AVX registers on Windows.
>    Support displaying and modifying Intel AVX registers $ymm0 - $ymm31.

Following the previous patch, this should update again to registers $ymm0 - $ymm31?

> +  Support displaying and modifying Intel AVX-512 registers $zmm0 - $zmm31
> +  and $k0 - $k7.
> 
>  * Configure changes
> 
> diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
> index c9a21d7c41f..30d49c07332 100644
> --- a/gdb/nat/windows-nat.c
> +++ b/gdb/nat/windows-nat.c
> @@ -1339,7 +1339,7 @@ initialize_loadable ()
>      {
>        /* Available XState features masked with implemented features.  */
>        xstate_features = (GetEnabledXStateFeatures ()
> -			 & X86_XSTATE_AVX_MASK);
> +			 & X86_XSTATE_AVX_AVX512_MASK);
>        /* The extended XState functions are only needed if the available
>  	 features exceed SSE.  */
>        if ((xstate_features & ~X86_XSTATE_SSE_MASK) == 0)
> diff --git a/gdb/x86-windows-nat.c b/gdb/x86-windows-nat.c
> index 1cefe6171be..425e343deca 100644
> --- a/gdb/x86-windows-nat.c
> +++ b/gdb/x86-windows-nat.c
> @@ -291,6 +291,41 @@ get_context_reg_ptr (Context *context, int r,
> i386_gdbarch_tdep *tdep)
>  	(context, X86_XSTATE_AVX_ID, NULL);
>        context_offset += 16 * (r - I387_YMM0H_REGNUM (tdep));
>      }
> +  else if (I387_ZMM0H_REGNUM (tdep) > 0 && r >= I387_ZMM0H_REGNUM
> (tdep)
> +	   && r < I387_ZMM16H_REGNUM (tdep) && r <
> I387_ZMMENDH_REGNUM (tdep))
> +    {

I basically have the same concern as in patch 7/8 regarding the guards on gdbserver
side but missing here.

> +      context_offset = (char *) locate_xstate_feature
> +	(context, X86_XSTATE_ZMM_H_ID, NULL);
> +      context_offset += 32 * (r - I387_ZMM0H_REGNUM (tdep));
> +    }
> +  else if (I387_ZMM0H_REGNUM (tdep) > 0 && r >=
> I387_ZMM16H_REGNUM (tdep)
> +	   && r < I387_ZMMENDH_REGNUM (tdep))
> +    {
> +      context_offset = (char *) locate_xstate_feature
> +	(context, X86_XSTATE_ZMM_ID, NULL);
> +      context_offset += 32 + 64 * (r - I387_ZMM16H_REGNUM (tdep));
> +    }
> +  else if (I387_K0_REGNUM (tdep) > 0 && r >= I387_K0_REGNUM (tdep)
> +	   && r < I387_KEND_REGNUM (tdep))
> +    {
> +      context_offset = (char *) locate_xstate_feature
> +	(context, X86_XSTATE_K_ID, NULL);
> +      context_offset += 8 * (r - I387_K0_REGNUM (tdep));
> +    }
> +  else if (I387_YMM16H_REGNUM (tdep) > 0 && r >=
> I387_YMM16H_REGNUM (tdep)
> +	   && r < I387_YMMH_AVX512_END_REGNUM (tdep))
> +    {
> +      context_offset = (char *) locate_xstate_feature
> +	(context, X86_XSTATE_ZMM_ID, NULL);
> +      context_offset += 16 + 64 * (r - I387_YMM16H_REGNUM (tdep));
> +    }
> +  else if (I387_XMM16_REGNUM (tdep) > 0 && r >= I387_XMM16_REGNUM
> (tdep)
> +	   && r < I387_XMM_AVX512_END_REGNUM (tdep))
> +    {
> +      context_offset = (char *) locate_xstate_feature
> +	(context, X86_XSTATE_ZMM_ID, NULL);
> +      context_offset += 64 * (r - I387_XMM16_REGNUM (tdep));
> +    }
>    else
>      gdb_assert_not_reached ("invalid register number %d", r);
> 
> diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
> index a7e83c0239c..6911587f977 100644
> --- a/gdbserver/win32-i386-low.cc
> +++ b/gdbserver/win32-i386-low.cc
> @@ -529,8 +529,11 @@ get_context_reg_ptr (Context *context, int r, const
> target_desc *tdesc)
>      }
> 
>    bool amd64 = register_size (tdesc, 0) == 8;
> -  int ymm0h_regnum;
> +  int ymm0h_regnum, zmm0h_regnum, k0_regnum;
> +  int xmm16_regnum, ymm16h_regnum, zmm16h_regnum;
>    const int num_xmm_registers = amd64 ? 16 : 8;
> +  const int num_zmm_high_registers = amd64 ? 16 : 0;
> +  const int num_avx512_k_registers = 8;
> 
>    char *context_offset;
>    if (r < mappings_count)
> @@ -543,6 +546,49 @@ get_context_reg_ptr (Context *context, int r, const
> target_desc *tdesc)
>  	(context, X86_XSTATE_AVX_ID, NULL);
>        context_offset += 16 * (r - ymm0h_regnum);
>      }
> +  else if ((xstate_features & X86_XSTATE_ZMM_H) != 0
> +	   && r >= (zmm0h_regnum = find_regno (tdesc, "zmm0h"))
> +	   && r < zmm0h_regnum + num_xmm_registers)
> +    {
> +      context_offset = (char *) locate_xstate_feature
> +	(context, X86_XSTATE_ZMM_H_ID, NULL);
> +      context_offset += 32 * (r - zmm0h_regnum);
> +    }
> +  else if ((xstate_features & X86_XSTATE_ZMM) != 0
> +	   && num_zmm_high_registers != 0
> +	   && r >= (zmm16h_regnum = find_regno (tdesc, "zmm16h"))
> +	   && r < zmm16h_regnum + num_zmm_high_registers)
> +    {
> +      context_offset = (char *) locate_xstate_feature
> +	(context, X86_XSTATE_ZMM_ID, NULL);
> +      context_offset += 32 + 64 * (r - zmm16h_regnum);
> +    }
> +  else if ((xstate_features & X86_XSTATE_K) != 0
> +	   && r >= (k0_regnum = find_regno (tdesc, "k0"))
> +	   && r < k0_regnum + num_avx512_k_registers)
> +    {
> +      context_offset = (char *) locate_xstate_feature
> +	(context, X86_XSTATE_K_ID, NULL);
> +      context_offset += 8 * (r - k0_regnum);
> +    }
> +  else if ((xstate_features & X86_XSTATE_ZMM) != 0
> +	   && num_zmm_high_registers != 0
> +	   && r >= (ymm16h_regnum = find_regno (tdesc, "ymm16h"))
> +	   && r < ymm16h_regnum + num_zmm_high_registers)
> +    {
> +      context_offset = (char *) locate_xstate_feature
> +	(context, X86_XSTATE_ZMM_ID, NULL);
> +      context_offset += 16 + 64 * (r - ymm16h_regnum);
> +    }
> +  else if ((xstate_features & X86_XSTATE_ZMM) != 0
> +	   && num_zmm_high_registers != 0
> +	   && r >= (xmm16_regnum = find_regno (tdesc, "xmm16"))
> +	   && r < xmm16_regnum + num_zmm_high_registers)
> +    {
> +      context_offset = (char *) locate_xstate_feature
> +	(context, X86_XSTATE_ZMM_ID, NULL);
> +      context_offset += 64 * (r - xmm16_regnum);
> +    }
>    else
>      gdb_assert_not_reached ("invalid register number %d", r);
> 

These are a lot of look-ups on gdbserver side.  Each "find_regno" iterates over
the complete set of registers and does string comparison.  It would be nice to
implement this like the GDB side and cache the register numbers.

I think it is not mandatory to implement this but would be nice to have.


> --
> 2.54.0

________________________________________
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.