[PATCH v3 1/8] gdb/testsuite: Add Windows replacement for aligned_alloc

[PATCH v3 1/8] gdb/testsuite: Add Windows replacement for aligned_alloc

[Hannes Domani]

_aligned_malloc has swapped arguments, and you have to use _aligned_free
to free the memory afterwards.

This is enough to make i386-sse.exp pass, and i386-avx.exp will pass at
the end of this series.

Approved-By: Tom Tromey <tom@tromey.com>
---
v2:
  - Mention which test pass after the change in the commit message
v3: no changes
---
 gdb/testsuite/gdb.arch/i386-avx.c         |  2 +-
 gdb/testsuite/gdb.arch/i386-sse.c         |  2 +-
 gdb/testsuite/lib/precise-aligned-alloc.c | 20 ++++++++++++++++++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.arch/i386-avx.c b/gdb/testsuite/gdb.arch/i386-avx.c
index b6cd89e5bea..229093e5164 100644
--- a/gdb/testsuite/gdb.arch/i386-avx.c
+++ b/gdb/testsuite/gdb.arch/i386-avx.c
@@ -116,7 +116,7 @@ main (int argc, char **argv)
 
   puts ("Bye!"); /* second breakpoint here */
 
-  free (allocated_ptr);
+  aligned_free (allocated_ptr);
 
   return 0;
 }
diff --git a/gdb/testsuite/gdb.arch/i386-sse.c b/gdb/testsuite/gdb.arch/i386-sse.c
index 21264ed74e7..5210aee12c3 100644
--- a/gdb/testsuite/gdb.arch/i386-sse.c
+++ b/gdb/testsuite/gdb.arch/i386-sse.c
@@ -134,7 +134,7 @@ main (int argc, char **argv)
       puts ("Bye!"); /* second breakpoint here */
     }
 
-  free (allocated_ptr);
+  aligned_free (allocated_ptr);
 
   return 0;
 }
diff --git a/gdb/testsuite/lib/precise-aligned-alloc.c b/gdb/testsuite/lib/precise-aligned-alloc.c
index 888814f12ef..1db35528bc8 100644
--- a/gdb/testsuite/lib/precise-aligned-alloc.c
+++ b/gdb/testsuite/lib/precise-aligned-alloc.c
@@ -21,6 +21,26 @@
 #include <string.h>
 #include <stdint.h>
 
+#ifdef _WIN32
+static void *
+aligned_alloc (size_t alignment, size_t size)
+{
+  return _aligned_malloc (size, alignment);
+}
+
+static void
+aligned_free (void *ptr)
+{
+  _aligned_free (ptr);
+}
+#else
+static void
+aligned_free (void *ptr)
+{
+  free (ptr);
+}
+#endif
+
 /* Return true if address P is ALIGNMENT-byte aligned.  */
 
 static int
-- 
2.54.0

[PATCH v3 2/8] Windows gdb: Use allocated buffer for CONTEXT

[Hannes Domani]

This is done in preparation for the XState functions, because the
extended registers are stored directly after the CONTEXT, and its actual
size depends on the available XState features.
---
v3:
  - fixed missing newline
  - merged initialize_context into the windows_process_info constructor
---
 gdb/aarch64-windows-nat.c      | 16 ++++++++--------
 gdb/nat/windows-nat.c          | 22 ++++++++++++++++++++++
 gdb/nat/windows-nat.h          | 20 +++++++++-----------
 gdbserver/win32-aarch64-low.cc | 10 +++++-----
 4 files changed, 44 insertions(+), 24 deletions(-)

