Re: [PATCH v4 8/9] Windows gdb: Implement AVX-512 register support

[Hannes Domani <ssbssa@yahoo.de>]

 Am Montag, 5. Oktober 2026 um 16:09:19 MESZ hat Rohr, Stephan <stephan.rohr@intel.com> Folgendes geschrieben:

> Hi Hanns,
> 
> also here, some minor findings below.
> 
> Thanks
> Stephan
> 
> > -----Original Message-----
> > From: Hannes Domani <ssbssa@yahoo.de>
> > Sent: Wednesday, 30 September 2026 17:12
> > To: gdb-patches@sourceware.org
> > Subject: [PATCH v4 8/9] Windows gdb: Implement AVX-512 register support
> >
> > Similar to commit 01f9f808e2e86187c95e7cff4aeb014a421a53ce for Linux,
> 
> Maybe name the commit title instead of the hash. Not sure what is best
> practice for this.

This part was suggested to me in another review, so maybe I should just
add both commit hash and title.


> > this patch adds support for the Intel AVX-512 registers for 32 and 64-bit
> > mode on Windows.
> > It enables accessing registers $ymm0 - $ymm31, $zmm0 - $zmm31, and
> > $k0 - $k7 where they are available.
> >
> 
> Wouldn't we want to mention "It enables accessing registers
> $ymm16 - $ymm31 [..]" instead?

Ok.


> > After this patch gdb.arch/i386-avx512.exp passes on windows.
> > ---
> > v3:
> > - merged gdb+gdbserver parts, and split again AVX/AVX-512 parts
> > v4:
> > - Add reference to equivalent Linux commit in commit message
> > - Add check for enabled xstate_features in get_context_reg_ptr of
> > x86-windows-nat.c
> > - Changed all NULL to nullptr
> > ---
> > gdb/NEWS | 4 +++-
> > gdb/nat/windows-nat.c | 4 ++--
> > gdb/x86-windows-nat.c | 40 +++++++++++++++++++++++++++++++
> > gdbserver/win32-i386-low.cc | 48
> > ++++++++++++++++++++++++++++++++++++-
> > 4 files changed, 92 insertions(+), 4 deletions(-)
> >
> > diff --git a/gdb/NEWS b/gdb/NEWS
> > index da4bc811c3a..704e6a7c8ba 100644
> > --- a/gdb/NEWS
> > +++ b/gdb/NEWS
> > @@ -195,7 +195,9 @@ info locals
> > The s390 64-bit target (s390x-*) remains supported.
> >
> > * Support for Intel AVX registers on Windows.
> > - Support displaying and modifying Intel AVX registers $ymm0 - $ymm15.
> > + Support displaying and modifying Intel AVX registers $ymm0 - $ymm31.
> 
> AVX is limited to $ymm0 - $ymm15. Maybe add another bullet
> "Support for Intel AVX-512 registers on Windows." and list what's introduced
> in this patch.

Ok.


> > + 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 cd3c827a5d5..3940465d06b 100644
> > --- a/gdb/nat/windows-nat.c
> > +++ b/gdb/nat/windows-nat.c
> > @@ -290,7 +290,7 @@ windows_thread_info::zero_xstate_features ()
> > if (zeroed_features == 0)
> > return;
> >
> > - for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_AVX_ID; f++)
> > + for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_ZMM_ID; f++)
> > {
> > DWORD64 flag = 1ULL << f;
> > if ((zeroed_features & flag) != 0)
> > @@ -1373,7 +1373,7 @@ initialize_loadable ()
> > {
> > /* Available XState features masked with implemented features. */
> > xstate_features = (GetEnabledXStateFeatures ()
> > - & X86_XSTATE_AVX_MASK);
> > + & X86_XSTATE_AVX_AVX512_MASK);
> 
> I think it makes sense to add a check that bits X86_XSTATE_K, X86_XSTATE_ZMM_H
> and X86_XSTATE_ZMM are all set. x86-xstate.h explicitly mentions
> "AVX 512 adds three feature bits. All three must be enabled.". I know this is a
> theoretical example but it doesn't hurt, e.g.,
> 
> if ((xstate_features & X86_XSTATE_AVX512) != X86_XSTATE_AVX512)
> xstate_features &= ~X86_XSTATE_AVX512;

I don't really get the why, but ok.


> > /* 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 1e44a45171c..afc71b4ef61 100644
> > --- a/gdb/x86-windows-nat.c
> > +++ b/gdb/x86-windows-nat.c
> > @@ -275,6 +275,46 @@ get_context_reg_ptr (Context *context, int r,
> > i386_gdbarch_tdep *tdep)
> > (context, X86_XSTATE_AVX_ID, nullptr);
> > context_offset += 16 * (r - I387_YMM0H_REGNUM (tdep));
> > }
> > + else if ((xstate_features & X86_XSTATE_ZMM_H) != 0
> > + && I387_ZMM0H_REGNUM (tdep) > 0 && r >=
> > I387_ZMM0H_REGNUM (tdep)
> > + && r < I387_ZMM16H_REGNUM (tdep) && r <
> > I387_ZMMENDH_REGNUM (tdep))
> > + {
> > + context_offset = (char *) locate_xstate_feature
> > + (context, X86_XSTATE_ZMM_H_ID, nullptr);
> > + context_offset += 32 * (r - I387_ZMM0H_REGNUM (tdep));
> > + }
> > + else if ((xstate_features & X86_XSTATE_ZMM) != 0
> > + && 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, nullptr);
> > + context_offset += 32 + 64 * (r - I387_ZMM16H_REGNUM (tdep));
> > + }
> > + else if ((xstate_features & X86_XSTATE_K) != 0
> > + && 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, nullptr);
> > + context_offset += 8 * (r - I387_K0_REGNUM (tdep));
> > + }
> > + else if ((xstate_features & X86_XSTATE_ZMM) != 0
> > + && 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, nullptr);
> > + context_offset += 16 + 64 * (r - I387_YMM16H_REGNUM (tdep));
> > + }
> > + else if ((xstate_features & X86_XSTATE_ZMM) != 0
> > + && 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, nullptr);
> > + 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 1a45b15e0cf..8c62787a31b 100644
> > --- a/gdbserver/win32-i386-low.cc
> > +++ b/gdbserver/win32-i386-low.cc
> > @@ -515,8 +515,11 @@ get_context_reg_ptr (Context *context, int r, const
> > target_desc *tdesc)
> > amd64 = false;
> > }
> >
> > - 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)
> > @@ -529,6 +532,49 @@ get_context_reg_ptr (Context *context, int r, const
> > target_desc *tdesc)
> > (context, X86_XSTATE_AVX_ID, nullptr);
> > 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, nullptr);
> > + 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, nullptr);
> > + 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, nullptr);
> > + 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, nullptr);
> > + 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, nullptr);
> > + context_offset += 64 * (r - xmm16_regnum);
> > + }
> > else
> > gdb_assert_not_reached ("invalid register number %d", r);
> >
> > --
> > 2.55.0


Hannes