diff --git a/gdb/aarch64-windows-nat.c b/gdb/aarch64-windows-nat.c
index ff2c9762467..630704a8caf 100644
--- a/gdb/aarch64-windows-nat.c
+++ b/gdb/aarch64-windows-nat.c
@@ -185,7 +185,7 @@ aarch64_windows_nat_target::cleanup_windows_arch ()
 void
 aarch64_windows_per_inferior::fill_thread_context (windows_thread_info *th)
 {
-  CONTEXT *context = &th->context;
+  CONTEXT *context = th->context;
 
   if (context->ContextFlags == 0)
     {
@@ -199,7 +199,7 @@ aarch64_windows_per_inferior::fill_thread_context (windows_thread_info *th)
 void
 aarch64_windows_per_inferior::invalidate_thread_context (windows_thread_info *th)
 {
-  CONTEXT *context = &th->context;
+  CONTEXT *context = th->context;
   context->ContextFlags = 0;
 }
 
@@ -209,7 +209,7 @@ void
 aarch64_windows_nat_target::thread_context_continue (windows_thread_info *th,
 						     int killed)
 {
-  CONTEXT *context = &th->context;
+  CONTEXT *context = th->context;
 
   if (th->debug_registers_changed)
     {
@@ -250,9 +250,9 @@ aarch64_windows_nat_target::thread_context_step (windows_thread_info *th,
 						 bool enable)
 {
   if (enable)
-    th->context.Cpsr |= 0x200000;
+    th->context->Cpsr |= 0x200000;
   else
-    th->context.Cpsr &= ~0x200000;
+    th->context->Cpsr &= ~0x200000;
 }
 
 /* See windows-nat.h.  */
@@ -263,7 +263,7 @@ aarch64_windows_nat_target::fetch_one_register (struct regcache *regcache,
 {
   gdb_assert (r >= 0);
 
-  char *context_ptr = (char *) &th->context;
+  char *context_ptr = (char *) th->context;
   char *context_offset = context_ptr + aarch64_windows_process.mappings[r];
   struct gdbarch *gdbarch = regcache->arch ();
 
@@ -292,9 +292,9 @@ aarch64_windows_nat_target::store_one_register (const struct regcache *regcache,
 						windows_thread_info *th, int r)
 {
   gdb_assert (r >= 0);
-  gdb_assert (th->context.ContextFlags != 0);
+  gdb_assert (th->context->ContextFlags != 0);
 
-  char *context_ptr = (char *) &th->context;
+  char *context_ptr = (char *) th->context;
 
   regcache->raw_collect (r, context_ptr + aarch64_windows_process.mappings[r]);
 }
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index e975892f487..974b1d3535b 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -79,6 +79,28 @@ DeleteProcThreadAttributeList_ftype *DeleteProcThreadAttributeList;
   debug_prefixed_printf_cond (debug_events, "windows events", fmt, \
 			      ## __VA_ARGS__)
 
+windows_thread_info::windows_thread_info (windows_process_info *proc_,
+					  DWORD tid_, HANDLE h_, CORE_ADDR tlb)
+  : proc (proc_),
+    tid (tid_),
+    h (h_),
+    thread_local_base (tlb)
+{
+#ifdef __x86_64__
+  if (proc->wow64_process)
+    {
+      context_buffer.reset (xmalloc (sizeof (WOW64_CONTEXT)));
+      wow64_context = (WOW64_CONTEXT *) context_buffer.get ();
+    }
+  else
+#endif
+    {
+      context_buffer.reset (xmalloc (sizeof (CONTEXT)));
+      context = (CONTEXT *) context_buffer.get ();
+    }
+  *proc->context_flags_ptr (this) = 0;
+}
+
 void
 windows_thread_info::suspend ()
 {
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 44b628c9bd1..ee85cca2984 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -63,13 +63,7 @@ enum stopping_kind
 struct windows_thread_info
 {
   windows_thread_info (windows_process_info *proc_,
-		       DWORD tid_, HANDLE h_, CORE_ADDR tlb)
-    : proc (proc_),
-      tid (tid_),
-      h (h_),
-      thread_local_base (tlb)
-  {
-  }
+		       DWORD tid_, HANDLE h_, CORE_ADDR tlb);
 
   DISABLE_COPY_AND_ASSIGN (windows_thread_info);
 
@@ -183,9 +177,9 @@ struct windows_thread_info
   /* The context of the thread, including any manipulations.  */
   union
   {
-    CONTEXT context {};
+    CONTEXT *context = nullptr;
 #ifdef __x86_64__
-    WOW64_CONTEXT wow64_context;
+    WOW64_CONTEXT *wow64_context;
 #endif
   };
 
@@ -204,6 +198,10 @@ struct windows_thread_info
 
   /* The name of the thread.  */
   gdb::unique_xmalloc_ptr<char> name;
+
+  /* The buffer for the thread context, including any XState registers if
+     available.  */
+  gdb::unique_xmalloc_ptr<void> context_buffer;
 };
 
 enum handle_exception_result
@@ -318,10 +316,10 @@ struct windows_process_info
   {
 #ifdef __x86_64__
     if (wow64_process)
-      return function (th != nullptr ? &th->wow64_context : nullptr);
+      return function (th != nullptr ? th->wow64_context : nullptr);
     else
 #endif
-      return function (th != nullptr ? &th->context : nullptr);
+      return function (th != nullptr ? th->context : nullptr);
   }
 
   DWORD *context_flags_ptr (windows_thread_info *th)
diff --git a/gdbserver/win32-aarch64-low.cc b/gdbserver/win32-aarch64-low.cc
index 5a7b72155db..e94548b6fef 100644
--- a/gdbserver/win32-aarch64-low.cc
+++ b/gdbserver/win32-aarch64-low.cc
@@ -169,7 +169,7 @@ aarch64_initial_stuff (process_info *proc)
 static void
 aarch64_get_thread_context (windows_thread_info *th)
 {
-  CONTEXT *context = &th->context;
+  CONTEXT *context = th->context;
 
   context->ContextFlags = (WindowsContext<decltype(context)>::full
 			   | WindowsContext<decltype(context)>::floating
@@ -193,7 +193,7 @@ aarch64_prepare_to_resume (windows_thread_info *th)
     {
       win32_require_context (th);
 
-      CONTEXT *context = &th->context;
+      CONTEXT *context = th->context;
 
       for (int i = 0; i < aarch64_num_bp_regs; i++)
 	{
@@ -223,7 +223,7 @@ aarch64_thread_added (windows_thread_info *th)
 static void
 aarch64_single_step (windows_thread_info *th)
 {
-  th->context.Cpsr |= 0x200000;
+  th->context->Cpsr |= 0x200000;
 }
 
 /* An array of offset mappings into a Win32 Context structure.
@@ -322,7 +322,7 @@ aarch64_fetch_inferior_register (struct regcache *regcache,
   int mappings_count;
   get_mappings (mappings, mappings_count);
 
-  char *context_ptr = (char *) &th->context;
+  char *context_ptr = (char *) th->context;
   char *context_offset;
   if (r < mappings_count)
     context_offset = context_ptr + mappings[r];
@@ -341,7 +341,7 @@ aarch64_store_inferior_register (struct regcache *regcache,
   int mappings_count;
   get_mappings (mappings, mappings_count);
 
-  char *context_ptr = (char *) &th->context;
+  char *context_ptr = (char *) th->context;
   char *context_offset;
   if (r < mappings_count)
     context_offset = context_ptr + mappings[r];
-- 
2.54.0

Re: [PATCH v3 2/8] Windows gdb: Use allocated buffer for CONTEXT

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:

Hannes> This is done in preparation for the XState functions, because the
Hannes> extended registers are stored directly after the CONTEXT, and its actual
Hannes> size depends on the available XState features.

Looks good, thanks.  I have one note but it's not really super
important.

Approved-By: Tom Tromey <tom@tromey.com>

Hannes> +windows_thread_info::windows_thread_info (windows_process_info *proc_,
Hannes> +					  DWORD tid_, HANDLE h_, CORE_ADDR tlb)
Hannes> +  : proc (proc_),
Hannes> +    tid (tid_),
Hannes> +    h (h_),
Hannes> +    thread_local_base (tlb)
Hannes> +{
Hannes> +#ifdef __x86_64__
Hannes> +  if (proc->wow64_process)
Hannes> +    {
Hannes> +      context_buffer.reset (xmalloc (sizeof (WOW64_CONTEXT)));
Hannes> +      wow64_context = (WOW64_CONTEXT *) context_buffer.get ();
Hannes> +    }
Hannes> +  else
Hannes> +#endif
Hannes> +    {
Hannes> +      context_buffer.reset (xmalloc (sizeof (CONTEXT)));
Hannes> +      context = (CONTEXT *) context_buffer.get ();
Hannes> +    }
Hannes> +  *proc->context_flags_ptr (this) = 0;

I guess this is because the memory isn't initialized?
You could use XCNEW instead of xmalloc to fix this.

Tom

Re: [PATCH v3 2/8] Windows gdb: Use allocated buffer for CONTEXT

[Hannes Domani]

 Am Dienstag, 1. September 2026 um 19:22:59 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
> 
> Hannes> This is done in preparation for the XState functions, because the
> Hannes> extended registers are stored directly after the CONTEXT, and its actual
> Hannes> size depends on the available XState features.
> 
> Looks good, thanks.  I have one note but it's not really super
> important.
> 
> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Hannes> +windows_thread_info::windows_thread_info (windows_process_info *proc_,
> Hannes> +                      DWORD tid_, HANDLE h_, CORE_ADDR tlb)
> Hannes> +  : proc (proc_),
> Hannes> +    tid (tid_),
> Hannes> +    h (h_),
> Hannes> +    thread_local_base (tlb)
> Hannes> +{
> Hannes> +#ifdef __x86_64__
> Hannes> +  if (proc->wow64_process)
> Hannes> +    {
> Hannes> +      context_buffer.reset (xmalloc (sizeof (WOW64_CONTEXT)));
> Hannes> +      wow64_context = (WOW64_CONTEXT *) context_buffer.get ();
> Hannes> +    }
> Hannes> +  else
> Hannes> +#endif
> Hannes> +    {
> Hannes> +      context_buffer.reset (xmalloc (sizeof (CONTEXT)));
> Hannes> +      context = (CONTEXT *) context_buffer.get ();
> Hannes> +    }
> Hannes> +  *proc->context_flags_ptr (this) = 0;
> 
> I guess this is because the memory isn't initialized?
> You could use XCNEW instead of xmalloc to fix this.

It's not just because of uninitialized memory from malloc.
Patch 6 then introduces InitializeContext, which actually initializes
context_flags to CONTEXT_ALL | CONTEXT_XSTATE, so it has to be reset
to 0 anyways.


Thanks
Hannes

[PATCH v3 3/8] Windows gdb: Remove mappings member from windows_per_inferior

[Hannes Domani]

It's not really that useful, and simplifies later XState changes.

Approved-By: Tom Tromey <tom@tromey.com>
---
v3: no changes
---
 gdb/aarch64-windows-nat.c |  6 ++----
 gdb/windows-nat.h         | 18 ------------------
 gdb/x86-windows-nat.c     | 34 ++++++++++++++++++++++------------
 3 files changed, 24 insertions(+), 34 deletions(-)

diff --git a/gdb/aarch64-windows-nat.c b/gdb/aarch64-windows-nat.c
index 630704a8caf..be900439047 100644
--- a/gdb/aarch64-windows-nat.c
+++ b/gdb/aarch64-windows-nat.c
@@ -168,8 +168,6 @@ aarch64_windows_nat_target::initialize_windows_arch (bool attaching)
 {
   memset (&aarch64_windows_process.dr_state, 0,
 	  sizeof (aarch64_windows_process.dr_state));
-
-  aarch64_windows_process.mappings = aarch64_mappings;
 }
 
 /* See windows-nat.h.  */
@@ -264,7 +262,7 @@ aarch64_windows_nat_target::fetch_one_register (struct regcache *regcache,
   gdb_assert (r >= 0);
 
   char *context_ptr = (char *) th->context;
-  char *context_offset = context_ptr + aarch64_windows_process.mappings[r];
+  char *context_offset = context_ptr + aarch64_mappings[r];
   struct gdbarch *gdbarch = regcache->arch ();
 
   gdb_assert (!gdbarch_read_pc_p (gdbarch));
@@ -296,7 +294,7 @@ aarch64_windows_nat_target::store_one_register (const struct regcache *regcache,
 
   char *context_ptr = (char *) th->context;
 
-  regcache->raw_collect (r, context_ptr + aarch64_windows_process.mappings[r]);
+  regcache->raw_collect (r, context_ptr + aarch64_mappings[r]);
 }
 
 /* See windows-nat.h.  */
diff --git a/gdb/windows-nat.h b/gdb/windows-nat.h
index 3349755022b..d6977ff4c85 100644
--- a/gdb/windows-nat.h
+++ b/gdb/windows-nat.h
@@ -139,24 +139,6 @@ struct windows_per_inferior : public windows_nat::windows_process_info
   void *wow64_dbgbreak = nullptr;
 #endif
 
-  /* This vector maps GDB's idea of a register's number into an offset
-     in the windows exception context vector.
-
-     It also contains the bit mask needed to load the register in question.
-
-     The contents of this table can only be computed by the units
-     that provide CPU-specific support for Windows native debugging.
-
-     One day we could read a reg, we could inspect the context we
-     already have loaded, if it doesn't have the bit set that we need,
-     we read that set of registers in using GetThreadContext.  If the
-     context already contains what we need, we just unpack it.  Then to
-     write a register, first we have to ensure that the context contains
-     the other regs of the group, and then we copy the info in and set
-     out bit.  */
-
-  const int *mappings = nullptr;
-
   std::vector<windows_solib> solibs;
 
 #ifdef __CYGWIN__
diff --git a/gdb/x86-windows-nat.c b/gdb/x86-windows-nat.c
index 2f556e47b39..b3ad8594020 100644
--- a/gdb/x86-windows-nat.c
+++ b/gdb/x86-windows-nat.c
@@ -85,16 +85,10 @@ x86_windows_nat_target::initialize_windows_arch (bool attaching)
     = !attaching && x86_windows_process.wow64_process;
 
   if (!x86_windows_process.wow64_process)
-    {
-      x86_windows_process.mappings = amd64_mappings;
-      x86_windows_process.segment_register_p = amd64_windows_segment_register_p;
-    }
+    x86_windows_process.segment_register_p = amd64_windows_segment_register_p;
   else
 #endif
-    {
-      x86_windows_process.mappings = i386_mappings;
-      x86_windows_process.segment_register_p = i386_windows_segment_register_p;
-    }
+    x86_windows_process.segment_register_p = i386_windows_segment_register_p;
 }
 
 /* See windows-nat.h.  */
@@ -242,7 +236,15 @@ x86_windows_nat_target::fetch_one_register (struct regcache *regcache,
       return (char *) context;
     });
 
-  char *context_offset = context_ptr + x86_windows_process.mappings[r];
+  const int *mappings;
+#ifdef __x86_64__
+  if (!x86_windows_process.wow64_process)
+    mappings = amd64_mappings;
+  else
+#endif
+    mappings = i386_mappings;
+
+  char *context_offset = context_ptr + mappings[r];
   struct gdbarch *gdbarch = regcache->arch ();
   i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep> (gdbarch);
 
@@ -308,6 +310,14 @@ x86_windows_nat_target::store_one_register (const struct regcache *regcache,
       return (char *) context;
     });
 
+  const int *mappings;
+#ifdef __x86_64__
+  if (!x86_windows_process.wow64_process)
+    mappings = amd64_mappings;
+  else
+#endif
+    mappings = i386_mappings;
+
   struct gdbarch *gdbarch = regcache->arch ();
   i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep> (gdbarch);
 
@@ -319,7 +329,7 @@ x86_windows_nat_target::store_one_register (const struct regcache *regcache,
     {
       gdb_byte bytes[4];
       regcache->raw_collect (r, bytes);
-      memcpy (context_ptr + x86_windows_process.mappings[r], bytes, 2);
+      memcpy (context_ptr + mappings[r], bytes, 2);
     }
   else if (r == I387_FOP_REGNUM (tdep))
     {
@@ -328,10 +338,10 @@ x86_windows_nat_target::store_one_register (const struct regcache *regcache,
       /* The value of FOP occupies the top two bytes in the context,
 	 so write the two low-order bytes from the cache into the
 	 appropriate spot.  */
-      memcpy (context_ptr + x86_windows_process.mappings[r] + 2, bytes, 2);
+      memcpy (context_ptr + mappings[r] + 2, bytes, 2);
     }
   else
-    regcache->raw_collect (r, context_ptr + x86_windows_process.mappings[r]);
+    regcache->raw_collect (r, context_ptr + mappings[r]);
 }
 
 /* See windows-nat.h.  */
-- 
2.54.0

[PATCH v3 4/8] Windows gdb: Refactor getting pointer to register inside context

[Hannes Domani]

These get_context_reg_ptr helper functions will be extended for XState
registers later.

Approved-By: Tom Tromey <tom@tromey.com>
---
v3: no changes
---
 gdb/amd64-windows-nat.c     |  3 ++
 gdb/i386-windows-nat.c      |  3 ++
 gdb/windows-nat.h           |  6 +++
 gdb/x86-windows-nat.c       | 75 ++++++++++++++++++++++---------------
 gdbserver/win32-i386-low.cc | 46 +++++++++++++++--------
 5 files changed, 87 insertions(+), 46 deletions(-)

diff --git a/gdb/amd64-windows-nat.c b/gdb/amd64-windows-nat.c
index cca606b5200..9d83a5f8334 100644
--- a/gdb/amd64-windows-nat.c
+++ b/gdb/amd64-windows-nat.c
@@ -86,6 +86,9 @@ const int amd64_mappings[] =
 };
 #undef context_offset
 
+const int amd64_mappings_count
+  = sizeof (amd64_mappings) / sizeof (amd64_mappings[0]);
+
 /* segment_register_p_ftype implementation for amd64.  */
 
 int
diff --git a/gdb/i386-windows-nat.c b/gdb/i386-windows-nat.c
index 991f301cad1..151b778036c 100644
--- a/gdb/i386-windows-nat.c
+++ b/gdb/i386-windows-nat.c
@@ -74,6 +74,9 @@ const int i386_mappings[] =
 #undef context_offset
 #undef CONTEXT
 
+const int i386_mappings_count
+  = sizeof (i386_mappings) / sizeof (i386_mappings[0]);
+
 /* segment_register_p_ftype implementation for x86.  */
 
 int
diff --git a/gdb/windows-nat.h b/gdb/windows-nat.h
index d6977ff4c85..69be069e419 100644
--- a/gdb/windows-nat.h
+++ b/gdb/windows-nat.h
@@ -392,12 +392,18 @@ int i386_windows_segment_register_p (int regnum);
 /* context register offsets for x86.  */
 extern const int i386_mappings[];
 
+/* number of context register offests for x86.  */
+extern const int i386_mappings_count;
+
 #ifdef __x86_64__
 /* segment_register_p_ftype implementation for amd64.  */
 int amd64_windows_segment_register_p (int regnum);
 
 /* context register offsets for amd64.  */
 extern const int amd64_mappings[];
+
+/* number of context register offests for amd64.  */
+extern const int amd64_mappings_count;
 #endif
 
 /* Creates an iterator that works like all_matching_threads_iterator,
diff --git a/gdb/x86-windows-nat.c b/gdb/x86-windows-nat.c
index b3ad8594020..3af5ef4dae0 100644
--- a/gdb/x86-windows-nat.c
+++ b/gdb/x86-windows-nat.c
@@ -223,31 +223,53 @@ x86_windows_nat_target::thread_context_step (windows_thread_info *th,
     });
 }
 
-/* See windows-nat.h.  */
+/* Get pointer to register R inside CONTEXT.  */
 
-void
-x86_windows_nat_target::fetch_one_register (struct regcache *regcache,
-					    windows_thread_info *th, int r)
+template<typename Context>
+static char *
+get_context_reg_ptr (Context *context, int r)
 {
-  gdb_assert (r >= 0);
-
-  char *context_ptr = x86_windows_process.with_context (th, [] (auto *context)
-    {
-      return (char *) context;
-    });
-
   const int *mappings;
+  int mappings_count;
 #ifdef __x86_64__
   if (!x86_windows_process.wow64_process)
-    mappings = amd64_mappings;
+    {
+      mappings = amd64_mappings;
+      mappings_count = amd64_mappings_count;
+    }
   else
 #endif
-    mappings = i386_mappings;
+    {
+      mappings = i386_mappings;
+      mappings_count = i386_mappings_count;
+    }
+
+  char *context_offset;
+  if (r < mappings_count)
+    context_offset = (char *) context + mappings[r];
+  else
+    gdb_assert_not_reached ("invalid register number %d", r);
+
+  return context_offset;
+}
+
+/* See windows-nat.h.  */
+
+void
+x86_windows_nat_target::fetch_one_register (struct regcache *regcache,
+					    windows_thread_info *th, int r)
+{
+  gdb_assert (r >= 0);
 
-  char *context_offset = context_ptr + mappings[r];
   struct gdbarch *gdbarch = regcache->arch ();
   i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep> (gdbarch);
 
+  char *context_offset
+    = x86_windows_process.with_context (th, [&] (auto *context)
+    {
+      return get_context_reg_ptr (context, r);
+    });
+
   gdb_assert (!gdbarch_read_pc_p (gdbarch));
   gdb_assert (gdbarch_pc_regnum (gdbarch) >= 0);
   gdb_assert (!gdbarch_write_pc_p (gdbarch));
@@ -304,23 +326,16 @@ x86_windows_nat_target::store_one_register (const struct regcache *regcache,
 {
   gdb_assert (r >= 0);
 
-  char *context_ptr = x86_windows_process.with_context (th, [] (auto *context)
+  struct gdbarch *gdbarch = regcache->arch ();
+  i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep> (gdbarch);
+
+  char *context_offset
+    = x86_windows_process.with_context (th, [&] (auto *context)
     {
       gdb_assert (context->ContextFlags != 0);
-      return (char *) context;
+      return get_context_reg_ptr (context, r);
     });
 
-  const int *mappings;
-#ifdef __x86_64__
-  if (!x86_windows_process.wow64_process)
-    mappings = amd64_mappings;
-  else
-#endif
-    mappings = i386_mappings;
-
-  struct gdbarch *gdbarch = regcache->arch ();
-  i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep> (gdbarch);
-
   /* GDB treats some registers as 32-bit, where they are in fact only
      16 bits long.  These cases must be handled specially to avoid
      overwriting other registers in the context.  */
@@ -329,7 +344,7 @@ x86_windows_nat_target::store_one_register (const struct regcache *regcache,
     {
       gdb_byte bytes[4];
       regcache->raw_collect (r, bytes);
-      memcpy (context_ptr + mappings[r], bytes, 2);
+      memcpy (context_offset, bytes, 2);
     }
   else if (r == I387_FOP_REGNUM (tdep))
     {
@@ -338,10 +353,10 @@ x86_windows_nat_target::store_one_register (const struct regcache *regcache,
       /* The value of FOP occupies the top two bytes in the context,
 	 so write the two low-order bytes from the cache into the
 	 appropriate spot.  */
-      memcpy (context_ptr + mappings[r] + 2, bytes, 2);
+      memcpy (context_offset + 2, bytes, 2);
     }
   else
-    regcache->raw_collect (r, context_ptr + mappings[r]);
+    regcache->raw_collect (r, context_offset);
 }
 
 /* See windows-nat.h.  */
diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
index 1aacd302074..b77f6adc6ed 100644
--- a/gdbserver/win32-i386-low.cc
+++ b/gdbserver/win32-i386-low.cc
@@ -473,22 +473,44 @@ is_segment_register (int r)
     return r >= I386_CS_REGNUM && r <= I386_GS_REGNUM;
 }
 
-/* Fetch register from gdbserver regcache data.  */
-static void
-i386_fetch_inferior_register (struct regcache *regcache,
-			      windows_thread_info *th, int r)
+/* Get pointer to register R inside CONTEXT.  */
+
+template<typename Context>
+static char *
+get_context_reg_ptr (Context *context, int r)
 {
   const int *mappings;
+  int mappings_count;
 #ifdef __x86_64__
   if (!windows_process.wow64_process)
-    mappings = amd64_mappings;
+    {
+      mappings = amd64_mappings;
+      mappings_count = sizeof (amd64_mappings) / sizeof (amd64_mappings[0]);
+    }
   else
 #endif
-    mappings = i386_mappings;
+    {
+      mappings = i386_mappings;
+      mappings_count = sizeof (i386_mappings) / sizeof (i386_mappings[0]);
+    }
+
+  char *context_offset;
+  if (r < mappings_count)
+    context_offset = (char *) context + mappings[r];
+  else
+    gdb_assert_not_reached ("invalid register number %d", r);
+
+  return context_offset;
+}
 
+/* Fetch register from gdbserver regcache data.  */
+static void
+i386_fetch_inferior_register (struct regcache *regcache,
+			      windows_thread_info *th, int r)
+{
   char *context_offset = windows_process.with_context (th, [&] (auto *context)
     {
-      return (char *) context + mappings[r];
+      return get_context_reg_ptr (context, r);
     });
 
   /* GDB treats some registers as 32-bit, where they are in fact only
@@ -514,17 +536,9 @@ static void
 i386_store_inferior_register (struct regcache *regcache,
 			      windows_thread_info *th, int r)
 {
-  const int *mappings;
-#ifdef __x86_64__
-  if (!windows_process.wow64_process)
-    mappings = amd64_mappings;
-  else
-#endif
-    mappings = i386_mappings;
-
   char *context_offset = windows_process.with_context (th, [&] (auto *context)
     {
-      return (char *) context + mappings[r];
+      return get_context_reg_ptr (context, r);
     });
 
   /* GDB treats some registers as 32-bit, where they are in fact only
-- 
2.54.0

[PATCH v3 5/8] Windows gdb: Prepare XState functions

[Hannes Domani]

These functions will then be used to access the XState registers.

Approved-By: Tom Tromey <tom@tromey.com>
---
v3: no changes
---
 gdb/nat/windows-nat.c |  31 +++++++++++
 gdb/nat/windows-nat.h | 116 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 147 insertions(+)

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 974b1d3535b..8f91e755334 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -73,6 +73,19 @@ InitializeProcThreadAttributeList_ftype *InitializeProcThreadAttributeList;
 UpdateProcThreadAttribute_ftype *UpdateProcThreadAttribute;
 DeleteProcThreadAttributeList_ftype *DeleteProcThreadAttributeList;
 
+#if defined __i386__ || defined __x86_64__
+GetEnabledXStateFeatures_ftype *GetEnabledXStateFeatures;
+InitializeContext_ftype *InitializeContext;
+GetXStateFeaturesMask_ftype *GetXStateFeaturesMask;
+SetXStateFeaturesMask_ftype *SetXStateFeaturesMask;
+LocateXStateFeature_ftype *LocateXStateFeature;
+#ifdef __x86_64__
+RtlGetExtendedFeaturesMask_ftype *RtlGetExtendedFeaturesMask;
+RtlSetExtendedFeaturesMask_ftype *RtlSetExtendedFeaturesMask;
+RtlLocateExtendedFeature_ftype *RtlLocateExtendedFeature;
+#endif
+#endif
+
 /* Note that 'debug_events' must be locally defined in the relevant
    functions.  */
 #define DEBUG_EVENTS(fmt, ...) \
@@ -1194,6 +1207,14 @@ initialize_loadable ()
       GPA (hm, InitializeProcThreadAttributeList);
       GPA (hm, UpdateProcThreadAttribute);
       GPA (hm, DeleteProcThreadAttributeList);
+
+#if defined __i386__ || defined __x86_64__
+      GPA (hm, GetEnabledXStateFeatures);
+      GPA (hm, InitializeContext);
+      GPA (hm, GetXStateFeaturesMask);
+      GPA (hm, SetXStateFeaturesMask);
+      GPA (hm, LocateXStateFeature);
+#endif
     }
 
   /* Set variables to dummy versions of these processes if the function
@@ -1259,6 +1280,16 @@ initialize_loadable ()
 	GPA (hm, GetThreadDescription);
     }
 
+#ifdef __x86_64__
+  hm = LoadLibrary (TEXT ("ntdll.dll"));
+  if (hm)
+    {
+      GPA (hm, RtlGetExtendedFeaturesMask);
+      GPA (hm, RtlSetExtendedFeaturesMask);
+      GPA (hm, RtlLocateExtendedFeature);
+    }
+#endif
+
 #undef GPA
 
   return result;
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index ee85cca2984..53023859b9a 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -35,6 +35,9 @@
 #define CONTEXT_EXTENDED_REGISTERS 0
 #endif
 
+#define CONTEXT_EXTENDED_REGISTERS_FLAG	0x20
+#define CONTEXT_XSTATE_FLAG		0x40
+
 namespace windows_nat
 {
 
@@ -446,6 +449,14 @@ extern BOOL create_process (const wchar_t *image, wchar_t *command_line,
 #define InitializeProcThreadAttributeList dyn_InitializeProcThreadAttributeList
 #define UpdateProcThreadAttribute dyn_UpdateProcThreadAttribute
 #define DeleteProcThreadAttributeList dyn_DeleteProcThreadAttributeList
+#define GetEnabledXStateFeatures	dyn_GetEnabledXStateFeatures
+#define InitializeContext		dyn_InitializeContext
+#define GetXStateFeaturesMask		dyn_GetXStateFeaturesMask
+#define SetXStateFeaturesMask		dyn_SetXStateFeaturesMask
+#define LocateXStateFeature		dyn_LocateXStateFeature
+#define RtlGetExtendedFeaturesMask	dyn_RtlGetExtendedFeaturesMask
+#define RtlSetExtendedFeaturesMask	dyn_RtlSetExtendedFeaturesMask
+#define RtlLocateExtendedFeature	dyn_RtlLocateExtendedFeature
 
 typedef BOOL WINAPI (AdjustTokenPrivileges_ftype) (HANDLE, BOOL,
 						   PTOKEN_PRIVILEGES,
@@ -540,6 +551,35 @@ extern DeleteProcThreadAttributeList_ftype *DeleteProcThreadAttributeList;
 
 extern bool disable_randomization_available ();
 
+#if defined __i386__ || defined __x86_64__
+typedef DWORD64 (WINAPI GetEnabledXStateFeatures_ftype) ();
+extern GetEnabledXStateFeatures_ftype *GetEnabledXStateFeatures;
+
+typedef BOOL (WINAPI InitializeContext_ftype) (PVOID, DWORD,
+					       PCONTEXT*, PDWORD);
+extern InitializeContext_ftype *InitializeContext;
+
+typedef BOOL (WINAPI GetXStateFeaturesMask_ftype) (PCONTEXT, PDWORD64);
+extern GetXStateFeaturesMask_ftype *GetXStateFeaturesMask;
+
+typedef BOOL (WINAPI SetXStateFeaturesMask_ftype) (PCONTEXT, DWORD64);
+extern SetXStateFeaturesMask_ftype *SetXStateFeaturesMask;
+
+typedef PVOID (WINAPI LocateXStateFeature_ftype) (PCONTEXT, DWORD, PDWORD);
+extern LocateXStateFeature_ftype *LocateXStateFeature;
+
+#ifdef __x86_64__
+typedef DWORD64 (WINAPI RtlGetExtendedFeaturesMask_ftype) (PVOID);
+extern RtlGetExtendedFeaturesMask_ftype *RtlGetExtendedFeaturesMask;
+
+typedef VOID (WINAPI RtlSetExtendedFeaturesMask_ftype) (PVOID, DWORD64);
+extern RtlSetExtendedFeaturesMask_ftype *RtlSetExtendedFeaturesMask;
+
+typedef PVOID (WINAPI RtlLocateExtendedFeature_ftype) (PVOID, DWORD, PDWORD);
+extern RtlLocateExtendedFeature_ftype *RtlLocateExtendedFeature;
+#endif
+#endif
+
 /* Helper classes to get the correct ContextFlags values based on the
    used type (CONTEXT or WOW64_CONTEXT).  */
 
@@ -605,16 +645,69 @@ enum_process_modules (CONTEXT *, HANDLE process,
   return EnumProcessModules (process, modules, size, needed);
 }
 
+#if defined __i386__ || defined __x86_64__
+static inline BOOL
+get_xstate_features_mask (CONTEXT *context, DWORD64 *mask)
+{
+  return GetXStateFeaturesMask (context, mask);
+}
+
+static inline BOOL
+set_xstate_features_mask (CONTEXT *context, DWORD64 mask)
+{
+  return SetXStateFeaturesMask (context, mask);
+}
+
+static inline PVOID
+locate_xstate_feature (CONTEXT *context, DWORD feature, DWORD *length)
+{
+  return LocateXStateFeature (context, feature, length);
+}
+#endif
+
 #ifdef __x86_64__
 static inline BOOL
 get_thread_context (HANDLE h, WOW64_CONTEXT *context)
 {
+  if ((context->ContextFlags & CONTEXT_XSTATE_FLAG) != 0)
+    {
+      /* Wow64GetThreadContext doesn't handle CONTEXT_EXTENDED_REGISTERS and
+	 CONTEXT_XSTATE combined correctly, but separate they work fine.  */
+      DWORD flags = context->ContextFlags;
+      context->ContextFlags &= ~CONTEXT_EXTENDED_REGISTERS_FLAG;
+      BOOL ret = Wow64GetThreadContext (h, context);
+      context->ContextFlags = flags;
+      if (!ret)
+	return FALSE;
+
+      context->ContextFlags &= ~CONTEXT_XSTATE_FLAG;
+      ret = Wow64GetThreadContext (h, context);
+      context->ContextFlags = flags;
+      return ret;
+    }
+
   return Wow64GetThreadContext (h, context);
 }
 
 static inline BOOL
 set_thread_context (HANDLE h, WOW64_CONTEXT *context)
 {
+  if ((context->ContextFlags & CONTEXT_XSTATE_FLAG) != 0)
+    {
+      /* Same limitation as Wow64GetThreadContext above.  */
+      DWORD flags = context->ContextFlags;
+      context->ContextFlags &= ~CONTEXT_EXTENDED_REGISTERS_FLAG;
+      BOOL ret = Wow64SetThreadContext (h, context);
+      context->ContextFlags = flags;
+      if (!ret)
+	return FALSE;
+
+      context->ContextFlags &= ~CONTEXT_XSTATE_FLAG;
+      ret = Wow64SetThreadContext (h, context);
+      context->ContextFlags = flags;
+      return ret;
+    }
+
   return Wow64SetThreadContext (h, context);
 }
 
@@ -632,6 +725,29 @@ enum_process_modules (WOW64_CONTEXT *, HANDLE process,
   return EnumProcessModulesEx (process, modules, size, needed,
 			       LIST_MODULES_32BIT);
 }
+
+static inline BOOL
+get_xstate_features_mask (WOW64_CONTEXT *context, DWORD64 *mask)
+{
+  /* Use lower level function, since there is no Wow64GetXStateFeaturesMask.  */
+  *mask = RtlGetExtendedFeaturesMask (context + 1);
+  return TRUE;
+}
+
+static inline BOOL
+set_xstate_features_mask (WOW64_CONTEXT *context, DWORD64 mask)
+{
+  /* Use lower level function, since there is no Wow64SetXStateFeaturesMask.  */
+  RtlSetExtendedFeaturesMask (context + 1, mask);
+  return TRUE;
+}
+
+static inline PVOID
+locate_xstate_feature (WOW64_CONTEXT *context, DWORD feature, DWORD *length)
+{
+  /* Use lower level function, since there is no Wow64LocateXStateFeature.  */
+  return RtlLocateExtendedFeature (context + 1, feature, length);
+}
 #endif
 
 /* This is available starting with Windows 10.  */
-- 
2.54.0

[PATCH v3 6/8] Windows gdb: Get available XState features

[Hannes Domani]

Also prepares the thread context for the additional registers.
---
v2:
  - Remove PKRU from the implemented features mask
v3:
  - use throw_winerror_with_name instead of error
  - fix formatting and value of xstate_features
---
 gdb/nat/windows-nat.c | 57 ++++++++++++++++++++++++++++++++++++++++++-
 gdb/nat/windows-nat.h |  5 ++++
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 8f91e755334..8930536f3ba 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -20,6 +20,7 @@
 #include "gdbsupport/common-debug.h"
 #include "gdbsupport/gdb_signals.h"
 #include "gdbsupport/gdb_wait.h"
+#include "gdbsupport/x86-xstate.h"
 #include "target/target.h"
 
 #undef GetModuleFileNameEx
@@ -84,6 +85,8 @@ RtlGetExtendedFeaturesMask_ftype *RtlGetExtendedFeaturesMask;
 RtlSetExtendedFeaturesMask_ftype *RtlSetExtendedFeaturesMask;
 RtlLocateExtendedFeature_ftype *RtlLocateExtendedFeature;
 #endif
+
+DWORD64 xstate_features;
 #endif
 
 /* Note that 'debug_events' must be locally defined in the relevant
@@ -99,12 +102,41 @@ windows_thread_info::windows_thread_info (windows_process_info *proc_,
     h (h_),
     thread_local_base (tlb)
 {
+#if defined __i386__ || defined __x86_64__
+  if (xstate_features != 0)
+    {
+      DWORD context_flags = proc->with_context (nullptr, [] (auto *context)
+	{
+	  return WindowsContext<decltype(context)>::all;
+	});
+      context_flags |= CONTEXT_XSTATE_FLAG;
+      DWORD xstate_size = 0;
+      InitializeContext (NULL, context_flags, NULL, &xstate_size);
+      context_buffer.reset (xmalloc (xstate_size));
+      CONTEXT *c = nullptr;
+      if (!InitializeContext (context_buffer.get (),
+			      context_flags, &c, &xstate_size))
+	{
+	  unsigned err = (unsigned) GetLastError ();
+	  throw_winerror_with_name (_("InitializeContext failed"), err);
+	}
 #ifdef __x86_64__
-  if (proc->wow64_process)
+      /* InitializeContext actually initializes a WOW64_CONTEXT when
+	 context_flags contains a WOW64_CONTEXT_* value, so a cast is needed.
+	 */
+      if (proc->wow64_process)
+	wow64_context = (WOW64_CONTEXT *) c;
+      else
+#endif
+	context = c;
+    }
+#ifdef __x86_64__
+  else if (proc->wow64_process)
     {
       context_buffer.reset (xmalloc (sizeof (WOW64_CONTEXT)));
       wow64_context = (WOW64_CONTEXT *) context_buffer.get ();
     }
+#endif
   else
 #endif
     {
@@ -1292,6 +1324,29 @@ initialize_loadable ()
 
 #undef GPA
 
+#if defined __i386__ || defined __x86_64__
+  if (GetEnabledXStateFeatures != nullptr
+      && InitializeContext != nullptr
+      && GetXStateFeaturesMask != nullptr
+      && SetXStateFeaturesMask != nullptr
+      && LocateXStateFeature != nullptr
+#ifdef __x86_64__
+      && RtlGetExtendedFeaturesMask != nullptr
+      && RtlSetExtendedFeaturesMask != nullptr
+      && RtlLocateExtendedFeature != nullptr
+#endif
+  )
+    {
+      /* Available XState features masked with implemented features.  */
+      xstate_features = (GetEnabledXStateFeatures ()
+			 & X86_XSTATE_SSE_MASK);
+      /* The extended XState functions are only needed if the available
+	 features exceed SSE.  */
+      if ((xstate_features & ~X86_XSTATE_SSE_MASK) == 0)
+	xstate_features = 0;
+    }
+#endif
+
   return result;
 }
 
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 53023859b9a..f0a55d40f05 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -750,6 +750,11 @@ locate_xstate_feature (WOW64_CONTEXT *context, DWORD feature, DWORD *length)
 }
 #endif
 
+#if defined __i386__ || defined __x86_64__
+/* Available XState features.  */
+extern DWORD64 xstate_features;
+#endif
+
 /* This is available starting with Windows 10.  */
 #ifndef DBG_REPLY_LATER
 # define DBG_REPLY_LATER 0x40010001L
-- 
2.54.0

Re: [PATCH v3 6/8] Windows gdb: Get available XState features

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:

Hannes> Also prepares the thread context for the additional registers.
Hannes> ---
Hannes> v2:
Hannes>   - Remove PKRU from the implemented features mask
Hannes> v3:
Hannes>   - use throw_winerror_with_name instead of error
Hannes>   - fix formatting and value of xstate_features

Thanks.

Hannes> +      /* Available XState features masked with implemented features.  */
Hannes> +      xstate_features = (GetEnabledXStateFeatures ()
Hannes> +			 & X86_XSTATE_SSE_MASK);
Hannes> +      /* The extended XState functions are only needed if the available
Hannes> +	 features exceed SSE.  */
Hannes> +      if ((xstate_features & ~X86_XSTATE_SSE_MASK) == 0)
Hannes> +	xstate_features = 0;

Won't this condition always be true?
Resulting in xstate_features==0 always?

Tom

Re: [PATCH v3 6/8] Windows gdb: Get available XState features

[Hannes Domani]

 Am Dienstag, 1. September 2026 um 19:38:06 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
> 
> Hannes> Also prepares the thread context for the additional registers.
> Hannes> ---
> Hannes> v2:
> Hannes>  - Remove PKRU from the implemented features mask
> Hannes> v3:
> Hannes>  - use throw_winerror_with_name instead of error
> Hannes>  - fix formatting and value of xstate_features
> 
> Thanks.
> 
> Hannes> +      /* Available XState features masked with implemented features.  */
> Hannes> +      xstate_features = (GetEnabledXStateFeatures ()
> Hannes> +            & X86_XSTATE_SSE_MASK);
> Hannes> +      /* The extended XState functions are only needed if the available
> Hannes> +    features exceed SSE.  */
> Hannes> +      if ((xstate_features & ~X86_XSTATE_SSE_MASK) == 0)
> Hannes> +    xstate_features = 0;
> 
> Won't this condition always be true?
> Resulting in xstate_features==0 always?

Yes, with this patch xstate_features is still 0.
The next patch then changes the 'and' of the first xstate_features to use
X86_XSTATE_AVX_MASK instead, so it will be possible to have other values.


Hannes

Re: [PATCH v3 6/8] Windows gdb: Get available XState features

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:

Hannes> Yes, with this patch xstate_features is still 0.
Hannes> The next patch then changes the 'and' of the first xstate_features to use
Hannes> X86_XSTATE_AVX_MASK instead, so it will be possible to have other values.

Thanks.  Eventually I figured it out... normally I read the patches in
order; sometimes I look ahead but I didn't really think of it this time.

Anyway I think this patch is ok.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

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

[Hannes Domani]

This adds support for the Intel AVX registers on Windows.
It enables accessing registers $ymm0 - $ymm15 where they are available.

After this patch gdb.arch/i386-avx.exp passes on windows.
---
v3:
  - merged gdb+gdbserver parts, and split again AVX/AVX-512 parts
---
 gdb/NEWS                    |  3 ++
 gdb/nat/windows-nat.c       |  2 +-
 gdb/x86-windows-nat.c       | 67 +++++++++++++++++++++++++++++++++++--
 gdbserver/win32-i386-low.cc | 61 +++++++++++++++++++++++++++++----
 gdbserver/win32-low.cc      | 15 ++++++---
 5 files changed, 133 insertions(+), 15 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 10c182067f9..f7effc822e9 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -118,6 +118,9 @@
   intent to remove it in a future release.
   The s390 64-bit target (s390x-*) remains supported.
 
+* Support for Intel AVX registers on Windows.
+  Support displaying and modifying Intel AVX registers $ymm0 - $ymm31.
+
 * Configure changes
 
 ** --with-babeltrace has been removed.  The babeltrace library was
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 8930536f3ba..c9a21d7c41f 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_SSE_MASK);
+			 & X86_XSTATE_AVX_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 3af5ef4dae0..1cefe6171be 100644
--- a/gdb/x86-windows-nat.c
+++ b/gdb/x86-windows-nat.c
@@ -27,6 +27,9 @@
 
 #include "i386-tdep.h"
 #include "i387-tdep.h"
+#ifdef __x86_64__
+#include "amd64-tdep.h"
+#endif
 
 using namespace windows_nat;
 
@@ -70,6 +73,8 @@ struct x86_windows_nat_target final : public x86_nat_target<windows_nat_target>
 			   windows_thread_info *th, int r) override;
 
   bool is_sw_breakpoint (const EXCEPTION_RECORD *er) const override;
+
+  const struct target_desc *read_description () override;
 };
 
 /* The current process.  */
@@ -109,7 +114,31 @@ x86_windows_per_inferior::fill_thread_context (windows_thread_info *th)
       if (context->ContextFlags == 0)
 	{
 	  context->ContextFlags = WindowsContext<decltype(context)>::all;
+	  if (xstate_features != 0)
+	    {
+	      context->ContextFlags |= CONTEXT_XSTATE_FLAG;
+	      set_xstate_features_mask (context, xstate_features);
+	    }
 	  CHECK (get_thread_context (th->h, context));
+
+	  if (xstate_features != 0)
+	    {
+	      DWORD64 features = 0;
+	      CHECK (get_xstate_features_mask (context, &features));
+	      DWORD64 zeroed_features = xstate_features & ~features;
+
+	      for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_CET_U_ID; f++)
+		{
+		  DWORD64 flag = 1ULL << f;
+		  if ((zeroed_features & flag) != 0)
+		    {
+		      DWORD size = 0;
+		      void *loc = locate_xstate_feature (context, f, &size);
+		      if (loc != nullptr && size > 0)
+			memset (loc, 0, size);
+		    }
+		}
+	    }
 	}
     });
 }
@@ -198,6 +227,14 @@ x86_windows_nat_target::thread_context_continue (windows_thread_info *th,
 	  if (GetExitCodeThread (th->h, &ec)
 	      && ec == STILL_ACTIVE)
 	    {
+	      DWORD debug_registers = WindowsContext<decltype(context)>::debug;
+	      if (xstate_features != 0
+		  && (context->ContextFlags & ~debug_registers) != 0)
+		{
+		  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
+		  set_xstate_features_mask (context, xstate_features);
+		}
+
 	      BOOL status = set_thread_context (th->h, context);
 
 	      if (!killed)
@@ -227,7 +264,7 @@ x86_windows_nat_target::thread_context_step (windows_thread_info *th,
 
 template<typename Context>
 static char *
-get_context_reg_ptr (Context *context, int r)
+get_context_reg_ptr (Context *context, int r, i386_gdbarch_tdep *tdep)
 {
   const int *mappings;
   int mappings_count;
@@ -247,6 +284,13 @@ get_context_reg_ptr (Context *context, int r)
   char *context_offset;
   if (r < mappings_count)
     context_offset = (char *) context + mappings[r];
+  else if (I387_YMM0H_REGNUM (tdep) > 0 && r >= I387_YMM0H_REGNUM (tdep)
+	   && r < I387_YMMENDH_REGNUM (tdep))
+    {
+      context_offset = (char *) locate_xstate_feature
+	(context, X86_XSTATE_AVX_ID, NULL);
+      context_offset += 16 * (r - I387_YMM0H_REGNUM (tdep));
+    }
   else
     gdb_assert_not_reached ("invalid register number %d", r);
 
@@ -267,7 +311,7 @@ x86_windows_nat_target::fetch_one_register (struct regcache *regcache,
   char *context_offset
     = x86_windows_process.with_context (th, [&] (auto *context)
     {
-      return get_context_reg_ptr (context, r);
+      return get_context_reg_ptr (context, r, tdep);
     });
 
   gdb_assert (!gdbarch_read_pc_p (gdbarch));
@@ -333,7 +377,7 @@ x86_windows_nat_target::store_one_register (const struct regcache *regcache,
     = x86_windows_process.with_context (th, [&] (auto *context)
     {
       gdb_assert (context->ContextFlags != 0);
-      return get_context_reg_ptr (context, r);
+      return get_context_reg_ptr (context, r, tdep);
     });
 
   /* GDB treats some registers as 32-bit, where they are in fact only
@@ -368,6 +412,23 @@ x86_windows_nat_target::is_sw_breakpoint (const EXCEPTION_RECORD *er) const
 	  || er->ExceptionCode == STATUS_WX86_BREAKPOINT);
 }
 
+const struct target_desc *
+x86_windows_nat_target::read_description ()
+{
+  if (inferior_ptid == null_ptid)
+    return this->beneath ()->read_description ();
+
+  if (xstate_features == 0)
+    return nullptr;
+
+#ifdef __x86_64__
+  if (!x86_windows_process.wow64_process)
+    return amd64_target_description (xstate_features, false);
+  else
+#endif
+    return i386_target_description (xstate_features, false);
+}
+
 /* Hardware watchpoint support, adapted from go32-nat.c code.  */
 
 /* Pass the address ADDR to the inferior in the I'th debug register.
diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
index b77f6adc6ed..a7e83c0239c 100644
--- a/gdbserver/win32-i386-low.cc
+++ b/gdbserver/win32-i386-low.cc
@@ -253,6 +253,11 @@ i386_get_thread_context (windows_thread_info *th)
 			       | WindowsContext<decltype(context)>::floating
 			       | WindowsContext<decltype(context)>::debug
 			       | extended_registers);
+      if (xstate_features != 0)
+	{
+	  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
+	  set_xstate_features_mask (context, xstate_features);
+	}
 
       BOOL ret = get_thread_context (th->h, context);
       if (!ret)
@@ -267,6 +272,24 @@ i386_get_thread_context (windows_thread_info *th)
 
 	  error (_("GetThreadContext failure %ld\n"), (long) e);
 	}
+
+      DWORD64 features = 0;
+      if (xstate_features != 0
+	  && get_xstate_features_mask (context, &features))
+	{
+	  DWORD64 zeroed_features = xstate_features & ~features;
+	  for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_CET_U_ID; f++)
+	    {
+	      DWORD64 flag = 1ULL << f;
+	      if ((zeroed_features & flag) != 0)
+		{
+		  DWORD size = 0;
+		  void *loc = locate_xstate_feature (context, f, &size);
+		  if (loc != nullptr && size > 0)
+		    memset (loc, 0, size);
+		}
+	    }
+	}
     });
 }
 
@@ -292,6 +315,17 @@ i386_prepare_to_resume (windows_thread_info *th)
 
       th->debug_registers_changed = false;
     }
+
+  windows_process.with_context (th, [&] (auto *context)
+    {
+      DWORD debug_registers = WindowsContext<decltype(context)>::debug;
+      if (xstate_features != 0
+	  && (context->ContextFlags & ~debug_registers) != 0)
+	{
+	  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
+	  set_xstate_features_mask (context, xstate_features);
+	}
+    });
 }
 
 static void
@@ -477,7 +511,7 @@ is_segment_register (int r)
 
 template<typename Context>
 static char *
-get_context_reg_ptr (Context *context, int r)
+get_context_reg_ptr (Context *context, int r, const target_desc *tdesc)
 {
   const int *mappings;
   int mappings_count;
@@ -494,9 +528,21 @@ get_context_reg_ptr (Context *context, int r)
       mappings_count = sizeof (i386_mappings) / sizeof (i386_mappings[0]);
     }
 
+  bool amd64 = register_size (tdesc, 0) == 8;
+  int ymm0h_regnum;
+  const int num_xmm_registers = amd64 ? 16 : 8;
+
   char *context_offset;
   if (r < mappings_count)
     context_offset = (char *) context + mappings[r];
+  else if ((xstate_features & X86_XSTATE_AVX) != 0
+	   && r >= (ymm0h_regnum = find_regno (tdesc, "ymm0h"))
+	   && r < ymm0h_regnum + num_xmm_registers)
+    {
+      context_offset = (char *) locate_xstate_feature
+	(context, X86_XSTATE_AVX_ID, NULL);
+      context_offset += 16 * (r - ymm0h_regnum);
+    }
   else
     gdb_assert_not_reached ("invalid register number %d", r);
 
@@ -510,7 +556,7 @@ i386_fetch_inferior_register (struct regcache *regcache,
 {
   char *context_offset = windows_process.with_context (th, [&] (auto *context)
     {
-      return get_context_reg_ptr (context, r);
+      return get_context_reg_ptr (context, r, regcache->tdesc);
     });
 
   /* GDB treats some registers as 32-bit, where they are in fact only
@@ -538,7 +584,7 @@ i386_store_inferior_register (struct regcache *regcache,
 {
   char *context_offset = windows_process.with_context (th, [&] (auto *context)
     {
-      return get_context_reg_ptr (context, r);
+      return get_context_reg_ptr (context, r, regcache->tdesc);
     });
 
   /* GDB treats some registers as 32-bit, where they are in fact only
@@ -571,14 +617,17 @@ i386_arch_setup (void)
 {
  target_desc_up tdesc;
 
+  DWORD64 xcr0 = xstate_features;
+  if (xcr0 == 0)
+    xcr0 = X86_XSTATE_SSE_MASK;
+
 #ifdef __x86_64__
-  tdesc = amd64_create_target_description (X86_XSTATE_SSE_MASK, false,
-					   false, false);
+  tdesc = amd64_create_target_description (xcr0, false, false, false);
   init_target_desc (tdesc.get (), amd64_expedite_regs, WINDOWS_OSABI);
   win32_tdesc = std::move (tdesc);
 #endif
 
-  tdesc = i386_create_target_description (X86_XSTATE_SSE_MASK, false, false);
+  tdesc = i386_create_target_description (xcr0, false, false);
   init_target_desc (tdesc.get (), i386_expedite_regs, WINDOWS_OSABI);
 #ifdef __x86_64__
   wow64_win32_tdesc = std::move (tdesc);
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 7629beca213..5ccdc89a7ef 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -33,6 +33,7 @@
 #include <process.h>
 #include "gdbsupport/gdb_tilde_expand.h"
 #include "gdbsupport/common-inferior.h"
+#include "tdesc.h"
 
 using namespace windows_nat;
 
@@ -426,8 +427,9 @@ child_fetch_inferior_registers (struct regcache *regcache, int r)
   int regno;
   windows_thread_info *th = windows_process.find_thread (current_thread->id);
   win32_require_context (th);
-  if (r == -1 || r > NUM_REGS)
-    child_fetch_inferior_registers (regcache, NUM_REGS);
+  if (r == -1)
+    child_fetch_inferior_registers (regcache,
+				    regcache->tdesc->reg_defs.size ());
   else
     for (regno = 0; regno < r; regno++)
       (*the_low_target.fetch_inferior_register) (regcache, th, regno);
@@ -441,8 +443,9 @@ child_store_inferior_registers (struct regcache *regcache, int r)
   int regno;
   windows_thread_info *th = windows_process.find_thread (current_thread->id);
   win32_require_context (th);
-  if (r == -1 || r == 0 || r > NUM_REGS)
-    child_store_inferior_registers (regcache, NUM_REGS);
+  if (r == -1)
+    child_store_inferior_registers (regcache,
+				    regcache->tdesc->reg_defs.size ());
   else
     for (regno = 0; regno < r; regno++)
       (*the_low_target.store_inferior_register) (regcache, th, regno);
@@ -1349,7 +1352,9 @@ void
 initialize_low (void)
 {
   set_target_ops (&the_win32_target);
-  the_low_target.arch_setup ();
 
   initialize_loadable ();
+  /* Has to be done after initialize_loadable, because it uses the xstate
+     functions if available.  */
+  the_low_target.arch_setup ();
 }
-- 
2.54.0

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

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:

Hannes> This adds support for the Intel AVX registers on Windows.
Hannes> It enables accessing registers $ymm0 - $ymm15 where they are available.

Hannes> After this patch gdb.arch/i386-avx.exp passes on windows.

I'd like it if Christina would review this.

Hannes> @@ -1339,7 +1339,7 @@ initialize_loadable ()
Hannes>      {
Hannes>        /* Available XState features masked with implemented features.  */
Hannes>        xstate_features = (GetEnabledXStateFeatures ()
Hannes> -			 & X86_XSTATE_SSE_MASK);
Hannes> +			 & X86_XSTATE_AVX_MASK);
Hannes>        /* The extended XState functions are only needed if the available
Hannes>  	 features exceed SSE.  */
Hannes>        if ((xstate_features & ~X86_XSTATE_SSE_MASK) == 0)

Ok, ignore my earlier message for patch 6.  I see what's going on now.

I don't have any problem with this patch.  I'm not certain I totally
understand it, but that's ok.

One thing I did see while doing the background reading is that
supposedly the xstate feature flags are vendor-specific.  So does some
check of the vendor ID need to be done?  I do not know.

thanks,
Tom

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

[Hannes Domani]

 Am Dienstag, 1. September 2026 um 19:48:28 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
> 
> Hannes> This adds support for the Intel AVX registers on Windows.
> Hannes> It enables accessing registers $ymm0 - $ymm15 where they are available.
> 
> Hannes> After this patch gdb.arch/i386-avx.exp passes on windows.
> 
> I'd like it if Christina would review this.
> 
> Hannes> @@ -1339,7 +1339,7 @@ initialize_loadable ()
> Hannes>      {
> Hannes>        /* Available XState features masked with implemented features.  */
> Hannes>        xstate_features = (GetEnabledXStateFeatures ()
> Hannes> -            & X86_XSTATE_SSE_MASK);
> Hannes> +            & X86_XSTATE_AVX_MASK);
> Hannes>        /* The extended XState functions are only needed if the available
> Hannes>      features exceed SSE.  */
> Hannes>        if ((xstate_features & ~X86_XSTATE_SSE_MASK) == 0)
> 
> Ok, ignore my earlier message for patch 6.  I see what's going on now.
> 
> I don't have any problem with this patch.  I'm not certain I totally
> understand it, but that's ok.
> 
> One thing I did see while doing the background reading is that
> supposedly the xstate feature flags are vendor-specific.  So does some
> check of the vendor ID need to be done?  I do not know.

As I understand it (and I might be wrong about that), there are some
vendor-specific XState-bits, but stuff regarding AVX and AVX-512 are the
same everywhere.


Hannes

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

[Joos, Christina]

> -----Original Message-----
> From: Hannes Domani <ssbssa@yahoo.de>
> Sent: Dienstag, 1. September 2026 20:01
> To: Tom Tromey <tom@tromey.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v3 7/8] Windows gdb: Implement AVX register support
> 
>  Am Dienstag, 1. September 2026 um 19:48:28 MESZ hat Tom Tromey
> <tom@tromey.com> Folgendes geschrieben:
> 
> > >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
> >
> > Hannes> This adds support for the Intel AVX registers on Windows.
> > Hannes> It enables accessing registers $ymm0 - $ymm15 where they are
> available.
> >
> > Hannes> After this patch gdb.arch/i386-avx.exp passes on windows.
> >
> > I'd like it if Christina would review this.
> >
> > Hannes> @@ -1339,7 +1339,7 @@ initialize_loadable ()
> > Hannes>      {
> > Hannes>        /* Available XState features masked with implemented
> > Hannes>features.  */
> > Hannes>        xstate_features = (GetEnabledXStateFeatures ()
> > Hannes> -            & X86_XSTATE_SSE_MASK);
> > Hannes> +            & X86_XSTATE_AVX_MASK);
> > Hannes>        /* The extended XState functions are only needed if the
> > Hannes>available
> > Hannes>      features exceed SSE.  */
> > Hannes>        if ((xstate_features & ~X86_XSTATE_SSE_MASK) == 0)
> >
> > Ok, ignore my earlier message for patch 6.  I see what's going on now.
> >
> > I don't have any problem with this patch.  I'm not certain I totally
> > understand it, but that's ok.
> >
> > One thing I did see while doing the background reading is that
> > supposedly the xstate feature flags are vendor-specific.  So does some
> > check of the vendor ID need to be done?  I do not know.
> 
> As I understand it (and I might be wrong about that), there are some vendor-
> specific XState-bits, but stuff regarding AVX and AVX-512 are the same
> everywhere.

The MPX state, for instance, is not implemented by AMD (and has been removed
from GDB in the meantime, since the feature is deprecated).
For AVX and AVX-512 I can confirm that they are both implemented for AMD and Intel.
For CET I'd need to double check, but we decided to not add it anyways for now.
So using the masks as you do in the 2 patches should be fine.

Due to AMD not implementing MPX, there are some differences in the xsave
offsets for AMD and Intel CPUs, see the dedicated handling here:
- gdb/i387-tdep.c:i387_guess_xsave_layout: This is relevant for corefiles only.
- gdb/i386-tdep.c: i387_fallback_xsave_layout

Both functions are called at some point from i386_gdbarch_init.

I believe for live targets, since you don't hard-code the offsets in your patch series
but get them using LocateXStateFeature in the end, it should be safe.

However, we still call i386_gdbarch_init for windows, too.
So tdep->xsave_layout should still be available but is not consumed or configured.
This it at least what I observed when doing some basic debugging in windows, which I just started today.
At my current level of understanding, I don't think it makes sense to use it for Windows.

Reading 
"The LocateXStateFeature function must be used to find an individual XState feature within an extensible CONTEXT structure. Features are not necessarily contiguous in memory and applications should not assume the offset between two consecutive features will remain constant in the future."
https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-locatexstatefeature confirms that I think.

Corefiles:
I started some basis testing in windows and checked out my own patch series
https://sourceware.org/pipermail/gdb-patches/2026-September/230051.html
which extends avx and avx512 testing for corefiles together with your patches.

I see UNSUPPORTED for those new corefile tests and other existing corefile tests in gdb return similar results. So I believe this is ok for windows.

If you have any concerns with those test extensions (or my reasoning above), I'd be glad if you'd let me now.

I will provide some feedback on the entire patch in a separate email.

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.

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

[Joos, Christina]

> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Dienstag, 1. September 2026 19:48
> To: Hannes Domani <ssbssa@yahoo.de>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v3 7/8] Windows gdb: Implement AVX register support
> 
> >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
> 
> Hannes> This adds support for the Intel AVX registers on Windows.
> Hannes> It enables accessing registers $ymm0 - $ymm15 where they are
> available.
> 
> Hannes> After this patch gdb.arch/i386-avx.exp passes on windows.
> 
> I'd like it if Christina would review this.

Yes, so far, I've only been able to review it at a high level.
 
You can expect my feedback next week (this time hopefully for real 😉).
Sorry for the delay.

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.

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

[Rohr, Stephan]

Hi Hannes,

please see some feedback inlined below.

Let me know if you have any questions.

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 7/8] Windows gdb: Implement AVX register support
> 
> This adds support for the Intel AVX registers on Windows.
> It enables accessing registers $ymm0 - $ymm15 where they are available.
> 
> After this patch gdb.arch/i386-avx.exp passes on windows.
> ---
> v3:
>   - merged gdb+gdbserver parts, and split again AVX/AVX-512 parts
> ---
>  gdb/NEWS                    |  3 ++
>  gdb/nat/windows-nat.c       |  2 +-
>  gdb/x86-windows-nat.c       | 67
> +++++++++++++++++++++++++++++++++++--
>  gdbserver/win32-i386-low.cc | 61 +++++++++++++++++++++++++++++----
>  gdbserver/win32-low.cc      | 15 ++++++---
>  5 files changed, 133 insertions(+), 15 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 10c182067f9..f7effc822e9 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -118,6 +118,9 @@
>    intent to remove it in a future release.
>    The s390 64-bit target (s390x-*) remains supported.
> 
> +* Support for Intel AVX registers on Windows.
> +  Support displaying and modifying Intel AVX registers $ymm0 - $ymm31.
> +

I think this should be registers $ymm0 - $ymm15 ?

>  * Configure changes
> 
>  ** --with-babeltrace has been removed.  The babeltrace library was
> diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
> index 8930536f3ba..c9a21d7c41f 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_SSE_MASK);
> +			 & X86_XSTATE_AVX_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 3af5ef4dae0..1cefe6171be 100644
> --- a/gdb/x86-windows-nat.c
> +++ b/gdb/x86-windows-nat.c
> @@ -27,6 +27,9 @@
> 
>  #include "i386-tdep.h"
>  #include "i387-tdep.h"
> +#ifdef __x86_64__
> +#include "amd64-tdep.h"
> +#endif
> 
>  using namespace windows_nat;
> 
> @@ -70,6 +73,8 @@ struct x86_windows_nat_target final : public
> x86_nat_target<windows_nat_target>
>  			   windows_thread_info *th, int r) override;
> 
>    bool is_sw_breakpoint (const EXCEPTION_RECORD *er) const override;
> +
> +  const struct target_desc *read_description () override;
>  };
> 
>  /* The current process.  */
> @@ -109,7 +114,31 @@ x86_windows_per_inferior::fill_thread_context
> (windows_thread_info *th)
>        if (context->ContextFlags == 0)
>  	{
>  	  context->ContextFlags = WindowsContext<decltype(context)>::all;
> +	  if (xstate_features != 0)
> +	    {
> +	      context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> +	      set_xstate_features_mask (context, xstate_features);
> +	    }

We have the same code in "i386_get_thread_context" in "win32-i386-low.cc".
Make a shared function in gdb/nat/windows-nat.h?

>  	  CHECK (get_thread_context (th->h, context));
> +
> +	  if (xstate_features != 0)
> +	    {
> +	      DWORD64 features = 0;
> +	      CHECK (get_xstate_features_mask (context, &features));

Should this be changed to sth. like 

  if (!get_xstate_features_mask (context, &features))
    {
      warning (..)
      return;
    }

The call of "CHECK" only prints a message but doesn't error out.  If this
call fails we may still have features == 0.  This implies
"zeroed_features == xstate_features".  With this, the loop clears all features.
IIUC, this would clear the AVX registers on the next call of
"SetThreadContext".

Also refer to the implementation in gdbserver/win32-i386-low.cc:

      DWORD64 features = 0;
      if (xstate_features != 0
	  && get_xstate_features_mask (context, &features))
	{

I think it makes sense to unify those as the rest of the code is basically identical.  Put 
shared function into gdb/nat/windows-nat.h?  This keeps the code consistent.

> +	      DWORD64 zeroed_features = xstate_features & ~features;
> +
> +	      for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_CET_U_ID; f++)
> +		{
> +		  DWORD64 flag = 1ULL << f;
> +		  if ((zeroed_features & flag) != 0)
> +		    {
> +		      DWORD size = 0;
> +		      void *loc = locate_xstate_feature (context, f, &size);
> +		      if (loc != nullptr && size > 0)
> +			memset (loc, 0, size);
> +		    }
> +		}
> +	    }
>  	}
>      });
>  }
> @@ -198,6 +227,14 @@
> x86_windows_nat_target::thread_context_continue (windows_thread_info
> *th,
>  	  if (GetExitCodeThread (th->h, &ec)
>  	      && ec == STILL_ACTIVE)
>  	    {
> +	      DWORD debug_registers =
> WindowsContext<decltype(context)>::debug;
> +	      if (xstate_features != 0
> +		  && (context->ContextFlags & ~debug_registers) != 0)
> +		{
> +		  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> +		  set_xstate_features_mask (context, xstate_features);
> +		}
> +
>  	      BOOL status = set_thread_context (th->h, context);
> 
>  	      if (!killed)
> @@ -227,7 +264,7 @@ x86_windows_nat_target::thread_context_step
> (windows_thread_info *th,
> 
>  template<typename Context>
>  static char *
> -get_context_reg_ptr (Context *context, int r)
> +get_context_reg_ptr (Context *context, int r, i386_gdbarch_tdep *tdep)
>  {
>    const int *mappings;
>    int mappings_count;
> @@ -247,6 +284,13 @@ get_context_reg_ptr (Context *context, int r)
>    char *context_offset;
>    if (r < mappings_count)
>      context_offset = (char *) context + mappings[r];
> +  else if (I387_YMM0H_REGNUM (tdep) > 0 && r >= I387_YMM0H_REGNUM
> (tdep)
> +	   && r < I387_YMMENDH_REGNUM (tdep))

The implementation on gdbserver side guards against 

  xstate_features & X86_XSTATE_AVX) != 0

I wonder if the same guard would be helpful here, too.  I understand the register
number is initialized to -1, so this should not fire.  I'm not sure if it is possible to
have $ymm0 register number > 0 w/o xstate support, e.g., if the target description
is read from file, see "target_find_description"?

> +    {
> +      context_offset = (char *) locate_xstate_feature
> +	(context, X86_XSTATE_AVX_ID, NULL);
> +      context_offset += 16 * (r - I387_YMM0H_REGNUM (tdep));
> +    }
>    else
>      gdb_assert_not_reached ("invalid register number %d", r);
> 
> @@ -267,7 +311,7 @@ x86_windows_nat_target::fetch_one_register (struct
> regcache *regcache,
>    char *context_offset
>      = x86_windows_process.with_context (th, [&] (auto *context)
>      {
> -      return get_context_reg_ptr (context, r);
> +      return get_context_reg_ptr (context, r, tdep);
>      });
> 
>    gdb_assert (!gdbarch_read_pc_p (gdbarch));
> @@ -333,7 +377,7 @@ x86_windows_nat_target::store_one_register (const
> struct regcache *regcache,
>      = x86_windows_process.with_context (th, [&] (auto *context)
>      {
>        gdb_assert (context->ContextFlags != 0);
> -      return get_context_reg_ptr (context, r);
> +      return get_context_reg_ptr (context, r, tdep);
>      });
> 
>    /* GDB treats some registers as 32-bit, where they are in fact only
> @@ -368,6 +412,23 @@ x86_windows_nat_target::is_sw_breakpoint (const
> EXCEPTION_RECORD *er) const
>  	  || er->ExceptionCode == STATUS_WX86_BREAKPOINT);
>  }
> 
> +const struct target_desc *
> +x86_windows_nat_target::read_description ()
> +{
> +  if (inferior_ptid == null_ptid)
> +    return this->beneath ()->read_description ();
> +
> +  if (xstate_features == 0)
> +    return nullptr;
> +
> +#ifdef __x86_64__
> +  if (!x86_windows_process.wow64_process)
> +    return amd64_target_description (xstate_features, false);
> +  else
> +#endif
> +    return i386_target_description (xstate_features, false);
> +}
> +
>  /* Hardware watchpoint support, adapted from go32-nat.c code.  */
> 
>  /* Pass the address ADDR to the inferior in the I'th debug register.
> diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
> index b77f6adc6ed..a7e83c0239c 100644
> --- a/gdbserver/win32-i386-low.cc
> +++ b/gdbserver/win32-i386-low.cc
> @@ -253,6 +253,11 @@ i386_get_thread_context (windows_thread_info
> *th)
>  			       | WindowsContext<decltype(context)>::floating
>  			       | WindowsContext<decltype(context)>::debug
>  			       | extended_registers);
> +      if (xstate_features != 0)
> +	{
> +	  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> +	  set_xstate_features_mask (context, xstate_features);
> +	}
> 
>        BOOL ret = get_thread_context (th->h, context);
>        if (!ret)
> @@ -267,6 +272,24 @@ i386_get_thread_context (windows_thread_info
> *th)
> 
>  	  error (_("GetThreadContext failure %ld\n"), (long) e);
>  	}
> +
> +      DWORD64 features = 0;
> +      if (xstate_features != 0
> +	  && get_xstate_features_mask (context, &features))
> +	{
> +	  DWORD64 zeroed_features = xstate_features & ~features;
> +	  for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_CET_U_ID; f++)
> +	    {
> +	      DWORD64 flag = 1ULL << f;
> +	      if ((zeroed_features & flag) != 0)
> +		{
> +		  DWORD size = 0;
> +		  void *loc = locate_xstate_feature (context, f, &size);
> +		  if (loc != nullptr && size > 0)
> +		    memset (loc, 0, size);
> +		}
> +	    }
> +	}
>      });
>  }
> 
> @@ -292,6 +315,17 @@ i386_prepare_to_resume (windows_thread_info
> *th)
> 
>        th->debug_registers_changed = false;
>      }
> +
> +  windows_process.with_context (th, [&] (auto *context)
> +    {
> +      DWORD debug_registers = WindowsContext<decltype(context)>::debug;
> +      if (xstate_features != 0
> +	  && (context->ContextFlags & ~debug_registers) != 0)
> +	{
> +	  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> +	  set_xstate_features_mask (context, xstate_features);
> +	}
> +    });
>  }
> 
>  static void
> @@ -477,7 +511,7 @@ is_segment_register (int r)
> 
>  template<typename Context>
>  static char *
> -get_context_reg_ptr (Context *context, int r)
> +get_context_reg_ptr (Context *context, int r, const target_desc *tdesc)
>  {
>    const int *mappings;
>    int mappings_count;
> @@ -494,9 +528,21 @@ get_context_reg_ptr (Context *context, int r)
>        mappings_count = sizeof (i386_mappings) / sizeof (i386_mappings[0]);
>      }
> 
> +  bool amd64 = register_size (tdesc, 0) == 8;

There is already an " if (!windows_process.wow64_process)" a few lines above.
Wouldn't it make sense to move the "bool amd64" in the ifdef blocks and
assign accordingly?

> +  int ymm0h_regnum;
> +  const int num_xmm_registers = amd64 ? 16 : 8;
> +
>    char *context_offset;
>    if (r < mappings_count)
>      context_offset = (char *) context + mappings[r];
> +  else if ((xstate_features & X86_XSTATE_AVX) != 0
> +	   && r >= (ymm0h_regnum = find_regno (tdesc, "ymm0h"))
> +	   && r < ymm0h_regnum + num_xmm_registers)
> +    {
> +      context_offset = (char *) locate_xstate_feature
> +	(context, X86_XSTATE_AVX_ID, NULL);
> +      context_offset += 16 * (r - ymm0h_regnum);
> +    }
>    else
>      gdb_assert_not_reached ("invalid register number %d", r);
> 
> @@ -510,7 +556,7 @@ i386_fetch_inferior_register (struct regcache
> *regcache,
>  {
>    char *context_offset = windows_process.with_context (th, [&] (auto
> *context)
>      {
> -      return get_context_reg_ptr (context, r);
> +      return get_context_reg_ptr (context, r, regcache->tdesc);
>      });
> 
>    /* GDB treats some registers as 32-bit, where they are in fact only
> @@ -538,7 +584,7 @@ i386_store_inferior_register (struct regcache
> *regcache,
>  {
>    char *context_offset = windows_process.with_context (th, [&] (auto
> *context)
>      {
> -      return get_context_reg_ptr (context, r);
> +      return get_context_reg_ptr (context, r, regcache->tdesc);
>      });
> 
>    /* GDB treats some registers as 32-bit, where they are in fact only
> @@ -571,14 +617,17 @@ i386_arch_setup (void)
>  {
>   target_desc_up tdesc;
> 
> +  DWORD64 xcr0 = xstate_features;
> +  if (xcr0 == 0)
> +    xcr0 = X86_XSTATE_SSE_MASK;
> +
>  #ifdef __x86_64__
> -  tdesc = amd64_create_target_description (X86_XSTATE_SSE_MASK, false,
> -					   false, false);
> +  tdesc = amd64_create_target_description (xcr0, false, false, false);
>    init_target_desc (tdesc.get (), amd64_expedite_regs, WINDOWS_OSABI);
>    win32_tdesc = std::move (tdesc);
>  #endif
> 
> -  tdesc = i386_create_target_description (X86_XSTATE_SSE_MASK, false,
> false);
> +  tdesc = i386_create_target_description (xcr0, false, false);
>    init_target_desc (tdesc.get (), i386_expedite_regs, WINDOWS_OSABI);
>  #ifdef __x86_64__
>    wow64_win32_tdesc = std::move (tdesc);
> diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
> index 7629beca213..5ccdc89a7ef 100644
> --- a/gdbserver/win32-low.cc
> +++ b/gdbserver/win32-low.cc
> @@ -33,6 +33,7 @@
>  #include <process.h>
>  #include "gdbsupport/gdb_tilde_expand.h"
>  #include "gdbsupport/common-inferior.h"
> +#include "tdesc.h"
> 
>  using namespace windows_nat;
> 
> @@ -426,8 +427,9 @@ child_fetch_inferior_registers (struct regcache
> *regcache, int r)
>    int regno;
>    windows_thread_info *th = windows_process.find_thread (current_thread-
> >id);
>    win32_require_context (th);
> -  if (r == -1 || r > NUM_REGS)
> -    child_fetch_inferior_registers (regcache, NUM_REGS);

IIUC this was the only use of the NUM_REGS define.  We can remove it.
Same for " i386_win32_num_regs (void)" and "aarch64_win32_num_regs ()".
This allows removing the num_regs hook in win32_target_ops.

> +  if (r == -1)
> +    child_fetch_inferior_registers (regcache,
> +				    regcache->tdesc->reg_defs.size ());
>    else
>      for (regno = 0; regno < r; regno++)
>        (*the_low_target.fetch_inferior_register) (regcache, th, regno);
> @@ -441,8 +443,9 @@ child_store_inferior_registers (struct regcache
> *regcache, int r)
>    int regno;
>    windows_thread_info *th = windows_process.find_thread (current_thread-
> >id);
>    win32_require_context (th);
> -  if (r == -1 || r == 0 || r > NUM_REGS)
> -    child_store_inferior_registers (regcache, NUM_REGS);
> +  if (r == -1)
> +    child_store_inferior_registers (regcache,
> +				    regcache->tdesc->reg_defs.size ());
>    else
>      for (regno = 0; regno < r; regno++)
>        (*the_low_target.store_inferior_register) (regcache, th, regno);
> @@ -1349,7 +1352,9 @@ void
>  initialize_low (void)
>  {
>    set_target_ops (&the_win32_target);
> -  the_low_target.arch_setup ();
> 
>    initialize_loadable ();
> +  /* Has to be done after initialize_loadable, because it uses the xstate
> +     functions if available.  */
> +  the_low_target.arch_setup ();
>  }
> --
> 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.

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

[Joos, Christina]

Hi Hannes, 

I saw that Stephan already reviewed this (thanks!).
I added my remarks on top, see below.

> -----Original Message-----
> From: Rohr, Stephan <stephan.rohr@intel.com>
> Sent: Dienstag, 8. September 2026 15:05
> To: Hannes Domani <ssbssa@yahoo.de>; gdb-patches@sourceware.org; gdb-
> patches@sourceware.org
> Cc: Joos, Christina <christina.joos@intel.com>; Tom Tromey
> <tom@tromey.com>
> Subject: RE: [PATCH v3 7/8] Windows gdb: Implement AVX register support
> 
> Hi Hannes,
> 
> please see some feedback inlined below.
> 
> Let me know if you have any questions.
> 
> 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 7/8] Windows gdb: Implement AVX register support
> >
> > This adds support for the Intel AVX registers on Windows.
> > It enables accessing registers $ymm0 - $ymm15 where they are available.
> >
> > After this patch gdb.arch/i386-avx.exp passes on windows.
> > ---
> > v3:
> >   - merged gdb+gdbserver parts, and split again AVX/AVX-512 parts
> > ---
> >  gdb/NEWS                    |  3 ++
> >  gdb/nat/windows-nat.c       |  2 +-
> >  gdb/x86-windows-nat.c       | 67
> > +++++++++++++++++++++++++++++++++++--
> >  gdbserver/win32-i386-low.cc | 61 +++++++++++++++++++++++++++++----
> >  gdbserver/win32-low.cc      | 15 ++++++---
> >  5 files changed, 133 insertions(+), 15 deletions(-)
> >
> > diff --git a/gdb/NEWS b/gdb/NEWS
> > index 10c182067f9..f7effc822e9 100644
> > --- a/gdb/NEWS
> > +++ b/gdb/NEWS
> > @@ -118,6 +118,9 @@
> >    intent to remove it in a future release.
> >    The s390 64-bit target (s390x-*) remains supported.
> >
> > +* Support for Intel AVX registers on Windows.
> > +  Support displaying and modifying Intel AVX registers $ymm0 - $ymm31.
> > +
> 
> I think this should be registers $ymm0 - $ymm15 ?

Yes, I agree with Stephan's feedback here.
The AVX state only comprises only YMM0–YMM15 for 64 bit. 
For 32-bit mode, it's YMM0- YMM7 only. 
See the docs added in commit "Add org.gnu.gdb.i386.avx."

> >  * Configure changes
> >
> >  ** --with-babeltrace has been removed.  The babeltrace library was
> > diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c index
> > 8930536f3ba..c9a21d7c41f 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_SSE_MASK);
> > +			 & X86_XSTATE_AVX_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
> > 3af5ef4dae0..1cefe6171be 100644
> > --- a/gdb/x86-windows-nat.c
> > +++ b/gdb/x86-windows-nat.c
> > @@ -27,6 +27,9 @@
> >
> >  #include "i386-tdep.h"
> >  #include "i387-tdep.h"
> > +#ifdef __x86_64__
> > +#include "amd64-tdep.h"
> > +#endif
> >
> >  using namespace windows_nat;
> >
> > @@ -70,6 +73,8 @@ struct x86_windows_nat_target final : public
> > x86_nat_target<windows_nat_target>
> >  			   windows_thread_info *th, int r) override;
> >
> >    bool is_sw_breakpoint (const EXCEPTION_RECORD *er) const override;
> > +
> > +  const struct target_desc *read_description () override;
> >  };
> >
> >  /* The current process.  */
> > @@ -109,7 +114,31 @@ x86_windows_per_inferior::fill_thread_context
> > (windows_thread_info *th)
> >        if (context->ContextFlags == 0)
> >  	{
> >  	  context->ContextFlags = WindowsContext<decltype(context)>::all;
> > +	  if (xstate_features != 0)
> > +	    {
> > +	      context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > +	      set_xstate_features_mask (context, xstate_features);
> > +	    }
> 
> We have the same code in "i386_get_thread_context" in "win32-i386-low.cc".
> Make a shared function in gdb/nat/windows-nat.h?
> 
> >  	  CHECK (get_thread_context (th->h, context));
> > +
> > +	  if (xstate_features != 0)
> > +	    {
> > +	      DWORD64 features = 0;
> > +	      CHECK (get_xstate_features_mask (context, &features));
> 
> Should this be changed to sth. like
> 
>   if (!get_xstate_features_mask (context, &features))
>     {
>       warning (..)
>       return;
>     }
> 
> The call of "CHECK" only prints a message but doesn't error out.  If this call fails
> we may still have features == 0.  This implies "zeroed_features ==
> xstate_features".  With this, the loop clears all features.
> IIUC, this would clear the AVX registers on the next call of "SetThreadContext".
> 
> Also refer to the implementation in gdbserver/win32-i386-low.cc:
> 
>       DWORD64 features = 0;
>       if (xstate_features != 0
> 	  && get_xstate_features_mask (context, &features))
> 	{
> 
> I think it makes sense to unify those as the rest of the code is basically identical.
> Put shared function into gdb/nat/windows-nat.h?  This keeps the code
> consistent.
> 
> > +	      DWORD64 zeroed_features = xstate_features & ~features;
> > +
> > +	      for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_CET_U_ID; f++)

In addition to Stephan's feedback for this code area:
Since we decided to not add CET, and this patch is even for AVX only for now only, do we need a loop at this point already?

For the follow up AVX-512 patch I think we can stop at the highest supported feature (AVX-512) in windows, can't we?

> > +		  DWORD64 flag = 1ULL << f;
> > +		  if ((zeroed_features & flag) != 0)
> > +		    {
> > +		      DWORD size = 0;
> > +		      void *loc = locate_xstate_feature (context, f, &size);
> > +		      if (loc != nullptr && size > 0)
> > +			memset (loc, 0, size);
> > +		    }
> > +		}
> > +	    }
> >  	}
> >      });
> >  }
> > @@ -198,6 +227,14 @@
> > x86_windows_nat_target::thread_context_continue (windows_thread_info
> > *th,
> >  	  if (GetExitCodeThread (th->h, &ec)
> >  	      && ec == STILL_ACTIVE)
> >  	    {
> > +	      DWORD debug_registers =
> > WindowsContext<decltype(context)>::debug;
> > +	      if (xstate_features != 0
> > +		  && (context->ContextFlags & ~debug_registers) != 0)
> > +		{
> > +		  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > +		  set_xstate_features_mask (context, xstate_features);
> > +		}
> > +
> >  	      BOOL status = set_thread_context (th->h, context);
> >
> >  	      if (!killed)
> > @@ -227,7 +264,7 @@ x86_windows_nat_target::thread_context_step
> > (windows_thread_info *th,
> >
> >  template<typename Context>
> >  static char *
> > -get_context_reg_ptr (Context *context, int r)
> > +get_context_reg_ptr (Context *context, int r, i386_gdbarch_tdep
> > +*tdep)
> >  {
> >    const int *mappings;
> >    int mappings_count;
> > @@ -247,6 +284,13 @@ get_context_reg_ptr (Context *context, int r)
> >    char *context_offset;
> >    if (r < mappings_count)
> >      context_offset = (char *) context + mappings[r];
> > +  else if (I387_YMM0H_REGNUM (tdep) > 0 && r >=
> I387_YMM0H_REGNUM
> > (tdep)
> > +	   && r < I387_YMMENDH_REGNUM (tdep))
> 
> The implementation on gdbserver side guards against
> 
>   xstate_features & X86_XSTATE_AVX) != 0
> 
> I wonder if the same guard would be helpful here, too.  I understand the
> register number is initialized to -1, so this should not fire.  I'm not sure if it is
> possible to have $ymm0 register number > 0 w/o xstate support, e.g., if the
> target description is read from file, see "target_find_description"?
> 
> > +    {
> > +      context_offset = (char *) locate_xstate_feature
> > +	(context, X86_XSTATE_AVX_ID, NULL);

We prefer to use nullptr.

> > +      context_offset += 16 * (r - I387_YMM0H_REGNUM (tdep));
> > +    }
> >    else
> >      gdb_assert_not_reached ("invalid register number %d", r);
> >
> > @@ -267,7 +311,7 @@ x86_windows_nat_target::fetch_one_register (struct
> > regcache *regcache,
> >    char *context_offset
> >      = x86_windows_process.with_context (th, [&] (auto *context)
> >      {
> > -      return get_context_reg_ptr (context, r);
> > +      return get_context_reg_ptr (context, r, tdep);
> >      });
> >
> >    gdb_assert (!gdbarch_read_pc_p (gdbarch)); @@ -333,7 +377,7 @@
> > x86_windows_nat_target::store_one_register (const struct regcache
> > *regcache,
> >      = x86_windows_process.with_context (th, [&] (auto *context)
> >      {
> >        gdb_assert (context->ContextFlags != 0);
> > -      return get_context_reg_ptr (context, r);
> > +      return get_context_reg_ptr (context, r, tdep);
> >      });
> >
> >    /* GDB treats some registers as 32-bit, where they are in fact only
> > @@ -368,6 +412,23 @@ x86_windows_nat_target::is_sw_breakpoint (const
> > EXCEPTION_RECORD *er) const
> >  	  || er->ExceptionCode == STATUS_WX86_BREAKPOINT);  }
> >
> > +const struct target_desc *
> > +x86_windows_nat_target::read_description () {
> > +  if (inferior_ptid == null_ptid)
> > +    return this->beneath ()->read_description ();
> > +
> > +  if (xstate_features == 0)
> > +    return nullptr;
> > +
> > +#ifdef __x86_64__
> > +  if (!x86_windows_process.wow64_process)
> > +    return amd64_target_description (xstate_features, false);
> > +  else
> > +#endif
> > +    return i386_target_description (xstate_features, false); }
> > +
> >  /* Hardware watchpoint support, adapted from go32-nat.c code.  */
> >
> >  /* Pass the address ADDR to the inferior in the I'th debug register.
> > diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
> > index b77f6adc6ed..a7e83c0239c 100644
> > --- a/gdbserver/win32-i386-low.cc
> > +++ b/gdbserver/win32-i386-low.cc
> > @@ -253,6 +253,11 @@ i386_get_thread_context (windows_thread_info
> > *th)
> >  			       | WindowsContext<decltype(context)>::floating
> >  			       | WindowsContext<decltype(context)>::debug
> >  			       | extended_registers);
> > +      if (xstate_features != 0)
> > +	{
> > +	  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > +	  set_xstate_features_mask (context, xstate_features);
> > +	}
> >
> >        BOOL ret = get_thread_context (th->h, context);
> >        if (!ret)
> > @@ -267,6 +272,24 @@ i386_get_thread_context (windows_thread_info
> > *th)
> >
> >  	  error (_("GetThreadContext failure %ld\n"), (long) e);
> >  	}
> > +
> > +      DWORD64 features = 0;
> > +      if (xstate_features != 0
> > +	  && get_xstate_features_mask (context, &features))
> > +	{
> > +	  DWORD64 zeroed_features = xstate_features & ~features;
> > +	  for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_CET_U_ID; f++)
> > +	    {
> > +	      DWORD64 flag = 1ULL << f;
> > +	      if ((zeroed_features & flag) != 0)
> > +		{
> > +		  DWORD size = 0;
> > +		  void *loc = locate_xstate_feature (context, f, &size);
> > +		  if (loc != nullptr && size > 0)
> > +		    memset (loc, 0, size);
> > +		}
> > +	    }
> > +	}
> >      });
> >  }
> >
> > @@ -292,6 +315,17 @@ i386_prepare_to_resume (windows_thread_info
> > *th)
> >
> >        th->debug_registers_changed = false;
> >      }
> > +
> > +  windows_process.with_context (th, [&] (auto *context)
> > +    {
> > +      DWORD debug_registers = WindowsContext<decltype(context)>::debug;
> > +      if (xstate_features != 0
> > +	  && (context->ContextFlags & ~debug_registers) != 0)
> > +	{
> > +	  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > +	  set_xstate_features_mask (context, xstate_features);
> > +	}
> > +    });
> >  }
> >
> >  static void
> > @@ -477,7 +511,7 @@ is_segment_register (int r)
> >
> >  template<typename Context>
> >  static char *
> > -get_context_reg_ptr (Context *context, int r)
> > +get_context_reg_ptr (Context *context, int r, const target_desc
> > +*tdesc)
> >  {
> >    const int *mappings;
> >    int mappings_count;
> > @@ -494,9 +528,21 @@ get_context_reg_ptr (Context *context, int r)
> >        mappings_count = sizeof (i386_mappings) / sizeof (i386_mappings[0]);
> >      }
> >
> > +  bool amd64 = register_size (tdesc, 0) == 8;
> 
> There is already an " if (!windows_process.wow64_process)" a few lines above.
> Wouldn't it make sense to move the "bool amd64" in the ifdef blocks and assign
> accordingly?
> 
> > +  int ymm0h_regnum;
> > +  const int num_xmm_registers = amd64 ? 16 : 8;
> > +
> >    char *context_offset;
> >    if (r < mappings_count)
> >      context_offset = (char *) context + mappings[r];
> > +  else if ((xstate_features & X86_XSTATE_AVX) != 0
> > +	   && r >= (ymm0h_regnum = find_regno (tdesc, "ymm0h"))
> > +	   && r < ymm0h_regnum + num_xmm_registers)
> > +    {
> > +      context_offset = (char *) locate_xstate_feature
> > +	(context, X86_XSTATE_AVX_ID, NULL);
> > +      context_offset += 16 * (r - ymm0h_regnum);
> > +    }
> >    else
> >      gdb_assert_not_reached ("invalid register number %d", r);
> >
> > @@ -510,7 +556,7 @@ i386_fetch_inferior_register (struct regcache
> > *regcache,  {
> >    char *context_offset = windows_process.with_context (th, [&] (auto
> > *context)
> >      {
> > -      return get_context_reg_ptr (context, r);
> > +      return get_context_reg_ptr (context, r, regcache->tdesc);
> >      });
> >
> >    /* GDB treats some registers as 32-bit, where they are in fact only
> > @@ -538,7 +584,7 @@ i386_store_inferior_register (struct regcache
> > *regcache,  {
> >    char *context_offset = windows_process.with_context (th, [&] (auto
> > *context)
> >      {
> > -      return get_context_reg_ptr (context, r);
> > +      return get_context_reg_ptr (context, r, regcache->tdesc);
> >      });
> >
> >    /* GDB treats some registers as 32-bit, where they are in fact only
> > @@ -571,14 +617,17 @@ i386_arch_setup (void)  {
> >   target_desc_up tdesc;
> >
> > +  DWORD64 xcr0 = xstate_features;
> > +  if (xcr0 == 0)
> > +    xcr0 = X86_XSTATE_SSE_MASK;
> > +
> >  #ifdef __x86_64__
> > -  tdesc = amd64_create_target_description (X86_XSTATE_SSE_MASK, false,
> > -					   false, false);
> > +  tdesc = amd64_create_target_description (xcr0, false, false,
> > + false);
> >    init_target_desc (tdesc.get (), amd64_expedite_regs, WINDOWS_OSABI);
> >    win32_tdesc = std::move (tdesc);
> >  #endif
> >
> > -  tdesc = i386_create_target_description (X86_XSTATE_SSE_MASK, false,
> > false);
> > +  tdesc = i386_create_target_description (xcr0, false, false);
> >    init_target_desc (tdesc.get (), i386_expedite_regs, WINDOWS_OSABI);
> > #ifdef __x86_64__
> >    wow64_win32_tdesc = std::move (tdesc); diff --git
> > a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc index
> > 7629beca213..5ccdc89a7ef 100644
> > --- a/gdbserver/win32-low.cc
> > +++ b/gdbserver/win32-low.cc
> > @@ -33,6 +33,7 @@
> >  #include <process.h>
> >  #include "gdbsupport/gdb_tilde_expand.h"
> >  #include "gdbsupport/common-inferior.h"
> > +#include "tdesc.h"
> >
> >  using namespace windows_nat;
> >
> > @@ -426,8 +427,9 @@ child_fetch_inferior_registers (struct regcache
> > *regcache, int r)
> >    int regno;
> >    windows_thread_info *th = windows_process.find_thread
> > (current_thread-
> > >id);
> >    win32_require_context (th);
> > -  if (r == -1 || r > NUM_REGS)
> > -    child_fetch_inferior_registers (regcache, NUM_REGS);
> 
> IIUC this was the only use of the NUM_REGS define.  We can remove it.

Yes, I agree. Would you mind explaining why we don't need this check anymore, too ? :)
I don't understand it yet, unfortunately. Was it necessary before or is this due to AVX?

> Same for " i386_win32_num_regs (void)" and "aarch64_win32_num_regs ()".
> This allows removing the num_regs hook in win32_target_ops.

> > +  if (r == -1)
> > +    child_fetch_inferior_registers (regcache,
> > +				    regcache->tdesc->reg_defs.size ());
> >    else
> >      for (regno = 0; regno < r; regno++)
> >        (*the_low_target.fetch_inferior_register) (regcache, th,
> > regno); @@ -441,8 +443,9 @@ child_store_inferior_registers (struct
> > regcache *regcache, int r)
> >    int regno;
> >    windows_thread_info *th = windows_process.find_thread
> > (current_thread-
> > >id);
> >    win32_require_context (th);
> > -  if (r == -1 || r == 0 || r > NUM_REGS)
> > -    child_store_inferior_registers (regcache, NUM_REGS);
> > +  if (r == -1)
> > +    child_store_inferior_registers (regcache,
> > +				    regcache->tdesc->reg_defs.size ());

I assume removing r == 0 is not related to AVX register support and just a cleanup, right?
Could we make it a separate cleanup patch (with reason)?

I know it seems like a super tiny nit, but it would help to understand why that kind of
refactoring/cleanup is necessary (e.g. due to AVX or not).


> >    else
> >      for (regno = 0; regno < r; regno++)
> >        (*the_low_target.store_inferior_register) (regcache, th,
> > regno); @@ -1349,7 +1352,9 @@ void  initialize_low (void)  {
> >    set_target_ops (&the_win32_target);
> > -  the_low_target.arch_setup ();
> >
> >    initialize_loadable ();
> > +  /* Has to be done after initialize_loadable, because it uses the xstate
> > +     functions if available.  */
> > +  the_low_target.arch_setup ();
> >  }
> > --
> > 2.54.0

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.

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

[Hannes Domani]

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.
+  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))
+    {
+      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);
 
-- 
2.54.0

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

[Eli Zaretskii]

> From: Hannes Domani <ssbssa@yahoo.de>
> Date: Sat, 29 Aug 2026 16:49:05 +0200
> 
> 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(-)

Thanks, the NEWS part is approved.

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

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:

Hannes> This adds support for the Intel AVX-512 registers on Windows.
Hannes> It enables accessing registers $ymm0 - $ymm31, $zmm0 - $zmm31, and
Hannes> $k0 - $k7 where they are available.

Hannes>        /* Available XState features masked with implemented features.  */
Hannes>        xstate_features = (GetEnabledXStateFeatures ()
Hannes> -			 & X86_XSTATE_AVX_MASK);
Hannes> +			 & X86_XSTATE_AVX_AVX512_MASK);

I guess I have the same question here about the features being
vendor-defined.

Tom

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

[Rohr, Stephan]

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.

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

[Joos, Christina]

Hi Hannes, 

Thank you for this patch and your work on this.

Please find my comments on top of Stephan's feedback. 

> -----Original Message-----
> From: Rohr, Stephan <stephan.rohr@intel.com>
> Sent: Dienstag, 8. September 2026 15:05
> To: Hannes Domani <ssbssa@yahoo.de>; gdb-patches@sourceware.org
> Cc: Joos, Christina <christina.joos@intel.com>; Tom Tromey
> <tom@tromey.com>
> Subject: RE: [PATCH v3 8/8] Windows gdb: Implement AVX-512 register support
> 
> 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.

Suggestion:
For the linux side we have this commit to enable AMX-512:
" Add AVX512 registers support to GDB and GDBserver."

It includes a description which registers are added, extended etc:

" Intel(R) AVX-512 is an extension to AVX to support 512-bit wide
SIMD registers in 64-bit mode (XMM0-XMM31, YMM0-YMM31, ZMM0-ZMM31).
The number of available registers in 32-bit mode is still 8
(XMM0-7, YMM0-7, ZMM0-7).  The lower 256-bits of the ZMM registers
are aliased to the respective 256-bit YMM registers.  The lower
128-bits are aliased to the respective 128-bit XMM registers.

There are also 8 new, dedicated mask registers (K0-K7) in both 32-bit
mode and 64-bit mode."

Maybe you could reference it, something like
"similar to commit xyz for linux this patch adds [...] for 32 and 64-bit mode in windows." ?

Then you have all the details described, if one would like to look it up to understand your
patch in detail. But you don't have to repeat it in your own commit message.

What do you think?

> > 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);
> > +    }

Similar comment  to the previous patch for nullptr here and below.

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

I have a similar comment on this, but for the offsets:

Those offsets are similar in gdbserver/i387-fp.cc and gdb/i387-tdep.c, but we
cannot use/share the complete logic for windows gdb & gdbserver I believe.

However, the offsets inside one feature (I don't mean the offsets between two
consecutive features) should be the same for any OS and independent of AMD or Intel.
I believe it would be nice if we could at least share the offset calculations at some point
maybe in gdbsupport/* sometime in future... 
But this is nothing that you must do as part of this patch of course.

For this patch:
I don't think this is strictly necessary, but we could make this code a bit more generic inside each individual file on gdb & gdbserver side.
For example, we could introduce some static helpers or similar, that are called in each else if. 
What do you think? 

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.

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

[Hannes Domani]

 Am Donnerstag, 10. September 2026 um 13:33:20 MESZ hat Joos, Christina <christina.joos@intel.com> Folgendes geschrieben:

> Hi Hannes,
> 
> I saw that Stephan already reviewed this (thanks!).
> I added my remarks on top, see below.
> 
> > -----Original Message-----
> > From: Rohr, Stephan <stephan.rohr@intel.com>
> > Sent: Dienstag, 8. September 2026 15:05
> > To: Hannes Domani <ssbssa@yahoo.de>; gdb-patches@sourceware.org; gdb-
> > patches@sourceware.org
> > Cc: Joos, Christina <christina.joos@intel.com>; Tom Tromey
> > <tom@tromey.com>
> > Subject: RE: [PATCH v3 7/8] Windows gdb: Implement AVX register support
> >
> > Hi Hannes,
> >
> > please see some feedback inlined below.
> >
> > Let me know if you have any questions.
> >
> > 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 7/8] Windows gdb: Implement AVX register support
> > >
> > > This adds support for the Intel AVX registers on Windows.
> > > It enables accessing registers $ymm0 - $ymm15 where they are available.
> > >
> > > After this patch gdb.arch/i386-avx.exp passes on windows.
> > > ---
> > > v3:
> > >  - merged gdb+gdbserver parts, and split again AVX/AVX-512 parts
> > > ---
> > >  gdb/NEWS                    |  3 ++
> > >  gdb/nat/windows-nat.c      |  2 +-
> > >  gdb/x86-windows-nat.c      | 67
> > > +++++++++++++++++++++++++++++++++++--
> > >  gdbserver/win32-i386-low.cc | 61 +++++++++++++++++++++++++++++----
> > >  gdbserver/win32-low.cc      | 15 ++++++---
> > >  5 files changed, 133 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/gdb/NEWS b/gdb/NEWS
> > > index 10c182067f9..f7effc822e9 100644
> > > --- a/gdb/NEWS
> > > +++ b/gdb/NEWS
> > > @@ -118,6 +118,9 @@
> > >    intent to remove it in a future release.
> > >    The s390 64-bit target (s390x-*) remains supported.
> > >
> > > +* Support for Intel AVX registers on Windows.
> > > +  Support displaying and modifying Intel AVX registers $ymm0 - $ymm31.
> > > +
> >
> > I think this should be registers $ymm0 - $ymm15 ?
> 
> Yes, I agree with Stephan's feedback here.
> The AVX state only comprises only YMM0–YMM15 for 64 bit.
> For 32-bit mode, it's YMM0- YMM7 only.
> See the docs added in commit "Add org.gnu.gdb.i386.avx."

Yes, I missed this when I split AVX/AVX-512.


> > >  * Configure changes
> > >
> > >  ** --with-babeltrace has been removed.  The babeltrace library was
> > > diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c index
> > > 8930536f3ba..c9a21d7c41f 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_SSE_MASK);
> > > +            & X86_XSTATE_AVX_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
> > > 3af5ef4dae0..1cefe6171be 100644
> > > --- a/gdb/x86-windows-nat.c
> > > +++ b/gdb/x86-windows-nat.c
> > > @@ -27,6 +27,9 @@
> > >
> > >  #include "i386-tdep.h"
> > >  #include "i387-tdep.h"
> > > +#ifdef __x86_64__
> > > +#include "amd64-tdep.h"
> > > +#endif
> > >
> > >  using namespace windows_nat;
> > >
> > > @@ -70,6 +73,8 @@ struct x86_windows_nat_target final : public
> > > x86_nat_target<windows_nat_target>
> > >                windows_thread_info *th, int r) override;
> > >
> > >    bool is_sw_breakpoint (const EXCEPTION_RECORD *er) const override;
> > > +
> > > +  const struct target_desc *read_description () override;
> > >  };
> > >
> > >  /* The current process.  */
> > > @@ -109,7 +114,31 @@ x86_windows_per_inferior::fill_thread_context
> > > (windows_thread_info *th)
> > >        if (context->ContextFlags == 0)
> > >      {
> > >        context->ContextFlags = WindowsContext<decltype(context)>::all;
> > > +      if (xstate_features != 0)
> > > +        {
> > > +          context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > > +          set_xstate_features_mask (context, xstate_features);
> > > +        }
> >
> > We have the same code in "i386_get_thread_context" in "win32-i386-low.cc".
> > Make a shared function in gdb/nat/windows-nat.h?
> >
> > >        CHECK (get_thread_context (th->h, context));
> > > +
> > > +      if (xstate_features != 0)
> > > +        {
> > > +          DWORD64 features = 0;
> > > +          CHECK (get_xstate_features_mask (context, &features));
> >
> > Should this be changed to sth. like
> >
> >  if (!get_xstate_features_mask (context, &features))
> >    {
> >      warning (..)
> >      return;
> >    }
> >
> > The call of "CHECK" only prints a message but doesn't error out.  If this call fails
> > we may still have features == 0.  This implies "zeroed_features ==
> > xstate_features".  With this, the loop clears all features.
> > IIUC, this would clear the AVX registers on the next call of "SetThreadContext".
> >
> > Also refer to the implementation in gdbserver/win32-i386-low.cc:
> >
> >      DWORD64 features = 0;
> >      if (xstate_features != 0
> >       && get_xstate_features_mask (context, &features))
> >     {
> >
> > I think it makes sense to unify those as the rest of the code is basically identical.
> > Put shared function into gdb/nat/windows-nat.h?  This keeps the code
> > consistent.

I will try that.


> > > +          DWORD64 zeroed_features = xstate_features & ~features;
> > > +
> > > +          for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_CET_U_ID; f++)
> 
> In addition to Stephan's feedback for this code area:
> Since we decided to not add CET, and this patch is even for AVX only for now only, do we need a loop at this point already?
> 
> For the follow up AVX-512 patch I think we can stop at the highest supported feature (AVX-512) in windows, can't we?

I would prefer it if we could keep it a loop, even if it's only X86_XSTATE_AVX_ID in the AVX patch.
And yes, I forgot to change it to X86_XSTATE_ZMM_ID when I removed the CET stuff.


> > > +          DWORD64 flag = 1ULL << f;
> > > +          if ((zeroed_features & flag) != 0)
> > > +            {
> > > +              DWORD size = 0;
> > > +              void *loc = locate_xstate_feature (context, f, &size);
> > > +              if (loc != nullptr && size > 0)
> > > +            memset (loc, 0, size);
> > > +            }
> > > +        }
> > > +        }
> > >      }
> > >      });
> > >  }
> > > @@ -198,6 +227,14 @@
> > > x86_windows_nat_target::thread_context_continue (windows_thread_info
> > > *th,
> > >        if (GetExitCodeThread (th->h, &ec)
> > >            && ec == STILL_ACTIVE)
> > >          {
> > > +          DWORD debug_registers =
> > > WindowsContext<decltype(context)>::debug;
> > > +          if (xstate_features != 0
> > > +          && (context->ContextFlags & ~debug_registers) != 0)
> > > +        {
> > > +          context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > > +          set_xstate_features_mask (context, xstate_features);
> > > +        }
> > > +
> > >            BOOL status = set_thread_context (th->h, context);
> > >
> > >            if (!killed)
> > > @@ -227,7 +264,7 @@ x86_windows_nat_target::thread_context_step
> > > (windows_thread_info *th,
> > >
> > >  template<typename Context>
> > >  static char *
> > > -get_context_reg_ptr (Context *context, int r)
> > > +get_context_reg_ptr (Context *context, int r, i386_gdbarch_tdep
> > > +*tdep)
> > >  {
> > >    const int *mappings;
> > >    int mappings_count;
> > > @@ -247,6 +284,13 @@ get_context_reg_ptr (Context *context, int r)
> > >    char *context_offset;
> > >    if (r < mappings_count)
> > >      context_offset = (char *) context + mappings[r];
> > > +  else if (I387_YMM0H_REGNUM (tdep) > 0 && r >=
> > I387_YMM0H_REGNUM
> > > (tdep)
> > > +      && r < I387_YMMENDH_REGNUM (tdep))
> >
> > The implementation on gdbserver side guards against
> >
> >  xstate_features & X86_XSTATE_AVX) != 0
> >
> > I wonder if the same guard would be helpful here, too.  I understand the
> > register number is initialized to -1, so this should not fire.  I'm not sure if it is
> > possible to have $ymm0 register number > 0 w/o xstate support, e.g., if the
> > target description is read from file, see "target_find_description"?

On gdbserver side I needed the guard because there is no register number
I could check.
I didn't think it was necessary on gdb side as well, but if you prefer it
like this, I will add it.


> > > +    {
> > > +      context_offset = (char *) locate_xstate_feature
> > > +    (context, X86_XSTATE_AVX_ID, NULL);
> 
> We prefer to use nullptr.

Right.


> > > +      context_offset += 16 * (r - I387_YMM0H_REGNUM (tdep));
> > > +    }
> > >    else
> > >      gdb_assert_not_reached ("invalid register number %d", r);
> > >
> > > @@ -267,7 +311,7 @@ x86_windows_nat_target::fetch_one_register (struct
> > > regcache *regcache,
> > >    char *context_offset
> > >      = x86_windows_process.with_context (th, [&] (auto *context)
> > >      {
> > > -      return get_context_reg_ptr (context, r);
> > > +      return get_context_reg_ptr (context, r, tdep);
> > >      });
> > >
> > >    gdb_assert (!gdbarch_read_pc_p (gdbarch)); @@ -333,7 +377,7 @@
> > > x86_windows_nat_target::store_one_register (const struct regcache
> > > *regcache,
> > >      = x86_windows_process.with_context (th, [&] (auto *context)
> > >      {
> > >        gdb_assert (context->ContextFlags != 0);
> > > -      return get_context_reg_ptr (context, r);
> > > +      return get_context_reg_ptr (context, r, tdep);
> > >      });
> > >
> > >    /* GDB treats some registers as 32-bit, where they are in fact only
> > > @@ -368,6 +412,23 @@ x86_windows_nat_target::is_sw_breakpoint (const
> > > EXCEPTION_RECORD *er) const
> > >        || er->ExceptionCode == STATUS_WX86_BREAKPOINT);  }
> > >
> > > +const struct target_desc *
> > > +x86_windows_nat_target::read_description () {
> > > +  if (inferior_ptid == null_ptid)
> > > +    return this->beneath ()->read_description ();
> > > +
> > > +  if (xstate_features == 0)
> > > +    return nullptr;
> > > +
> > > +#ifdef __x86_64__
> > > +  if (!x86_windows_process.wow64_process)
> > > +    return amd64_target_description (xstate_features, false);
> > > +  else
> > > +#endif
> > > +    return i386_target_description (xstate_features, false); }
> > > +
> > >  /* Hardware watchpoint support, adapted from go32-nat.c code.  */
> > >
> > >  /* Pass the address ADDR to the inferior in the I'th debug register.
> > > diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
> > > index b77f6adc6ed..a7e83c0239c 100644
> > > --- a/gdbserver/win32-i386-low.cc
> > > +++ b/gdbserver/win32-i386-low.cc
> > > @@ -253,6 +253,11 @@ i386_get_thread_context (windows_thread_info
> > > *th)
> > >                    | WindowsContext<decltype(context)>::floating
> > >                    | WindowsContext<decltype(context)>::debug
> > >                    | extended_registers);
> > > +      if (xstate_features != 0)
> > > +    {
> > > +      context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > > +      set_xstate_features_mask (context, xstate_features);
> > > +    }
> > >
> > >        BOOL ret = get_thread_context (th->h, context);
> > >        if (!ret)
> > > @@ -267,6 +272,24 @@ i386_get_thread_context (windows_thread_info
> > > *th)
> > >
> > >        error (_("GetThreadContext failure %ld\n"), (long) e);
> > >      }
> > > +
> > > +      DWORD64 features = 0;
> > > +      if (xstate_features != 0
> > > +      && get_xstate_features_mask (context, &features))
> > > +    {
> > > +      DWORD64 zeroed_features = xstate_features & ~features;
> > > +      for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_CET_U_ID; f++)
> > > +        {
> > > +          DWORD64 flag = 1ULL << f;
> > > +          if ((zeroed_features & flag) != 0)
> > > +        {
> > > +          DWORD size = 0;
> > > +          void *loc = locate_xstate_feature (context, f, &size);
> > > +          if (loc != nullptr && size > 0)
> > > +            memset (loc, 0, size);
> > > +        }
> > > +        }
> > > +    }
> > >      });
> > >  }
> > >
> > > @@ -292,6 +315,17 @@ i386_prepare_to_resume (windows_thread_info
> > > *th)
> > >
> > >        th->debug_registers_changed = false;
> > >      }
> > > +
> > > +  windows_process.with_context (th, [&] (auto *context)
> > > +    {
> > > +      DWORD debug_registers = WindowsContext<decltype(context)>::debug;
> > > +      if (xstate_features != 0
> > > +      && (context->ContextFlags & ~debug_registers) != 0)
> > > +    {
> > > +      context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > > +      set_xstate_features_mask (context, xstate_features);
> > > +    }
> > > +    });
> > >  }
> > >
> > >  static void
> > > @@ -477,7 +511,7 @@ is_segment_register (int r)
> > >
> > >  template<typename Context>
> > >  static char *
> > > -get_context_reg_ptr (Context *context, int r)
> > > +get_context_reg_ptr (Context *context, int r, const target_desc
> > > +*tdesc)
> > >  {
> > >    const int *mappings;
> > >    int mappings_count;
> > > @@ -494,9 +528,21 @@ get_context_reg_ptr (Context *context, int r)
> > >        mappings_count = sizeof (i386_mappings) / sizeof (i386_mappings[0]);
> > >      }
> > >
> > > +  bool amd64 = register_size (tdesc, 0) == 8;
> >
> > There is already an " if (!windows_process.wow64_process)" a few lines above.
> > Wouldn't it make sense to move the "bool amd64" in the ifdef blocks and assign
> > accordingly?

Yes, I agree.


> > > +  int ymm0h_regnum;
> > > +  const int num_xmm_registers = amd64 ? 16 : 8;
> > > +
> > >    char *context_offset;
> > >    if (r < mappings_count)
> > >      context_offset = (char *) context + mappings[r];
> > > +  else if ((xstate_features & X86_XSTATE_AVX) != 0
> > > +      && r >= (ymm0h_regnum = find_regno (tdesc, "ymm0h"))
> > > +      && r < ymm0h_regnum + num_xmm_registers)
> > > +    {
> > > +      context_offset = (char *) locate_xstate_feature
> > > +    (context, X86_XSTATE_AVX_ID, NULL);
> > > +      context_offset += 16 * (r - ymm0h_regnum);
> > > +    }
> > >    else
> > >      gdb_assert_not_reached ("invalid register number %d", r);
> > >
> > > @@ -510,7 +556,7 @@ i386_fetch_inferior_register (struct regcache
> > > *regcache,  {
> > >    char *context_offset = windows_process.with_context (th, [&] (auto
> > > *context)
> > >      {
> > > -      return get_context_reg_ptr (context, r);
> > > +      return get_context_reg_ptr (context, r, regcache->tdesc);
> > >      });
> > >
> > >    /* GDB treats some registers as 32-bit, where they are in fact only
> > > @@ -538,7 +584,7 @@ i386_store_inferior_register (struct regcache
> > > *regcache,  {
> > >    char *context_offset = windows_process.with_context (th, [&] (auto
> > > *context)
> > >      {
> > > -      return get_context_reg_ptr (context, r);
> > > +      return get_context_reg_ptr (context, r, regcache->tdesc);
> > >      });
> > >
> > >    /* GDB treats some registers as 32-bit, where they are in fact only
> > > @@ -571,14 +617,17 @@ i386_arch_setup (void)  {
> > >  target_desc_up tdesc;
> > >
> > > +  DWORD64 xcr0 = xstate_features;
> > > +  if (xcr0 == 0)
> > > +    xcr0 = X86_XSTATE_SSE_MASK;
> > > +
> > >  #ifdef __x86_64__
> > > -  tdesc = amd64_create_target_description (X86_XSTATE_SSE_MASK, false,
> > > -                      false, false);
> > > +  tdesc = amd64_create_target_description (xcr0, false, false,
> > > + false);
> > >    init_target_desc (tdesc.get (), amd64_expedite_regs, WINDOWS_OSABI);
> > >    win32_tdesc = std::move (tdesc);
> > >  #endif
> > >
> > > -  tdesc = i386_create_target_description (X86_XSTATE_SSE_MASK, false,
> > > false);
> > > +  tdesc = i386_create_target_description (xcr0, false, false);
> > >    init_target_desc (tdesc.get (), i386_expedite_regs, WINDOWS_OSABI);
> > > #ifdef __x86_64__
> > >    wow64_win32_tdesc = std::move (tdesc); diff --git
> > > a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc index
> > > 7629beca213..5ccdc89a7ef 100644
> > > --- a/gdbserver/win32-low.cc
> > > +++ b/gdbserver/win32-low.cc
> > > @@ -33,6 +33,7 @@
> > >  #include <process.h>
> > >  #include "gdbsupport/gdb_tilde_expand.h"
> > >  #include "gdbsupport/common-inferior.h"
> > > +#include "tdesc.h"
> > >
> > >  using namespace windows_nat;
> > >
> > > @@ -426,8 +427,9 @@ child_fetch_inferior_registers (struct regcache
> > > *regcache, int r)
> > >    int regno;
> > >    windows_thread_info *th = windows_process.find_thread
> > > (current_thread-
> > > >id);
> > >    win32_require_context (th);
> > > -  if (r == -1 || r > NUM_REGS)
> > > -    child_fetch_inferior_registers (regcache, NUM_REGS);
> >
> > IIUC this was the only use of the NUM_REGS define.  We can remove it.
> 
> Yes, I agree. Would you mind explaining why we don't need this check anymore, too ? :)
> I don't understand it yet, unfortunately. Was it necessary before or is this due to AVX?

I don't think the check was necessary before AVX.
But with AVX it doesn't work, because NUM_REGS is the number of fix registers,
so for any AVX register it would go to the 'else' part.


> > Same for " i386_win32_num_regs (void)" and "aarch64_win32_num_regs ()".
> > This allows removing the num_regs hook in win32_target_ops.
> 
> > > +  if (r == -1)
> > > +    child_fetch_inferior_registers (regcache,
> > > +                    regcache->tdesc->reg_defs.size ());
> > >    else
> > >      for (regno = 0; regno < r; regno++)
> > >        (*the_low_target.fetch_inferior_register) (regcache, th,
> > > regno); @@ -441,8 +443,9 @@ child_store_inferior_registers (struct
> > > regcache *regcache, int r)
> > >    int regno;
> > >    windows_thread_info *th = windows_process.find_thread
> > > (current_thread-
> > > >id);
> > >    win32_require_context (th);
> > > -  if (r == -1 || r == 0 || r > NUM_REGS)
> > > -    child_store_inferior_registers (regcache, NUM_REGS);
> > > +  if (r == -1)
> > > +    child_store_inferior_registers (regcache,
> > > +                    regcache->tdesc->reg_defs.size ());
> 
> I assume removing r == 0 is not related to AVX register support and just a cleanup, right?
> Could we make it a separate cleanup patch (with reason)?
> 
> I know it seems like a super tiny nit, but it would help to understand why that kind of
> refactoring/cleanup is necessary (e.g. due to AVX or not).

I removed r == 0 because it wasn't there in child_fetch_inferior_registers
either, and I didn't like the this inconsistency.


> > >    else
> > >      for (regno = 0; regno < r; regno++)
> > >        (*the_low_target.store_inferior_register) (regcache, th,
> > > regno); @@ -1349,7 +1352,9 @@ void  initialize_low (void)  {
> > >    set_target_ops (&the_win32_target);
> > > -  the_low_target.arch_setup ();
> > >
> > >    initialize_loadable ();
> > > +  /* Has to be done after initialize_loadable, because it uses the xstate
> > > +    functions if available.  */
> > > +  the_low_target.arch_setup ();
> > >  }
> > > --
> > > 2.54.0
> 
> Christina


Thanks
Hannes

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

[Joos, Christina]

> -----Original Message-----
> From: Hannes Domani <ssbssa@yahoo.de>
> Sent: Donnerstag, 10. September 2026 16:39
> To: Rohr, Stephan <stephan.rohr@intel.com>; gdb-patches@sourceware.org;
> Joos, Christina <christina.joos@intel.com>
> Cc: Tom Tromey <tom@tromey.com>
> Subject: Re: [PATCH v3 8/8] Windows gdb: Implement AVX-512 register
> support
> 
>  Am Donnerstag, 10. September 2026 um 13:33:20 MESZ hat Joos, Christina
> <christina.joos@intel.com> Folgendes geschrieben:
> 
> > Hi Hannes,
> >
> > I saw that Stephan already reviewed this (thanks!).
> > I added my remarks on top, see below.
> >
> > > -----Original Message-----
> > > From: Rohr, Stephan <stephan.rohr@intel.com>
> > > Sent: Dienstag, 8. September 2026 15:05
> > > To: Hannes Domani <ssbssa@yahoo.de>; gdb-patches@sourceware.org;
> > > gdb- patches@sourceware.org
> > > Cc: Joos, Christina <christina.joos@intel.com>; Tom Tromey
> > > <tom@tromey.com>
> > > Subject: RE: [PATCH v3 7/8] Windows gdb: Implement AVX register
> > > support
> > >
> > > Hi Hannes,
> > >
> > > please see some feedback inlined below.
> > >
> > > Let me know if you have any questions.
> > >
> > > 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 7/8] Windows gdb: Implement AVX register
> > > > support
> > > >
> > > > This adds support for the Intel AVX registers on Windows.
> > > > It enables accessing registers $ymm0 - $ymm15 where they are available.
> > > >
> > > > After this patch gdb.arch/i386-avx.exp passes on windows.
> > > > ---
> > > > v3:
> > > >  - merged gdb+gdbserver parts, and split again AVX/AVX-512 parts
> > > > ---
> > > >  gdb/NEWS                    |  3 ++
> > > >  gdb/nat/windows-nat.c      |  2 +-
> > > >  gdb/x86-windows-nat.c      | 67
> > > > +++++++++++++++++++++++++++++++++++--
> > > >  gdbserver/win32-i386-low.cc | 61
> > > >+++++++++++++++++++++++++++++----
> > > >  gdbserver/win32-low.cc      | 15 ++++++---
> > > >  5 files changed, 133 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/gdb/NEWS b/gdb/NEWS
> > > > index 10c182067f9..f7effc822e9 100644
> > > > --- a/gdb/NEWS
> > > > +++ b/gdb/NEWS
> > > > @@ -118,6 +118,9 @@
> > > >    intent to remove it in a future release.
> > > >    The s390 64-bit target (s390x-*) remains supported.
> > > >
> > > > +* Support for Intel AVX registers on Windows.
> > > > +  Support displaying and modifying Intel AVX registers $ymm0 -
> $ymm31.
> > > > +
> > >
> > > I think this should be registers $ymm0 - $ymm15 ?
> >
> > Yes, I agree with Stephan's feedback here.
> > The AVX state only comprises only YMM0–YMM15 for 64 bit.
> > For 32-bit mode, it's YMM0- YMM7 only.
> > See the docs added in commit "Add org.gnu.gdb.i386.avx."
> 
> Yes, I missed this when I split AVX/AVX-512.
> 
> 
> > > >  * Configure changes
> > > >
> > > >  ** --with-babeltrace has been removed.  The babeltrace library
> > > >was  diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
> > > >index  8930536f3ba..c9a21d7c41f 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_SSE_MASK);
> > > > +            & X86_XSTATE_AVX_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
> > > >3af5ef4dae0..1cefe6171be 100644
> > > > --- a/gdb/x86-windows-nat.c
> > > > +++ b/gdb/x86-windows-nat.c
> > > > @@ -27,6 +27,9 @@
> > > >
> > > >  #include "i386-tdep.h"
> > > >  #include "i387-tdep.h"
> > > > +#ifdef __x86_64__
> > > > +#include "amd64-tdep.h"
> > > > +#endif
> > > >
> > > >  using namespace windows_nat;
> > > >
> > > > @@ -70,6 +73,8 @@ struct x86_windows_nat_target final : public
> > > >x86_nat_target<windows_nat_target>
> > > >                windows_thread_info *th, int r) override;
> > > >
> > > >    bool is_sw_breakpoint (const EXCEPTION_RECORD *er) const
> > > >override;
> > > > +
> > > > +  const struct target_desc *read_description () override;
> > > >  };
> > > >
> > > >  /* The current process.  */
> > > > @@ -109,7 +114,31 @@ x86_windows_per_inferior::fill_thread_context
> > > > (windows_thread_info *th)
> > > >        if (context->ContextFlags == 0)
> > > >      {
> > > >        context->ContextFlags =
> > > >WindowsContext<decltype(context)>::all;
> > > > +      if (xstate_features != 0)
> > > > +        {
> > > > +          context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > > > +          set_xstate_features_mask (context, xstate_features);
> > > > +        }
> > >
> > > We have the same code in "i386_get_thread_context" in "win32-i386-
> low.cc".
> > > Make a shared function in gdb/nat/windows-nat.h?
> > >
> > > >        CHECK (get_thread_context (th->h, context));
> > > > +
> > > > +      if (xstate_features != 0)
> > > > +        {
> > > > +          DWORD64 features = 0;
> > > > +          CHECK (get_xstate_features_mask (context, &features));
> > >
> > > Should this be changed to sth. like
> > >
> > >  if (!get_xstate_features_mask (context, &features))
> > >    {
> > >      warning (..)
> > >      return;
> > >    }
> > >
> > > The call of "CHECK" only prints a message but doesn't error out.  If
> > > this call fails we may still have features == 0.  This implies
> > > "zeroed_features == xstate_features".  With this, the loop clears all features.
> > > IIUC, this would clear the AVX registers on the next call of
> "SetThreadContext".
> > >
> > > Also refer to the implementation in gdbserver/win32-i386-low.cc:
> > >
> > >      DWORD64 features = 0;
> > >      if (xstate_features != 0
> > >       && get_xstate_features_mask (context, &features))
> > >     {
> > >
> > > I think it makes sense to unify those as the rest of the code is basically
> identical.
> > > Put shared function into gdb/nat/windows-nat.h?  This keeps the code
> > > consistent.
> 
> I will try that.
> 
> 
> > > > +          DWORD64 zeroed_features = xstate_features & ~features;
> > > > +
> > > > +          for (int f = X86_XSTATE_AVX_ID; f <=
> > > > +X86_XSTATE_CET_U_ID; f++)
> >
> > In addition to Stephan's feedback for this code area:
> > Since we decided to not add CET, and this patch is even for AVX only for now
> only, do we need a loop at this point already?
> >
> > For the follow up AVX-512 patch I think we can stop at the highest supported
> feature (AVX-512) in windows, can't we?
> 
> I would prefer it if we could keep it a loop, even if it's only X86_XSTATE_AVX_ID
> in the AVX patch.
> And yes, I forgot to change it to X86_XSTATE_ZMM_ID when I removed the CET
> stuff.
> 
> 
> > > > +          DWORD64 flag = 1ULL << f;
> > > > +          if ((zeroed_features & flag) != 0)
> > > > +            {
> > > > +              DWORD size = 0;
> > > > +              void *loc = locate_xstate_feature (context, f,
> > > > +&size);
> > > > +              if (loc != nullptr && size > 0)
> > > > +            memset (loc, 0, size);
> > > > +            }
> > > > +        }
> > > > +        }
> > > >      }
> > > >      });
> > > >  }
> > > > @@ -198,6 +227,14 @@
> > > > x86_windows_nat_target::thread_context_continue
> > > >(windows_thread_info  *th,
> > > >        if (GetExitCodeThread (th->h, &ec)
> > > >            && ec == STILL_ACTIVE)
> > > >          {
> > > > +          DWORD debug_registers =
> > > > WindowsContext<decltype(context)>::debug;
> > > > +          if (xstate_features != 0
> > > > +          && (context->ContextFlags & ~debug_registers) != 0)
> > > > +        {
> > > > +          context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > > > +          set_xstate_features_mask (context, xstate_features);
> > > > +        }
> > > > +
> > > >            BOOL status = set_thread_context (th->h, context);
> > > >
> > > >            if (!killed)
> > > > @@ -227,7 +264,7 @@ x86_windows_nat_target::thread_context_step
> > > > (windows_thread_info *th,
> > > >
> > > >  template<typename Context>
> > > >  static char *
> > > > -get_context_reg_ptr (Context *context, int r)
> > > > +get_context_reg_ptr (Context *context, int r, i386_gdbarch_tdep
> > > > +*tdep)
> > > >  {
> > > >    const int *mappings;
> > > >    int mappings_count;
> > > > @@ -247,6 +284,13 @@ get_context_reg_ptr (Context *context, int r)
> > > >    char *context_offset;
> > > >    if (r < mappings_count)
> > > >      context_offset = (char *) context + mappings[r];
> > > > +  else if (I387_YMM0H_REGNUM (tdep) > 0 && r >=
> > > I387_YMM0H_REGNUM
> > > > (tdep)
> > > > +      && r < I387_YMMENDH_REGNUM (tdep))
> > >
> > > The implementation on gdbserver side guards against
> > >
> > >  xstate_features & X86_XSTATE_AVX) != 0
> > >
> > > I wonder if the same guard would be helpful here, too.  I understand
> > > the register number is initialized to -1, so this should not fire.
> > > I'm not sure if it is possible to have $ymm0 register number > 0 w/o
> > > xstate support, e.g., if the target description is read from file, see
> "target_find_description"?
> 
> On gdbserver side I needed the guard because there is no register number I
> could check.
> I didn't think it was necessary on gdb side as well, but if you prefer it like this, I
> will add it.
> 
> 
> > > > +    {
> > > > +      context_offset = (char *) locate_xstate_feature
> > > > +    (context, X86_XSTATE_AVX_ID, NULL);
> >
> > We prefer to use nullptr.
> 
> Right.
> 
> 
> > > > +      context_offset += 16 * (r - I387_YMM0H_REGNUM (tdep));
> > > > +    }
> > > >    else
> > > >      gdb_assert_not_reached ("invalid register number %d", r);
> > > >
> > > > @@ -267,7 +311,7 @@ x86_windows_nat_target::fetch_one_register
> > > >(struct  regcache *regcache,
> > > >    char *context_offset
> > > >      = x86_windows_process.with_context (th, [&] (auto *context)
> > > >      {
> > > > -      return get_context_reg_ptr (context, r);
> > > > +      return get_context_reg_ptr (context, r, tdep);
> > > >      });
> > > >
> > > >    gdb_assert (!gdbarch_read_pc_p (gdbarch)); @@ -333,7 +377,7 @@
> > > >x86_windows_nat_target::store_one_register (const struct regcache
> > > >*regcache,
> > > >      = x86_windows_process.with_context (th, [&] (auto *context)
> > > >      {
> > > >        gdb_assert (context->ContextFlags != 0);
> > > > -      return get_context_reg_ptr (context, r);
> > > > +      return get_context_reg_ptr (context, r, tdep);
> > > >      });
> > > >
> > > >    /* GDB treats some registers as 32-bit, where they are in fact
> > > >only  @@ -368,6 +412,23 @@
> x86_windows_nat_target::is_sw_breakpoint
> > > >(const  EXCEPTION_RECORD *er) const
> > > >        || er->ExceptionCode == STATUS_WX86_BREAKPOINT);  }
> > > >
> > > > +const struct target_desc *
> > > > +x86_windows_nat_target::read_description () {
> > > > +  if (inferior_ptid == null_ptid)
> > > > +    return this->beneath ()->read_description ();
> > > > +
> > > > +  if (xstate_features == 0)
> > > > +    return nullptr;
> > > > +
> > > > +#ifdef __x86_64__
> > > > +  if (!x86_windows_process.wow64_process)
> > > > +    return amd64_target_description (xstate_features, false);
> > > > +  else
> > > > +#endif
> > > > +    return i386_target_description (xstate_features, false); }
> > > > +
> > > >  /* Hardware watchpoint support, adapted from go32-nat.c code.  */
> > > >
> > > >  /* Pass the address ADDR to the inferior in the I'th debug register.
> > > > diff --git a/gdbserver/win32-i386-low.cc
> > > >b/gdbserver/win32-i386-low.cc  index b77f6adc6ed..a7e83c0239c
> > > >100644
> > > > --- a/gdbserver/win32-i386-low.cc
> > > > +++ b/gdbserver/win32-i386-low.cc
> > > > @@ -253,6 +253,11 @@ i386_get_thread_context (windows_thread_info
> > > > *th)
> > > >                    | WindowsContext<decltype(context)>::floating
> > > >                    | WindowsContext<decltype(context)>::debug
> > > >                    | extended_registers);
> > > > +      if (xstate_features != 0)
> > > > +    {
> > > > +      context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > > > +      set_xstate_features_mask (context, xstate_features);
> > > > +    }
> > > >
> > > >        BOOL ret = get_thread_context (th->h, context);
> > > >        if (!ret)
> > > > @@ -267,6 +272,24 @@ i386_get_thread_context (windows_thread_info
> > > > *th)
> > > >
> > > >        error (_("GetThreadContext failure %ld\n"), (long) e);
> > > >      }
> > > > +
> > > > +      DWORD64 features = 0;
> > > > +      if (xstate_features != 0
> > > > +      && get_xstate_features_mask (context, &features))
> > > > +    {
> > > > +      DWORD64 zeroed_features = xstate_features & ~features;
> > > > +      for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_CET_U_ID;
> > > > +f++)
> > > > +        {
> > > > +          DWORD64 flag = 1ULL << f;
> > > > +          if ((zeroed_features & flag) != 0)
> > > > +        {
> > > > +          DWORD size = 0;
> > > > +          void *loc = locate_xstate_feature (context, f, &size);
> > > > +          if (loc != nullptr && size > 0)
> > > > +            memset (loc, 0, size);
> > > > +        }
> > > > +        }
> > > > +    }
> > > >      });
> > > >  }
> > > >
> > > > @@ -292,6 +315,17 @@ i386_prepare_to_resume
> (windows_thread_info
> > > > *th)
> > > >
> > > >        th->debug_registers_changed = false;
> > > >      }
> > > > +
> > > > +  windows_process.with_context (th, [&] (auto *context)
> > > > +    {
> > > > +      DWORD debug_registers =
> > > > +WindowsContext<decltype(context)>::debug;
> > > > +      if (xstate_features != 0
> > > > +      && (context->ContextFlags & ~debug_registers) != 0)
> > > > +    {
> > > > +      context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > > > +      set_xstate_features_mask (context, xstate_features);
> > > > +    }
> > > > +    });
> > > >  }
> > > >
> > > >  static void
> > > > @@ -477,7 +511,7 @@ is_segment_register (int r)
> > > >
> > > >  template<typename Context>
> > > >  static char *
> > > > -get_context_reg_ptr (Context *context, int r)
> > > > +get_context_reg_ptr (Context *context, int r, const target_desc
> > > > +*tdesc)
> > > >  {
> > > >    const int *mappings;
> > > >    int mappings_count;
> > > > @@ -494,9 +528,21 @@ get_context_reg_ptr (Context *context, int r)
> > > >        mappings_count = sizeof (i386_mappings) / sizeof
> > > >(i386_mappings[0]);
> > > >      }
> > > >
> > > > +  bool amd64 = register_size (tdesc, 0) == 8;
> > >
> > > There is already an " if (!windows_process.wow64_process)" a few lines
> above.
> > > Wouldn't it make sense to move the "bool amd64" in the ifdef blocks
> > > and assign accordingly?
> 
> Yes, I agree.
> 
> 
> > > > +  int ymm0h_regnum;
> > > > +  const int num_xmm_registers = amd64 ? 16 : 8;
> > > > +
> > > >    char *context_offset;
> > > >    if (r < mappings_count)
> > > >      context_offset = (char *) context + mappings[r];
> > > > +  else if ((xstate_features & X86_XSTATE_AVX) != 0
> > > > +      && r >= (ymm0h_regnum = find_regno (tdesc, "ymm0h"))
> > > > +      && r < ymm0h_regnum + num_xmm_registers)
> > > > +    {
> > > > +      context_offset = (char *) locate_xstate_feature
> > > > +    (context, X86_XSTATE_AVX_ID, NULL);
> > > > +      context_offset += 16 * (r - ymm0h_regnum);
> > > > +    }
> > > >    else
> > > >      gdb_assert_not_reached ("invalid register number %d", r);
> > > >
> > > > @@ -510,7 +556,7 @@ i386_fetch_inferior_register (struct regcache
> > > >*regcache,  {
> > > >    char *context_offset = windows_process.with_context (th, [&]
> > > >(auto
> > > > *context)
> > > >      {
> > > > -      return get_context_reg_ptr (context, r);
> > > > +      return get_context_reg_ptr (context, r, regcache->tdesc);
> > > >      });
> > > >
> > > >    /* GDB treats some registers as 32-bit, where they are in fact
> > > >only  @@ -538,7 +584,7 @@ i386_store_inferior_register (struct
> > > >regcache  *regcache,  {
> > > >    char *context_offset = windows_process.with_context (th, [&]
> > > >(auto
> > > > *context)
> > > >      {
> > > > -      return get_context_reg_ptr (context, r);
> > > > +      return get_context_reg_ptr (context, r, regcache->tdesc);
> > > >      });
> > > >
> > > >    /* GDB treats some registers as 32-bit, where they are in fact
> > > >only  @@ -571,14 +617,17 @@ i386_arch_setup (void)  {
> > > >  target_desc_up tdesc;
> > > >
> > > > +  DWORD64 xcr0 = xstate_features;
> > > > +  if (xcr0 == 0)
> > > > +    xcr0 = X86_XSTATE_SSE_MASK;
> > > > +
> > > >  #ifdef __x86_64__
> > > > -  tdesc = amd64_create_target_description (X86_XSTATE_SSE_MASK,
> > > >false,
> > > > -                      false, false);
> > > > +  tdesc = amd64_create_target_description (xcr0, false, false,
> > > > +false);
> > > >    init_target_desc (tdesc.get (), amd64_expedite_regs,
> > > >WINDOWS_OSABI);
> > > >    win32_tdesc = std::move (tdesc);
> > > >  #endif
> > > >
> > > > -  tdesc = i386_create_target_description (X86_XSTATE_SSE_MASK,
> > > > false, false);
> > > > +  tdesc = i386_create_target_description (xcr0, false, false);
> > > >    init_target_desc (tdesc.get (), i386_expedite_regs,
> > > >WINDOWS_OSABI);  #ifdef __x86_64__
> > > >    wow64_win32_tdesc = std::move (tdesc); diff --git
> > > >a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc index
> > > >7629beca213..5ccdc89a7ef 100644
> > > > --- a/gdbserver/win32-low.cc
> > > > +++ b/gdbserver/win32-low.cc
> > > > @@ -33,6 +33,7 @@
> > > >  #include <process.h>
> > > >  #include "gdbsupport/gdb_tilde_expand.h"
> > > >  #include "gdbsupport/common-inferior.h"
> > > > +#include "tdesc.h"
> > > >
> > > >  using namespace windows_nat;
> > > >
> > > > @@ -426,8 +427,9 @@ child_fetch_inferior_registers (struct
> > > >regcache  *regcache, int r)
> > > >    int regno;
> > > >    windows_thread_info *th = windows_process.find_thread
> > > > (current_thread-
> > > > >id);
> > > >    win32_require_context (th);
> > > > -  if (r == -1 || r > NUM_REGS)
> > > > -    child_fetch_inferior_registers (regcache, NUM_REGS);
> > >
> > > IIUC this was the only use of the NUM_REGS define.  We can remove it.
> >
> > Yes, I agree. Would you mind explaining why we don't need this check
> > anymore, too ? :) I don't understand it yet, unfortunately. Was it necessary
> before or is this due to AVX?
> 
> I don't think the check was necessary before AVX.
> But with AVX it doesn't work, because NUM_REGS is the number of fix
> registers, so for any AVX register it would go to the 'else' part.
> 
> 
> > > Same for " i386_win32_num_regs (void)" and "aarch64_win32_num_regs
> ()".
> > > This allows removing the num_regs hook in win32_target_ops.
> >
> > > > +  if (r == -1)
> > > > +    child_fetch_inferior_registers (regcache,
> > > > +                    regcache->tdesc->reg_defs.size ());
> > > >    else
> > > >      for (regno = 0; regno < r; regno++)
> > > >        (*the_low_target.fetch_inferior_register) (regcache, th,
> > > >regno); @@ -441,8 +443,9 @@ child_store_inferior_registers (struct
> > > >regcache *regcache, int r)
> > > >    int regno;
> > > >    windows_thread_info *th = windows_process.find_thread
> > > > (current_thread-
> > > > >id);
> > > >    win32_require_context (th);
> > > > -  if (r == -1 || r == 0 || r > NUM_REGS)
> > > > -    child_store_inferior_registers (regcache, NUM_REGS);
> > > > +  if (r == -1)
> > > > +    child_store_inferior_registers (regcache,
> > > > +                    regcache->tdesc->reg_defs.size ());
> >
> > I assume removing r == 0 is not related to AVX register support and just a
> cleanup, right?
> > Could we make it a separate cleanup patch (with reason)?
> >
> > I know it seems like a super tiny nit, but it would help to understand
> > why that kind of refactoring/cleanup is necessary (e.g. due to AVX or not).
> 
> I removed r == 0 because it wasn't there in child_fetch_inferior_registers either,
> and I didn't like the this inconsistency.

Ok, any change which is not related to the AVX patch should be in a separate
cleanup patch.

Thanks,
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.

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

[Hannes Domani]

 I'm not sure what happened, but I replied earlier to 8/8 already, but
with the contents to my reply for 7/8...


Am Donnerstag, 10. September 2026 um 14:08:10 MESZ hat Joos, Christina <christina.joos@intel.com> Folgendes geschrieben:

> Hi Hannes,
> 
> Thank you for this patch and your work on this.
> 
> Please find my comments on top of Stephan's feedback.
> 
> > -----Original Message-----
> > From: Rohr, Stephan <stephan.rohr@intel.com>
> > Sent: Dienstag, 8. September 2026 15:05
> > To: Hannes Domani <ssbssa@yahoo.de>; gdb-patches@sourceware.org
> > Cc: Joos, Christina <christina.joos@intel.com>; Tom Tromey
> > <tom@tromey.com>
> > Subject: RE: [PATCH v3 8/8] Windows gdb: Implement AVX-512 register support
> >
> > 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.
> 
> Suggestion:
> For the linux side we have this commit to enable AMX-512:
> " Add AVX512 registers support to GDB and GDBserver."
> 
> It includes a description which registers are added, extended etc:
> 
> " Intel(R) AVX-512 is an extension to AVX to support 512-bit wide
> SIMD registers in 64-bit mode (XMM0-XMM31, YMM0-YMM31, ZMM0-ZMM31).
> The number of available registers in 32-bit mode is still 8
> (XMM0-7, YMM0-7, ZMM0-7).  The lower 256-bits of the ZMM registers
> are aliased to the respective 256-bit YMM registers.  The lower
> 128-bits are aliased to the respective 128-bit XMM registers.
> 
> There are also 8 new, dedicated mask registers (K0-K7) in both 32-bit
> mode and 64-bit mode."
> 
> Maybe you could reference it, something like
> "similar to commit xyz for linux this patch adds [...] for 32 and 64-bit mode in windows." ?
> 
> Then you have all the details described, if one would like to look it up to understand your
> patch in detail. But you don't have to repeat it in your own commit message.
> 
> What do you think?

Sounds good.


> > > 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?

Right.


> > > +  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);
> > > +    }
> 
> Similar comment  to the previous patch for nullptr here and below.
> 
> > > +  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.
> 
> I have a similar comment on this, but for the offsets:
> 
> Those offsets are similar in gdbserver/i387-fp.cc and gdb/i387-tdep.c, but we
> cannot use/share the complete logic for windows gdb & gdbserver I believe.
> 
> However, the offsets inside one feature (I don't mean the offsets between two
> consecutive features) should be the same for any OS and independent of AMD or Intel.
> I believe it would be nice if we could at least share the offset calculations at some point
> maybe in gdbsupport/* sometime in future...
> But this is nothing that you must do as part of this patch of course.
> 
> For this patch:
> I don't think this is strictly necessary, but we could make this code a bit more generic inside each individual file on gdb & gdbserver side.
> For example, we could introduce some static helpers or similar, that are called in each else if.
> What do you think?
> 
> Christina

I was also thinking that these look-ups are not ideal.
As I understand it, the offsets for each feature are not fixed values on
windows, but they should be the same for all threads on a system.
In that case, maybe we could calculate these offsets for all registers
once, put them in a vector, and use that in get_context_reg_ptr?
I'm also assuming that the register numbers always stay the same.
I'm just not sure at which point the offset calculation should be done.
Any suggestions?


Hannes

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

[Joos, Christina]

> -----Original Message-----
> From: Hannes Domani <ssbssa@yahoo.de>
> Sent: Donnerstag, 10. September 2026 18:54
> To: Rohr, Stephan <stephan.rohr@intel.com>; gdb-patches@sourceware.org;
> Joos, Christina <christina.joos@intel.com>
> Cc: Tom Tromey <tom@tromey.com>
> Subject: Re: [PATCH v3 8/8] Windows gdb: Implement AVX-512 register
> support
> 
>  I'm not sure what happened, but I replied earlier to 8/8 already, but with the
> contents to my reply for 7/8...
> 
> 
> Am Donnerstag, 10. September 2026 um 14:08:10 MESZ hat Joos, Christina
> <christina.joos@intel.com> Folgendes geschrieben:
> 
> > Hi Hannes,
> >
> > Thank you for this patch and your work on this.
> >
> > Please find my comments on top of Stephan's feedback.
> >
> > > -----Original Message-----
> > > From: Rohr, Stephan <stephan.rohr@intel.com>
> > > Sent: Dienstag, 8. September 2026 15:05
> > > To: Hannes Domani <ssbssa@yahoo.de>; gdb-patches@sourceware.org
> > > Cc: Joos, Christina <christina.joos@intel.com>; Tom Tromey
> > > <tom@tromey.com>
> > > Subject: RE: [PATCH v3 8/8] Windows gdb: Implement AVX-512 register
> > > support
> > >
> > > 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.
> >
> > Suggestion:
> > For the linux side we have this commit to enable AMX-512:
> > " Add AVX512 registers support to GDB and GDBserver."
> >
> > It includes a description which registers are added, extended etc:
> >
> > " Intel(R) AVX-512 is an extension to AVX to support 512-bit wide SIMD
> > registers in 64-bit mode (XMM0-XMM31, YMM0-YMM31, ZMM0-ZMM31).
> > The number of available registers in 32-bit mode is still 8 (XMM0-7,
> > YMM0-7, ZMM0-7).  The lower 256-bits of the ZMM registers are aliased
> > to the respective 256-bit YMM registers.  The lower 128-bits are
> > aliased to the respective 128-bit XMM registers.
> >
> > There are also 8 new, dedicated mask registers (K0-K7) in both 32-bit
> > mode and 64-bit mode."
> >
> > Maybe you could reference it, something like "similar to commit xyz
> > for linux this patch adds [...] for 32 and 64-bit mode in windows." ?
> >
> > Then you have all the details described, if one would like to look it
> > up to understand your patch in detail. But you don't have to repeat it in your
> own commit message.
> >
> > What do you think?
> 
> Sounds good.
> 
> 
> > > > 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?
> 
> Right.
> 
> 
> > > > +  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);
> > > > +    }
> >
> > Similar comment  to the previous patch for nullptr here and below.
> >
> > > > +  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.
> >
> > I have a similar comment on this, but for the offsets:
> >
> > Those offsets are similar in gdbserver/i387-fp.cc and gdb/i387-tdep.c,
> > but we cannot use/share the complete logic for windows gdb & gdbserver I
> believe.
> >
> > However, the offsets inside one feature (I don't mean the offsets
> > between two consecutive features) should be the same for any OS and
> independent of AMD or Intel.
> > I believe it would be nice if we could at least share the offset
> > calculations at some point maybe in gdbsupport/* sometime in future...
> > But this is nothing that you must do as part of this patch of course.
> >
> > For this patch:
> > I don't think this is strictly necessary, but we could make this code a bit more
> generic inside each individual file on gdb & gdbserver side.
> > For example, we could introduce some static helpers or similar, that are
> called in each else if.
> > What do you think?
> >
> > Christina
> 
> I was also thinking that these look-ups are not ideal.
> As I understand it, the offsets for each feature are not fixed values on windows,
> but they should be the same for all threads on a system.

I don't understand which offsets you mean here. 
The ones you hardcode (1) or the context_offset returned by locate_xstate_feature (2)?
My feedback in this email is only for the hardcoded ones.

For (2) I replied in a different email thread:
https://sourceware.org/pipermail/gdb-patches/2026-September/230194.html

> In that case, maybe we could calculate these offsets for all registers once, put
> them in a vector, and use that in get_context_reg_ptr?
> I'm also assuming that the register numbers always stay the same.
> I'm just not sure at which point the offset calculation should be done.
> Any suggestions?

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.