public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v5 0/7] Support for recording some AVX instructions
@ 2024-10-23 16:51 Guinevere Larsen
  2024-10-23 16:51 ` [PATCH v5 1/7] gdb: Allow replayed threads to read and write pseudo registers Guinevere Larsen
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Guinevere Larsen @ 2024-10-23 16:51 UTC (permalink / raw)
  To: gdb-patches; +Cc: Guinevere Larsen

This patch is the third version of adding support to record and replay
AVX instructions on x86 and x86_64 CPUs. This version has one more
preparatory patch, as the record subsystem could read pseudo registers
(like ymm). After this patch series, GDB should be able to fully record
__memset_avx2_unaligned_erms from glibc!

For rationale, supporting AVX instructions is important since glibc
has been shipping functions that use them for a long time, meaning
most standard library functions would not be supported by record,
which is a big inconvenience for end users.

The first patch of the series makes GDB aware that it can read pseudo
registers freom executing and replaying threads, no user-visible
changes expected. The second adds the capability for the main i386 record
function to identify and read VEX prefixes, but doesn't add support for
any instruction yet, so minimal user visible changes (only a slight
tweak to an error message). The remaining patches add support to related
instruction, and tests those instructions.

Since this series only supports a very small subset of avx instructions,
I am not sure if I should add a NEWS entry (and what to put in it if I
should).

Changes for v5:
* the new function record_full_is_replaying now uses dynamic_cast to
  check if the record-full target is enabled.

Guinevere Larsen (7):
  gdb: Allow replayed threads to read and write pseudo registers
  gdb: Start supporting AVX instruction
  gdb/record: add support to vmovd and vmovq instructions
  gdb/record: add support to AVX unpack instructions
  gdb/record: Add recording support to vpbroadcast instructions
  gdb/record: support AVX instructions VMOVDQ(U|A) when recording
  gdb/record: add support to vzeroupper instruction

 gdb/amd64-tdep.c                              |   3 +-
 gdb/i386-tdep.c                               | 268 +++++++++++++-
 gdb/i386-tdep.h                               |   1 +
 gdb/record-full.c                             |  11 +-
 gdb/record-full.h                             |   3 +
 gdb/testsuite/gdb.reverse/i386-avx-reverse.c  | 234 ++++++++++++
 .../gdb.reverse/i386-avx-reverse.exp          | 333 ++++++++++++++++++
 gdb/testsuite/gdb.reverse/step-precsave.exp   |   4 +-
 gdb/thread.c                                  |  15 +-
 9 files changed, 864 insertions(+), 8 deletions(-)
 create mode 100644 gdb/testsuite/gdb.reverse/i386-avx-reverse.c
 create mode 100644 gdb/testsuite/gdb.reverse/i386-avx-reverse.exp

-- 
2.47.0


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

* [PATCH v5 1/7] gdb: Allow replayed threads to read and write pseudo registers
  2024-10-23 16:51 [PATCH v5 0/7] Support for recording some AVX instructions Guinevere Larsen
@ 2024-10-23 16:51 ` Guinevere Larsen
  2024-10-23 16:51 ` [PATCH v5 2/7] gdb: Start supporting AVX instruction Guinevere Larsen
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Guinevere Larsen @ 2024-10-23 16:51 UTC (permalink / raw)
  To: gdb-patches; +Cc: Guinevere Larsen

In an effort to support AVX instructions when recording, we need to
allow replaying threads to access pseudo registers. Currently, if
we try to do that gdb will fail in a call to validate_registers_access,
because the thread is executing so GDB thinks it is unsafe to read
pseudo registers.

When replaying, the thread is really executing for all intents and
purposes, but the execution is just having GDB change values on
registers, so it will always be safe to read and write pseudo registers.
This commit changes functions that check for register access to allow
access when we are replaying. The check to whether we are replaying must
not happen when writing a core file, as record_full_list could be nullptr,
so we only check it if the thread is executing.

As of this commit, I don't know of a way to trigger this commit without
AVX support on record, so a test isn't provided. However, as soon as
record-full supports saving ymm registers, the AVX tests will test this
as well.
---
 gdb/record-full.c |  9 +++++++++
 gdb/record-full.h |  3 +++
 gdb/thread.c      | 15 +++++++++++++--
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/gdb/record-full.c b/gdb/record-full.c
index a681ef9fe51..074d0cd63fc 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -370,6 +370,15 @@ record_full_is_used (void)
 	  || t == &record_full_core_ops);
 }
 
+/* see record-full.h.  */
+bool
+record_full_is_replaying ()
+{
+  auto target = dynamic_cast<record_full_target *>
+		  (current_inferior ()->target_at (record_stratum));
+  return target != nullptr && RECORD_FULL_IS_REPLAY;
+}
+
 
 /* Command lists for "set/show record full".  */
 static struct cmd_list_element *set_record_full_cmdlist;
diff --git a/gdb/record-full.h b/gdb/record-full.h
index e8eb041311b..07f97d241a2 100644
--- a/gdb/record-full.h
+++ b/gdb/record-full.h
@@ -31,6 +31,9 @@ extern int record_full_arch_list_add_end (void);
 /* Returns true if the process record target is open.  */
 extern int record_full_is_used (void);
 
+/* Whether the inferior is being replayed, or is executing normally.  */
+extern bool record_full_is_replaying ();
+
 extern scoped_restore_tmpl<int> record_full_gdb_operation_disable_set ();
 
 #endif /* RECORD_FULL_H */
diff --git a/gdb/thread.c b/gdb/thread.c
index 4ee46936861..5892b158603 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -50,6 +50,7 @@
 #include "inline-frame.h"
 #include "stack.h"
 #include "interps.h"
+#include "record-full.h"
 
 /* See gdbthread.h.  */
 
@@ -998,7 +999,13 @@ validate_registers_access (void)
      reason, but is marked running from the user's perspective.  E.g.,
      the thread is waiting for its turn in the step-over queue.  */
   if (tp->executing ())
-    error (_("Selected thread is running."));
+    {
+      /* If we are replaying with the record-full subsystem, even though
+	 the thread is executing, it is always safe to read from it since
+	 replay execution is just GDB reading and writing to a regcache.  */
+      if (!record_full_is_replaying ())
+	error (_("Selected thread is running."));
+    }
 }
 
 /* See gdbthread.h.  */
@@ -1016,7 +1023,11 @@ can_access_registers_thread (thread_info *thread)
 
   /* ... or from a spinning thread.  FIXME: see validate_registers_access.  */
   if (thread->executing ())
-    return false;
+    {
+      /* See validate_registers_access.  */
+      if (!record_full_is_replaying ())
+	return false;
+    }
 
   return true;
 }
-- 
2.47.0


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

* [PATCH v5 2/7] gdb: Start supporting AVX instruction
  2024-10-23 16:51 [PATCH v5 0/7] Support for recording some AVX instructions Guinevere Larsen
  2024-10-23 16:51 ` [PATCH v5 1/7] gdb: Allow replayed threads to read and write pseudo registers Guinevere Larsen
@ 2024-10-23 16:51 ` Guinevere Larsen
  2024-10-23 16:51 ` [PATCH v5 3/7] gdb/record: add support to vmovd and vmovq instructions Guinevere Larsen
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Guinevere Larsen @ 2024-10-23 16:51 UTC (permalink / raw)
  To: gdb-patches; +Cc: Guinevere Larsen

This patch introduces the information needed to properly identify the
VEX prefix, used to signal an AVX and AVX2 instruction, and introduces
a helper function to handle all AVX instruction, instead of adding to
the 3000 line long recording function.

This new function will temporarily set the current thread as "not
executing" so that it can read from pseudo registers as we record, since
most AVX/AVX2 instructions would benefit from recording ymm registers.

The new helper also handles unsupported instructions so that the largest
part of the i386_process_record doesn't have to be shifted by 2 spaces,
which made an unreadably big patch file.

The only expected difference to the end user added by this patch is a
small change to the unsupported message. This patch also updates the
test gdb.reverse/step-precsave.exp, by recognizing the new output.

As a note for the future, we don't handle xmm16-31 and ymm16-31 because
those require the EVEX prefix, meaning avx512 support.
---
 gdb/amd64-tdep.c                            |   3 +-
 gdb/i386-tdep.c                             | 103 +++++++++++++++++++-
 gdb/i386-tdep.h                             |   1 +
 gdb/testsuite/gdb.reverse/step-precsave.exp |   2 +-
 4 files changed, 105 insertions(+), 4 deletions(-)

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index b63e35d522a..c17b5f7f6f3 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -3122,7 +3122,8 @@ static const int amd64_record_regmap[] =
   AMD64_R8_REGNUM, AMD64_R9_REGNUM, AMD64_R10_REGNUM, AMD64_R11_REGNUM,
   AMD64_R12_REGNUM, AMD64_R13_REGNUM, AMD64_R14_REGNUM, AMD64_R15_REGNUM,
   AMD64_RIP_REGNUM, AMD64_EFLAGS_REGNUM, AMD64_CS_REGNUM, AMD64_SS_REGNUM,
-  AMD64_DS_REGNUM, AMD64_ES_REGNUM, AMD64_FS_REGNUM, AMD64_GS_REGNUM
+  AMD64_DS_REGNUM, AMD64_ES_REGNUM, AMD64_FS_REGNUM, AMD64_GS_REGNUM,
+  AMD64_XMM0_REGNUM
 };
 
 /* Implement the "in_indirect_branch_thunk" gdbarch function.  */
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index d2c3efbc5af..0ff380728c1 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -4451,6 +4451,12 @@ struct i386_record_s
   int rip_offset;
   int popl_esp_hack;
   const int *regmap;
+
+  /* These are used by VEX and XOP prefixes.  */
+  uint8_t map_select;
+  uint8_t vvvv;
+  uint8_t pp;
+  uint8_t l;
 };
 
 /* Parse the "modrm" part of the memory address irp->addr points at.
@@ -4789,6 +4795,43 @@ static int i386_record_floats (struct gdbarch *gdbarch,
   return 0;
 }
 
+/* i386_process_record helper to deal with instructions that start
+   with VEX prefix.  */
+
+static int
+i386_record_vex (struct i386_record_s *ir, uint8_t vex_w, uint8_t vex_r,
+		 int opcode, struct gdbarch *gdbarch)
+{
+  /* We need this to find YMM (and once AVX-512 is supported, ZMM) registers.
+     We should always save the largest available register, since an
+     instruction that handles a smaller reg may zero out the higher bits,
+     so we must have them saved.  */
+  i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep> (gdbarch);
+
+  /* Since we are reading pseudo registers, we need to tell GDB that it is
+     safe to do so, by saying we aren't _really_ running the inferior right
+     now.  */
+  SCOPE_EXIT { inferior_thread ()->set_executing (true); };
+  inferior_thread () -> set_executing (false);
+
+  switch (opcode)
+    {
+    default:
+      gdb_printf (gdb_stderr,
+		  _("Process record does not support VEX instruction 0x%02x "
+		    "at address %s.\n"),
+		  (unsigned int) (opcode),
+		  paddress (gdbarch, ir->orig_addr));
+      return -1;
+    }
+
+  record_full_arch_list_add_reg (ir->regcache, ir->regmap[X86_RECORD_REIP_REGNUM]);
+  if (record_full_arch_list_add_end ())
+    return -1;
+
+  return 0;
+}
+
 /* Parse the current instruction, and record the values of the
    registers and memory that will be changed by the current
    instruction.  Returns -1 if something goes wrong, 0 otherwise.  */
@@ -4811,6 +4854,7 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
   i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep> (gdbarch);
   uint8_t rex_w = -1;
   uint8_t rex_r = 0;
+  bool vex_prefix = false;
 
   memset (&ir, 0, sizeof (struct i386_record_s));
   ir.regcache = regcache;
@@ -4896,6 +4940,53 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
 	  else					/* 32 bit target */
 	    goto out_prefixes;
 	  break;
+	case 0xc4:	/* 3-byte VEX prefixes (for AVX/AVX2 instructions).  */
+	  {
+	    /* The first byte just identifies the VEX prefix.  Data is stored
+	       on the following 2 bytes.  */
+	    uint8_t byte;
+	    if (record_read_memory (gdbarch, ir.addr, &byte, 1))
+	      return -1;
+	    ir.addr++;
+
+	    rex_r = !((byte >> 7) & 0x1);
+	    ir.rex_x = !((byte >> 6) & 0x1);
+	    ir.rex_b = !((byte >> 5) & 0x1);
+	    ir.map_select = byte & 0x1f;
+	    /* Collect the last byte of the prefix.  */
+	    if (record_read_memory (gdbarch, ir.addr, &byte, 1))
+	      return -1;
+	    ir.addr++;
+	    rex_w = (byte >> 7) & 0x1;
+	    ir.vvvv = (~(byte >> 3) & 0xf);
+	    ir.l = (byte >> 2) & 0x1;
+	    ir.pp = byte & 0x3;
+	    vex_prefix = true;
+
+	    break;
+	  }
+	case 0xc5:	/* 2-byte VEX prefix for AVX/AVX2 instructions.  */
+	  {
+	    /* The first byte just identifies the VEX prefix.  Data is stored
+	       on the following 2 bytes.  */
+	    uint8_t byte;
+	    if (record_read_memory (gdbarch, ir.addr, &byte, 1))
+	      return -1;
+	    ir.addr++;
+
+	    /* On the 2-byte versions, these are pre-defined.  */
+	    ir.rex_x = 0;
+	    ir.rex_b = 0;
+	    rex_w = 0;
+	    ir.map_select = 1;
+
+	    rex_r = !((byte >> 7) & 0x1);
+	    ir.vvvv = (~(byte >> 3) & 0xf);
+	    ir.l = (byte >> 2) & 0x1;
+	    ir.pp = byte & 0x3;
+	    vex_prefix = true;
+	    break;
+	  }
 	default:
 	  goto out_prefixes;
 	  break;
@@ -4918,6 +5009,12 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
 
   /* Now check op code.  */
   opcode = (uint32_t) opcode8;
+  if (vex_prefix)
+    {
+      /* If we found the VEX prefix, i386 will either record or warn that
+	 the instruction isn't supported, so we can return the VEX result.  */
+      return i386_record_vex (&ir, rex_w, rex_r, opcode, gdbarch);
+    }
  reswitch:
   switch (opcode)
     {
@@ -7902,9 +7999,11 @@ static const int i386_record_regmap[] =
 {
   I386_EAX_REGNUM, I386_ECX_REGNUM, I386_EDX_REGNUM, I386_EBX_REGNUM,
   I386_ESP_REGNUM, I386_EBP_REGNUM, I386_ESI_REGNUM, I386_EDI_REGNUM,
-  0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0,
+  0, 0, 0, 0,
   I386_EIP_REGNUM, I386_EFLAGS_REGNUM, I386_CS_REGNUM, I386_SS_REGNUM,
-  I386_DS_REGNUM, I386_ES_REGNUM, I386_FS_REGNUM, I386_GS_REGNUM
+  I386_DS_REGNUM, I386_ES_REGNUM, I386_FS_REGNUM, I386_GS_REGNUM,
+  /* xmm0_regnum */ 0
 };
 
 /* Check that the given address appears suitable for a fast
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 82676c24056..9df8611b553 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -325,6 +325,7 @@ enum record_i386_regnum
   X86_RECORD_ES_REGNUM,
   X86_RECORD_FS_REGNUM,
   X86_RECORD_GS_REGNUM,
+  X86_RECORD_XMM0_REGNUM,
 };
 
 #define I386_NUM_GREGS	16
diff --git a/gdb/testsuite/gdb.reverse/step-precsave.exp b/gdb/testsuite/gdb.reverse/step-precsave.exp
index a3979b7afd4..1dc42518ba7 100644
--- a/gdb/testsuite/gdb.reverse/step-precsave.exp
+++ b/gdb/testsuite/gdb.reverse/step-precsave.exp
@@ -46,7 +46,7 @@ with_timeout_factor 20 {
 	-re -wrap "Breakpoint .* end of main .*" {
 	    pass $gdb_test_name
 	}
-	-re -wrap "Process record does not support instruction 0xc5 at.*" {
+	-re -wrap "Process record does not support VEX instruction.*" {
 	    kfail "record/23188" $gdb_test_name
 	}
 	-re -wrap "Process record does not support instruction 0xfae64 at.*" {
-- 
2.47.0


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

* [PATCH v5 3/7] gdb/record: add support to vmovd and vmovq instructions
  2024-10-23 16:51 [PATCH v5 0/7] Support for recording some AVX instructions Guinevere Larsen
  2024-10-23 16:51 ` [PATCH v5 1/7] gdb: Allow replayed threads to read and write pseudo registers Guinevere Larsen
  2024-10-23 16:51 ` [PATCH v5 2/7] gdb: Start supporting AVX instruction Guinevere Larsen
@ 2024-10-23 16:51 ` Guinevere Larsen
  2024-10-23 16:51 ` [PATCH v5 4/7] gdb/record: add support to AVX unpack instructions Guinevere Larsen
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Guinevere Larsen @ 2024-10-23 16:51 UTC (permalink / raw)
  To: gdb-patches; +Cc: Guinevere Larsen

This commit adds support to the x86_64 AVX instructions vmovd and vmovq.
The programmers manuals for Intel and AMD describe these 2 instructions
as being almost the same, but my local testing, using gcc 13.2 on Fedora
39, showed several differences and inconsistencies.

The instruction is supposed to always use the 3-byte VEX prefix, but I
could only find 2-byte versions. The instructions aren't differentiated
by the VEX.w bit, but by opcodes and VEX.pp.

This patch adds a test with many different uses for both vmovd and
vmovq. It also updates the test gdb.reverse/step-precsave.exp to
reference the generic "missing avx support" bug open in the bug tracker
(17346), instead of pointing to one that specifically calls out to
vmovd instructions.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23188
---
 gdb/i386-tdep.c                               |  70 +++++++
 gdb/record-full.c                             |   2 +-
 gdb/testsuite/gdb.reverse/i386-avx-reverse.c  |  97 +++++++++
 .../gdb.reverse/i386-avx-reverse.exp          | 190 ++++++++++++++++++
 gdb/testsuite/gdb.reverse/step-precsave.exp   |   2 +-
 5 files changed, 359 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.reverse/i386-avx-reverse.c
 create mode 100644 gdb/testsuite/gdb.reverse/i386-avx-reverse.exp

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 0ff380728c1..ef1c848b1a2 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -4816,6 +4816,76 @@ i386_record_vex (struct i386_record_s *ir, uint8_t vex_w, uint8_t vex_r,
 
   switch (opcode)
     {
+    case 0x6e:	/* VMOVD XMM, reg/mem  */
+      /* This is moving from a regular register or memory region into an
+	 XMM register. */
+      i386_record_modrm (ir);
+      /* ModR/M only has the 3 least significant bits of the destination
+	 register, the last one is indicated by VEX.R (stored inverted).  */
+      record_full_arch_list_add_reg (ir->regcache,
+				     tdep->ymm0_regnum
+				       + ir->reg + vex_r * 8);
+      break;
+    case 0x7e:	/* VMOV(D/Q)  */
+      i386_record_modrm (ir);
+      /* Both the intel and AMD manual are wrong about this.  According to
+	 it, the only difference between vmovq and vmovd should be the rex_w
+	 bit, but in empirical testing, it seems that they share this opcode,
+	 and the way to differentiate it here is looking at VEX.PP.  */
+      if (ir->pp == 2)
+	{
+	  /* This is vmovq moving from a regular register or memory
+	     into an XMM register.  As above, VEX.R is the final bit for
+	     destination register.  */
+	  record_full_arch_list_add_reg (ir->regcache,
+					 tdep->ymm0_regnum
+					 + ir->reg + vex_r * 8);
+	}
+      else if (ir->pp == 1)
+	{
+	  /* This is the vmovd version that stores into a regular register
+	     or memory region.  */
+	  /* If ModRM.mod is 11 we are saving into a register.  */
+	  if (ir->mod == 3)
+	    record_full_arch_list_add_reg (ir->regcache, ir->regmap[ir->rm]);
+	  else
+	    {
+	      /* Calculate the size of memory that will be modified
+		 and store it in the form of 1 << ir->ot, since that
+		 is how the function uses it.  In theory, VEX.W is supposed
+		 to indicate the size of the memory. In practice, I only
+		 ever seen it set to 0, and for 16 bytes, 0xD6 opcode
+		 is used.  */
+	      if (vex_w)
+		ir->ot = 4;
+	      else
+		ir->ot = 3;
+
+	      i386_record_lea_modrm (ir);
+	    }
+	}
+      else
+	{
+	  gdb_printf ("Unrecognized VEX.PP value %d at address %s.",
+		      ir->pp, paddress(gdbarch, ir->orig_addr));
+	  return -1;
+	}
+      break;
+    case 0xd6: /* VMOVQ reg/mem XMM  */
+      i386_record_modrm (ir);
+      /* This is the vmovq version that stores into a regular register
+	 or memory region.  */
+      /* If ModRM.mod is 11 we are saving into a register.  */
+      if (ir->mod == 3)
+	record_full_arch_list_add_reg (ir->regcache, ir->regmap[ir->rm]);
+      else
+	{
+	  /* We know that this operation is always 64 bits.  */
+	  ir->ot = 4;
+	  i386_record_lea_modrm (ir);
+	}
+      break;
+
     default:
       gdb_printf (gdb_stderr,
 		  _("Process record does not support VEX instruction 0x%02x "
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 074d0cd63fc..22a513ac79a 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -632,7 +632,7 @@ record_full_arch_list_add_reg (struct regcache *regcache, int regnum)
 
   rec = record_full_reg_alloc (regcache, regnum);
 
-  regcache->raw_read (regnum, record_full_get_loc (rec));
+  regcache->cooked_read (regnum, record_full_get_loc (rec));
 
   record_full_arch_list_add (rec);
 
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
new file mode 100644
index 00000000000..fd1c68ae378
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
@@ -0,0 +1,97 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Architecture tests for intel i386 platform.  */
+
+#include <stdlib.h>
+
+char global_buf0[] = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+		      0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f};
+char global_buf1[] = {0, 0, 0, 0, 0, 0, 0, 0,
+		      0, 0, 0, 0, 0, 0, 0, 0};
+char *dyn_buf0;
+char *dyn_buf1;
+
+int
+vmov_test ()
+{
+  char buf0[] = {0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
+		 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f};
+  char buf1[] = {0, 0, 0, 0, 0, 0, 0, 0,
+		 0, 0, 0, 0, 0, 0, 0, 0};
+
+  /*start vmov_test.  */
+
+  /* Operations on registers.  */
+  asm volatile ("mov $0, %rcx");
+  asm volatile ("mov $0xbeef, %rax");
+  asm volatile ("vmovd %rax, %xmm0");
+  asm volatile ("vmovd %xmm0, %rcx");
+  asm volatile ("vmovq %xmm0, %xmm15");
+  asm volatile ("vmovq %0, %%xmm15": : "m" (buf1));
+
+  /* Operations based on local buffers.  */
+  asm volatile ("vmovd %0, %%xmm0": : "m"(buf0));
+  asm volatile ("vmovd %%xmm0, %0": "=m"(buf1));
+  asm volatile ("vmovq %0, %%xmm0": : "m"(buf0));
+  asm volatile ("vmovq %%xmm0, %0": "=m"(buf1));
+
+  /* Operations based on global buffers.  */
+  asm volatile ("vmovd %0, %%xmm0": : "m"(global_buf0));
+  asm volatile ("vmovd %%xmm0, %0": "=m"(global_buf1));
+  asm volatile ("vmovq %0, %%xmm0": : "m"(global_buf0));
+  asm volatile ("vmovq %%xmm0, %0": "=m"(global_buf1));
+
+  /* Operations based on dynamic buffers.  */
+  asm volatile ("vmovd %0, %%xmm0": : "m"(*dyn_buf0));
+  asm volatile ("vmovd %%xmm0, %0": "=m"(*dyn_buf1));
+  asm volatile ("vmovq %0, %%xmm0": : "m"(*dyn_buf0));
+  asm volatile ("vmovq %%xmm0, %0": "=m"(*dyn_buf1));
+
+  /* Reset all relevant buffers. */
+  asm volatile ("vmovq %%xmm15, %0": "=m" (buf1));
+  asm volatile ("vmovq %%xmm15, %0": "=m" (global_buf1));
+  asm volatile ("vmovq %%xmm15, %0": "=m" (*dyn_buf1));
+
+  /* Quick test for a different xmm register.  */
+  asm volatile ("vmovd %0, %%xmm15": "=m" (buf0));
+  asm volatile ("vmovd %0, %%xmm15": "=m" (buf1));
+  asm volatile ("vmovq %0, %%xmm15": "=m" (buf0));
+  asm volatile ("vmovq %0, %%xmm15": "=m" (buf1));
+
+  /* We have a return statement to deal with
+     epilogue in different compilers.  */
+  return 0; /* end vmov_test */
+}
+
+int
+main ()
+{
+  dyn_buf0 = (char *) malloc(sizeof(char) * 16);
+  dyn_buf1 = (char *) malloc(sizeof(char) * 16);
+  for (int i =0; i < 16; i++)
+    {
+      dyn_buf0[i] = 0x20 + i;
+      dyn_buf1[i] = 0;
+    }
+  /* Zero relevant xmm registers, se we know what to look for.  */
+  asm volatile ("vmovq %0, %%xmm0": : "m" (global_buf1));
+  asm volatile ("vmovq %0, %%xmm15": : "m" (global_buf1));
+
+  vmov_test ();
+  return 0;	/* end of main */
+}
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
new file mode 100644
index 00000000000..65e982efc63
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
@@ -0,0 +1,190 @@
+# Copyright 2009-2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+#
+# This test tests some i386 general instructions for reverse execution.
+#
+
+require supports_reverse
+
+if {![istarget "*86*-*linux*"]} {
+    verbose "Skipping i386 reverse tests."
+    return
+}
+
+# TODO: this is the case because I used xmm15 all over the test.
+# Some parts of the test require xmm15 to validate some code paths, but
+# that could be done only on 64bit targets and the rest could use xmm7
+# instead.
+if {![istarget "x86_64-*-*"]} {
+    verbose "avx-reverse requires 64 bit targets"
+    return
+}
+
+standard_testfile
+
+# some targets have leading underscores on assembly symbols.
+set additional_flags [gdb_target_symbol_prefix_flags]
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+	 [list debug $additional_flags]]} {
+    return -1
+}
+
+# Shorthand to test reversing through one instruction and
+# testing if a register has the expected value.
+# Prefix, if included, should end with a colon and space.
+
+proc test_one_register {insn register value {prefix ""}} {
+    gdb_test "reverse-step" "$insn.*" \
+	"${prefix}reverse-step from $insn to test register $register"
+
+    gdb_test "info register $register" \
+	"$register.*uint128 = $value.*" \
+	"${prefix}verify $register before $insn"
+}
+
+# Shorthand to test reversing through one instruction and
+# testing if a variable has the expected value.
+# Prefix, if used, should end with a colon and space.
+
+proc test_one_memory {insn mem value {dynamic false} {prefix ""}} {
+    gdb_test "reverse-step" "$insn.*" \
+	"${prefix}reverse-step from $insn to test memory $mem"
+
+    # For the dynamic buffer, we have to cast and dereference the pointer
+    set cast ""
+    if {$dynamic == true} {
+	set cast {(char [32]) *}
+    }
+
+    gdb_test "p/x $cast$mem" \
+	".*$value.*" \
+	"${prefix}verify $mem before $insn"
+
+}
+
+# Record the execution for the whole function, and stop at its end
+# to check if we can correctly reconstruct the state.
+# In the source code, the function must be FUNCTION_test, and
+# at the end, it must have a comment in the form:
+# /* end FUNCTION_test */
+# Returns true if the full function could be recorded, false otherwise.
+proc record_full_function {function} {
+    set end [gdb_get_line_number "end ${function}_test "]
+    set start [gdb_get_line_number "start ${function}_test"]
+    gdb_breakpoint $start temporary
+    gdb_breakpoint $end temporary
+    gdb_continue_to_breakpoint "start ${function}_test"
+
+    if [supports_process_record] {
+	# Activate process record/replay.
+	gdb_test_no_output "record" "${function}: turn on process record"
+    }
+
+    # We return early in gdb_test_multiple because the rest of the
+    # function is aborting recording and cleaning up to put us back in
+    # a known location.
+    gdb_test_multiple "continue" "continue to end of ${function}_test" {
+	-re " end ${function}_test .*\r\n$::gdb_prompt $" {
+	    pass $gdb_test_name
+	    return true
+	}
+	-re " Illegal instruction.*\r\n$::gdb_prompt $" {
+	    fail $gdb_test_name
+	}
+	-re "Process record does not support VEX instruction.*" {
+	    fail $gdb_test_name
+	}
+    }
+
+    gdb_test "record stop" "Process record is stopped.*" \
+	"delete failed record history"
+    # If we didn't exit early, the temporary breakpoint at the end of
+    # the function hasn't been hit yet, so we can just continue.
+    gdb_continue_to_breakpoint "end ${function}_test" ".*end ${function}_test.*"
+
+    return false
+}
+
+runto_main
+
+global hex
+global decimal
+
+# Record all the execution for vmov tests first.
+
+if {[record_full_function "vmov"] == true} {
+    # Now execute backwards, checking all instructions.
+
+    test_one_register "vmovq" "xmm15" "0x3736353433323130" "reg_reset: "
+    test_one_register "vmovq" "xmm15" "0x0"
+    test_one_register "vmovd" "xmm15" "0x33323130" "reg_reset: "
+    test_one_register "vmovd" "xmm15" "0x0"
+
+    with_test_prefix buffer_reset {
+	test_one_memory "vmovq" "dyn_buf1" \
+	    "0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x0" true
+	test_one_memory "vmovq" "global_buf1" \
+	    "0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x0"
+	test_one_memory "vmovq" "buf1" \
+	    "0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x0"
+    }
+
+    with_test_prefix dynamic_buffers {
+	test_one_memory "vmovq" "dyn_buf1" "0x20, 0x21, 0x22, 0x23, 0x0" true
+	test_one_register "vmovq" "xmm0" "0x23222120"
+	test_one_memory "vmovd" "dyn_buf1" "0x0 .repeats 32 times" true
+	test_one_register "vmovd" "xmm0" "0x1716151413121110"
+    }
+
+    with_test_prefix global_buffers {
+	test_one_memory "vmovq" "global_buf1" "0x10, 0x11, 0x12, 0x13, 0x0"
+	test_one_register "vmovq" "xmm0" "0x13121110"
+	test_one_memory "vmovd" "global_buf1" "0x0 .repeats 32 times"
+	test_one_register "vmovd" "xmm0" "0x3736353433323130"
+    }
+
+    with_test_prefix local_buffers {
+	test_one_memory "vmovq" "buf1" "0x30, 0x31, 0x32, 0x33, 0x0"
+	test_one_register "vmovq" "xmm0" "0x33323130"
+	test_one_memory "vmovd" "buf1" "0x0 .repeats 32 times"
+	test_one_register "vmovd" "xmm0" "0xbeef"
+    }
+
+    # regular registers don't have uint128 members, so do it manually.
+    with_test_prefix registers {
+	test_one_register "vmovq" "xmm15" "0xbeef" "reset xmm15: "
+
+	test_one_register "vmovq" "xmm15" "0x0" "xmm0 to xmm15: "
+
+	gdb_test "reverse-step" "vmovd %xmm0, %rcx.*" \
+	    "reverse step to check rcx recording"
+	gdb_test "print/x \$rcx" "= 0x0" "rcx was recorded"
+
+	test_one_register "vmovd" "xmm0" "0x0"
+    }
+
+    # Stop recording to get a clean history.
+    gdb_test "record stop" "Process record is stopped.*" \
+	"delete history for vmov_test"
+} else {
+    untested "could not record vmov_test"
+}
+
+# Move to the end of vmov_test to set up next.
+gdb_test "finish" "Run till exit from.*vmov_test.*" "leaving vmov_test"
diff --git a/gdb/testsuite/gdb.reverse/step-precsave.exp b/gdb/testsuite/gdb.reverse/step-precsave.exp
index 1dc42518ba7..27b417ec262 100644
--- a/gdb/testsuite/gdb.reverse/step-precsave.exp
+++ b/gdb/testsuite/gdb.reverse/step-precsave.exp
@@ -47,7 +47,7 @@ with_timeout_factor 20 {
 	    pass $gdb_test_name
 	}
 	-re -wrap "Process record does not support VEX instruction.*" {
-	    kfail "record/23188" $gdb_test_name
+	    kfail "record/17346" $gdb_test_name
 	}
 	-re -wrap "Process record does not support instruction 0xfae64 at.*" {
 	    kfail "record/25038" $gdb_test_name
-- 
2.47.0


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

* [PATCH v5 4/7] gdb/record: add support to AVX unpack instructions
  2024-10-23 16:51 [PATCH v5 0/7] Support for recording some AVX instructions Guinevere Larsen
                   ` (2 preceding siblings ...)
  2024-10-23 16:51 ` [PATCH v5 3/7] gdb/record: add support to vmovd and vmovq instructions Guinevere Larsen
@ 2024-10-23 16:51 ` Guinevere Larsen
  2024-10-23 16:51 ` [PATCH v5 5/7] gdb/record: Add recording support to vpbroadcast instructions Guinevere Larsen
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Guinevere Larsen @ 2024-10-23 16:51 UTC (permalink / raw)
  To: gdb-patches; +Cc: Guinevere Larsen

This commit adds support to recording instructions to unpack high
or low data from XMM registers, identified by the mnemonics in the
form: VPUNPCK [L|H] [BW|WD|DQ|QDQ].
All these instructions are encoded the exact same way, and only affect
the destination register, making them trivial to implement together.

It also updates the test gdb.reverse/i386-avx-reverse.exp to test these
new instructions.  The test always uses ymm because the vpunpck
instructions overwrite the high bits, so we have to be able to record
the full ymm register, not just the output size.
---
 gdb/i386-tdep.c                               | 16 ++++++
 gdb/testsuite/gdb.reverse/i386-avx-reverse.c  | 55 +++++++++++++++++++
 .../gdb.reverse/i386-avx-reverse.exp          | 51 ++++++++++++++++-
 3 files changed, 121 insertions(+), 1 deletion(-)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index ef1c848b1a2..a0e0181a2c5 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -4886,6 +4886,22 @@ i386_record_vex (struct i386_record_s *ir, uint8_t vex_w, uint8_t vex_r,
 	}
       break;
 
+    case 0x60:	/* VPUNPCKLBW  */
+    case 0x61:	/* VPUNPCKLWD  */
+    case 0x62:	/* VPUNPCKLDQ  */
+    case 0x6c:	/* VPUNPCKLQDQ */
+    case 0x68:	/* VPUNPCKHBW  */
+    case 0x69:	/* VPUNPCKHWD  */
+    case 0x6a:	/* VPUNPCKHDQ  */
+    case 0x6d:	/* VPUNPCKHQDQ */
+      {
+	i386_record_modrm (ir);
+	int reg_offset = ir->reg + vex_r * 8;
+	record_full_arch_list_add_reg (ir->regcache,
+				       tdep->ymm0_regnum + reg_offset);
+      }
+      break;
+
     default:
       gdb_printf (gdb_stderr,
 		  _("Process record does not support VEX instruction 0x%02x "
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
index fd1c68ae378..c897436995e 100644
--- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
@@ -78,6 +78,58 @@ vmov_test ()
   return 0; /* end vmov_test */
 }
 
+/* Test if we can properly record (and undo) vpunpck style instructions.
+   Most tests will use xmm0 and xmm1 as sources. The registers xmm15 and xmm2
+   are used as destination to ensure we're reading the VEX.R bit correctly.  */
+int
+vpunpck_test  ()
+{
+  /* Using GDB, load these values onto registers, for ease of testing.
+     ymm0.v2_int128  = {0x1f1e1d1c1b1a19181716151413121110, 0x2f2e2d2c2b2a29282726252423222120}
+     ymm1.v2_int128  = {0x4f4e4d4c4b4a49484746454443424140, 0x3f3e3d3c3b3a39383736353433323130}
+     ymm15.v2_int128 = {0xdead, 0xbeef}
+     so that's easy to confirm that the unpacking went as expected.  */
+
+  /* start vpunpck_test.  */
+
+  /* First try all low bit unpack instructions with xmm registers.  */
+  /* 17 27 16 26 15 25 14 24 ...*/
+  asm volatile ("vpunpcklbw  %xmm0, %xmm1, %xmm15");
+  /* 17 16 27 26 15 14 25 24 ...*/
+  asm volatile ("vpunpcklwd  %0, %%xmm1, %%xmm15"
+      : : "m" (global_buf0));
+  /* 17 16 15 14 27 26 25 24 ...*/
+  asm volatile ("vpunpckldq  %0, %%xmm1,  %%xmm2"
+      : : "m" (global_buf0));
+  /* 17 16 15 14 13 12 11 10 ...*/
+  asm volatile ("vpunpcklqdq %xmm0, %xmm1, %xmm2");
+
+  /* Then try all high bit unpack instructions with xmm registers.  */
+  /* 17 27 16 26 15 25 14 24 ...*/
+  asm volatile ("vpunpckhbw  %xmm0, %xmm1, %xmm15");
+  /* 17 16 27 26 15 14 25 24 ...*/
+  asm volatile ("vpunpckhwd  %0, %%xmm1, %%xmm15"
+      : : "m" (global_buf0));
+  /* 17 16 15 14 27 26 25 24 ...*/
+  asm volatile ("vpunpckhdq  %0, %%xmm1,  %%xmm2"
+      : : "m" (global_buf0));
+  /* 17 16 15 14 13 12 11 10 ...*/
+  asm volatile ("vpunpckhqdq %xmm0, %xmm1, %xmm2");
+
+  /* Lastly, lets test a few unpack instructions with ymm registers.  */
+  /* 17 27 16 26 15 25 14 24 ...*/
+  asm volatile ("vpunpcklbw  %ymm0, %ymm1, %ymm15");
+  /* 17 16 27 26 15 14 25 24 ...*/
+  asm volatile ("vpunpcklwd  %ymm0, %ymm1, %ymm15");
+  /* 17 16 15 14 27 26 25 24 ...*/
+  asm volatile ("vpunpckhdq  %ymm0, %ymm1,  %ymm15");
+  /* 17 16 15 14 13 12 11 10 ...*/
+  asm volatile ("vpunpckhqdq %ymm0, %ymm1, %ymm15");
+  /* We have a return statement to deal with
+     epilogue in different compilers.  */
+  return 0; /* end vpunpck_test */
+}
+
 int
 main ()
 {
@@ -90,8 +142,11 @@ main ()
     }
   /* Zero relevant xmm registers, se we know what to look for.  */
   asm volatile ("vmovq %0, %%xmm0": : "m" (global_buf1));
+  asm volatile ("vmovq %0, %%xmm1": : "m" (global_buf1));
+  asm volatile ("vmovq %0, %%xmm2": : "m" (global_buf1));
   asm volatile ("vmovq %0, %%xmm15": : "m" (global_buf1));
 
   vmov_test ();
+  vpunpck_test ();
   return 0;	/* end of main */
 }
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
index 65e982efc63..718dca3429a 100644
--- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
@@ -53,8 +53,12 @@ proc test_one_register {insn register value {prefix ""}} {
     gdb_test "reverse-step" "$insn.*" \
 	"${prefix}reverse-step from $insn to test register $register"
 
+    if {[regexp {^ymm} $register]} {
+	set value "\\\{$value\\\}"
+    }
+
     gdb_test "info register $register" \
-	"$register.*uint128 = $value.*" \
+	"$register.*int128 = $value.*" \
 	"${prefix}verify $register before $insn"
 }
 
@@ -188,3 +192,48 @@ if {[record_full_function "vmov"] == true} {
 
 # Move to the end of vmov_test to set up next.
 gdb_test "finish" "Run till exit from.*vmov_test.*" "leaving vmov_test"
+
+# Starting vpunpck tests.
+gdb_test_no_output \
+    "set \$ymm0.v4_int64 = {0x1716151413121110, 0x1f1e1d1c1b1a1918, 0x2726252423222120, 0x2f2e2d2c2b2a2928}"
+gdb_test_no_output \
+    "set \$ymm1.v4_int64 = {0x3736353433323130, 0x3f3e3d3c3b3a3938, 0x4746454443424140, 0x4f4e4d4c4b4a4948}"
+gdb_test_no_output "set \$ymm2.v2_int128 = {0x0, 0x0}"
+gdb_test_no_output "set \$ymm15.v2_int128 = {0xdead, 0xbeef}"
+if {[record_full_function "vpunpck"] == true} {
+    test_one_register "vpunpckhqdq" "ymm15" \
+	"0x1f1e1d1c3f3e3d3c1b1a19183b3a3938, 0x2f2e2d2c4f4e4d4c2b2a29284b4a4948" \
+	"ymm: "
+    test_one_register "vpunpckhdq" "ymm15" \
+	"0x17163736151435341312333211103130, 0x27264746252445442322434221204140" \
+	"ymm: "
+    test_one_register "vpunpcklwd" "ymm15" \
+	"0x17371636153514341333123211311030, 0x27472646254524442343224221412040" \
+	"ymm: "
+    test_one_register "vpunpcklbw" "ymm15" \
+	"0x1f1e3f3e1d1c3d3c1b1a3b3a19183938, 0x0" "ymm: "
+
+    test_one_register "vpunpckhqdq" "ymm2" \
+	"0x1f1e1d1c3f3e3d3c1b1a19183b3a3938, 0x0"
+    test_one_register "vpunpckhdq" "ymm2" \
+	"0x17161514131211103736353433323130, 0x0"
+    test_one_register "vpunpckhwd" "ymm15" \
+	"0x1f3f1e3e1d3d1c3c1b3b1a3a19391838, 0x0"
+    test_one_register "vpunpckhbw" "ymm15" \
+	"0x17163736151435341312333211103130, 0x0"
+
+    test_one_register "vpunpcklqdq" "ymm2" \
+	"0x17161514373635341312111033323130, 0x0"
+    test_one_register "vpunpckldq" "ymm2" "0x0, 0x0"
+    test_one_register "vpunpcklwd" "ymm15" \
+	"0x17371636153514341333123211311030, 0x0"
+    test_one_register "vpunpcklbw" "ymm15" "0xdead, 0xbeef"
+} else {
+    untested "couldn't test vpunpck tests"
+}
+
+# Move to the end of vmov_test to set up next.
+# Stop recording in case of recording errors.
+gdb_test "record stop" "Process record is stopped.*" \
+    "delete history for vpunpck_test"
+gdb_test "finish" "Run till exit from.*vpunpck_test.*" "leaving vpunpck_test"
-- 
2.47.0


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

* [PATCH v5 5/7] gdb/record: Add recording support to vpbroadcast instructions
  2024-10-23 16:51 [PATCH v5 0/7] Support for recording some AVX instructions Guinevere Larsen
                   ` (3 preceding siblings ...)
  2024-10-23 16:51 ` [PATCH v5 4/7] gdb/record: add support to AVX unpack instructions Guinevere Larsen
@ 2024-10-23 16:51 ` Guinevere Larsen
  2024-10-23 16:51 ` [PATCH v5 6/7] gdb/record: support AVX instructions VMOVDQ(U|A) when recording Guinevere Larsen
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Guinevere Larsen @ 2024-10-23 16:51 UTC (permalink / raw)
  To: gdb-patches; +Cc: Guinevere Larsen

This commit adds recording support to all AVX and AVX2 instructions
of the form vpbroadcast. GDB is not yet concerned about AVX512 in
recording mode, so for now we only support the AVX2 registers and
instructions.

This commit also updates the gdb.reverse/i386-avx-reverse.exp to test
broadcast instructions.
---
 gdb/i386-tdep.c                               | 13 +++++++++
 gdb/testsuite/gdb.reverse/i386-avx-reverse.c  | 29 +++++++++++++++++++
 .../gdb.reverse/i386-avx-reverse.exp          | 29 +++++++++++++++++++
 3 files changed, 71 insertions(+)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index a0e0181a2c5..b9ac12edfa3 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -4902,6 +4902,19 @@ i386_record_vex (struct i386_record_s *ir, uint8_t vex_w, uint8_t vex_r,
       }
       break;
 
+    case 0x78:	/* VPBROADCASTB  */
+    case 0x79:	/* VPBROADCASTW  */
+    case 0x58:	/* VPBROADCASTD  */
+    case 0x59:	/* VPBROADCASTQ  */
+      {
+	i386_record_modrm (ir);
+	int reg_offset = ir->reg + vex_r * 8;
+	gdb_assert (tdep->num_ymm_regs > reg_offset);
+	record_full_arch_list_add_reg (ir->regcache,
+				       tdep->ymm0_regnum + reg_offset);
+      }
+      break;
+
     default:
       gdb_printf (gdb_stderr,
 		  _("Process record does not support VEX instruction 0x%02x "
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
index c897436995e..16303a42248 100644
--- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
@@ -130,6 +130,34 @@ vpunpck_test  ()
   return 0; /* end vpunpck_test */
 }
 
+/* Test if we can record vpbroadcast instructions.  */
+int
+vpbroadcast_test ()
+{
+  /* Using GDB, load this value onto the register, for ease of testing.
+     xmm0.uint128  = 0x0
+     xmm1.uint128  = 0x1f1e1d1c1b1a19181716151413121110
+     xmm15.uint128 = 0x0
+     this way it's easy to confirm we're undoing things correctly.  */
+  /* start vpbroadcast_test.  */
+
+  asm volatile ("vpbroadcastb %xmm1, %xmm0");
+  asm volatile ("vpbroadcastb %xmm1, %xmm15");
+
+  asm volatile ("vpbroadcastw %xmm1, %ymm0");
+  asm volatile ("vpbroadcastw %xmm1, %ymm15");
+
+  asm volatile ("vpbroadcastd %xmm1, %xmm0");
+  asm volatile ("vpbroadcastd %xmm1, %xmm15");
+
+  asm volatile ("vpbroadcastq %xmm1, %ymm0");
+  asm volatile ("vpbroadcastq %xmm1, %ymm15");
+
+  /* We have a return statement to deal with
+     epilogue in different compilers.  */
+  return 0; /* end vpbroadcast_test */
+}
+
 int
 main ()
 {
@@ -148,5 +176,6 @@ main ()
 
   vmov_test ();
   vpunpck_test ();
+  vpbroadcast_test ();
   return 0;	/* end of main */
 }
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
index 718dca3429a..75c313c2225 100644
--- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
@@ -237,3 +237,32 @@ if {[record_full_function "vpunpck"] == true} {
 gdb_test "record stop" "Process record is stopped.*" \
     "delete history for vpunpck_test"
 gdb_test "finish" "Run till exit from.*vpunpck_test.*" "leaving vpunpck_test"
+
+# Start vpbroadcast tests
+gdb_test_no_output "set \$ymm0.v2_int128 = {0x0, 0x0}" "set xmm0 for vpbroadcast"
+gdb_test_no_output "set \$xmm1.v2_int64 = {0x1716151413121110, 0x1f1e1d1c1b1a1918}" \
+    "set xmm1 for vpbroadcast"
+gdb_test_no_output "set \$ymm15.v2_int128 = {0x0, 0x0}" "set xmm15 for vpbroadcast"
+if {[record_full_function "vpbroadcast"] == true} {
+    test_one_register "vpbroadcastq" "ymm15" "0x13121110131211101312111013121110, 0x0"
+    test_one_register "vpbroadcastq" "ymm0"  "0x13121110131211101312111013121110, 0x0"
+
+    test_one_register "vpbroadcastd" "ymm15" \
+	"0x11101110111011101110111011101110, 0x11101110111011101110111011101110"
+    test_one_register "vpbroadcastd" "ymm0"  \
+	"0x11101110111011101110111011101110, 0x11101110111011101110111011101110"
+
+    test_one_register "vpbroadcastw" "ymm15" "0x10101010101010101010101010101010, 0x0"
+    test_one_register "vpbroadcastw" "ymm0"  "0x10101010101010101010101010101010, 0x0"
+
+    test_one_register "vpbroadcastb" "ymm15" "0x0, 0x0"
+    test_one_register "vpbroadcastb" "ymm0"  "0x0, 0x0"
+
+    gdb_test "record stop" "Process record is stopped.*" \
+	"delete history for vpbroadcast_test"
+} else {
+    untested "couldn't run vpbroadcast tests"
+}
+
+gdb_test "finish" "Run till exit from.*vpbroadcast_test.*" \
+    "leaving vpbroadcast"
-- 
2.47.0


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

* [PATCH v5 6/7] gdb/record: support AVX instructions VMOVDQ(U|A) when recording
  2024-10-23 16:51 [PATCH v5 0/7] Support for recording some AVX instructions Guinevere Larsen
                   ` (4 preceding siblings ...)
  2024-10-23 16:51 ` [PATCH v5 5/7] gdb/record: Add recording support to vpbroadcast instructions Guinevere Larsen
@ 2024-10-23 16:51 ` Guinevere Larsen
  2024-10-23 16:51 ` [PATCH v5 7/7] gdb/record: add support to vzeroupper instruction Guinevere Larsen
  2024-10-25 15:46 ` [PATCH v5 0/7] Support for recording some AVX instructions Tom Tromey
  7 siblings, 0 replies; 10+ messages in thread
From: Guinevere Larsen @ 2024-10-23 16:51 UTC (permalink / raw)
  To: gdb-patches; +Cc: Guinevere Larsen

This commit adds support for the instructions VMOVDQU and VMOVDQA, used
to move values to/from 256 bit registers. Unfortunately, the
programmer's manual is very incomplete (if not wrong) about these
instructions, so the logic had to be reverse engineered from how gcc
actually encodes the instruction.

This commit also changes the memory regions from the test to store 256
bits, so its easier to test the instructions and that we're recording
ymm registers correctly.
---
 gdb/i386-tdep.c                               | 49 +++++++++++++++++++
 gdb/testsuite/gdb.reverse/i386-avx-reverse.c  | 42 ++++++++++++++--
 .../gdb.reverse/i386-avx-reverse.exp          | 28 +++++++++++
 3 files changed, 115 insertions(+), 4 deletions(-)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index b9ac12edfa3..938a654bf39 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -4886,6 +4886,55 @@ i386_record_vex (struct i386_record_s *ir, uint8_t vex_w, uint8_t vex_r,
 	}
       break;
 
+    case 0x6f: /* VMOVDQ (U|A)  */
+    case 0x7f: /* VMOVDQ (U|A)  */
+      /* vmovdq instructions have information about source/destination
+	 spread over many places, so this code ended up messier than
+	 I'd like.  */
+      /* The VEX.pp bits identify if the move is aligned or not, but this
+	 doesn't influence the recording so we can ignore it.  */
+      i386_record_modrm (ir);
+      /* The first bit of modrm identifies if both operands of the instruction
+	 are registers (bit = 1) or if one of the operands is memory.  */
+      if (ir->mod & 2)
+	{
+	  if (opcode == 0x6f)
+	    {
+	      /* vex_r will identify the high bit of the destination
+		 register.  Source is identified by ir->rex_b, but that
+		 doesn't matter for recording.  */
+	      record_full_arch_list_add_reg (ir->regcache,
+					     tdep->ymm0_regnum + 8*vex_r + ir->reg);
+	    }
+	  else
+	    {
+	      /* The origin operand is >7 and destination operand is <= 7.
+		 This is special cased because in this one vex_r is used to
+		 identify the high bit of the SOURCE operand, not destination
+		 which would mess the previous expression.  */
+	      record_full_arch_list_add_reg (ir->regcache,
+					     tdep->ymm0_regnum + ir->rm);
+	    }
+	}
+      else
+	{
+	  /* This is the easy branch.  We just need to check the opcode
+	     to see if the source or destination is memory.  */
+	  if (opcode == 0x6f)
+	    {
+	      record_full_arch_list_add_reg (ir->regcache,
+					     tdep->ymm0_regnum
+					      + ir->reg + vex_r * 8);
+	    }
+	  else
+	    {
+	      /* We're writing 256 bits, so 1<<8.  */
+	      ir->ot = 8;
+	      i386_record_lea_modrm (ir);
+	    }
+	}
+      break;
+
     case 0x60:	/* VPUNPCKLBW  */
     case 0x61:	/* VPUNPCKLWD  */
     case 0x62:	/* VPUNPCKLDQ  */
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
index 16303a42248..87574983c8a 100644
--- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
@@ -20,8 +20,12 @@
 #include <stdlib.h>
 
 char global_buf0[] = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+		      0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,
+		      0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
 		      0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f};
 char global_buf1[] = {0, 0, 0, 0, 0, 0, 0, 0,
+		      0, 0, 0, 0, 0, 0, 0, 0,
+		      0, 0, 0, 0, 0, 0, 0, 0,
 		      0, 0, 0, 0, 0, 0, 0, 0};
 char *dyn_buf0;
 char *dyn_buf1;
@@ -30,8 +34,12 @@ int
 vmov_test ()
 {
   char buf0[] = {0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
+		 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f,
+		 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
 		 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f};
   char buf1[] = {0, 0, 0, 0, 0, 0, 0, 0,
+		 0, 0, 0, 0, 0, 0, 0, 0,
+		 0, 0, 0, 0, 0, 0, 0, 0,
 		 0, 0, 0, 0, 0, 0, 0, 0};
 
   /*start vmov_test.  */
@@ -73,6 +81,32 @@ vmov_test ()
   asm volatile ("vmovq %0, %%xmm15": "=m" (buf0));
   asm volatile ("vmovq %0, %%xmm15": "=m" (buf1));
 
+  /* Test vmovdq style instructions.  */
+  /* For local and dynamic buffers, we can't guarantee they will be aligned.
+     However, the aligned and unaligned versions seem to be encoded the same,
+     so testing one is enough to validate both.  */
+
+  /* Operations based on local buffers.  */
+  asm volatile ("vmovdqu %0, %%ymm0": : "m"(buf0));
+  asm volatile ("vmovdqu %%ymm0, %0": "=m"(buf1));
+
+  /* Operations based on global buffers.  */
+  /* Global buffers seem to always be aligned, lets sanity check vmovdqa.  */
+  asm volatile ("vmovdqa %0, %%ymm15": : "m"(global_buf0));
+  asm volatile ("vmovdqa %%ymm15, %0": "=m"(global_buf1));
+  asm volatile ("vmovdqu %0, %%ymm0": : "m"(global_buf0));
+  asm volatile ("vmovdqu %%ymm0, %0": "=m"(global_buf1));
+
+  /* Operations based on dynamic buffers.  */
+  /* The dynamic buffers are not aligned, so we skip vmovdqa.  */
+  asm volatile ("vmovdqu %0, %%ymm0": : "m"(*dyn_buf0));
+  asm volatile ("vmovdqu %%ymm0, %0": "=m"(*dyn_buf1));
+
+  /* Operations between 2 registers.  */
+  asm volatile ("vmovdqu %ymm15, %ymm0");
+  asm volatile ("vmovdqu %ymm2, %ymm15");
+  asm volatile ("vmovdqa %ymm15, %ymm0");
+
   /* We have a return statement to deal with
      epilogue in different compilers.  */
   return 0; /* end vmov_test */
@@ -161,11 +195,11 @@ vpbroadcast_test ()
 int
 main ()
 {
-  dyn_buf0 = (char *) malloc(sizeof(char) * 16);
-  dyn_buf1 = (char *) malloc(sizeof(char) * 16);
-  for (int i =0; i < 16; i++)
+  dyn_buf0 = (char *) malloc(sizeof(char) * 32);
+  dyn_buf1 = (char *) malloc(sizeof(char) * 32);
+  for (int i =0; i < 32; i++)
     {
-      dyn_buf0[i] = 0x20 + i;
+      dyn_buf0[i] = 0x20 + (i % 16);
       dyn_buf1[i] = 0;
     }
   /* Zero relevant xmm registers, se we know what to look for.  */
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
index 75c313c2225..aea5e395cf8 100644
--- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
@@ -134,6 +134,34 @@ global decimal
 
 if {[record_full_function "vmov"] == true} {
     # Now execute backwards, checking all instructions.
+    test_one_register "vmovdqa" "ymm0" \
+	"0x1f1e1d1c1b1a19181716151413121110, 0x1f1e1d1c1b1a19181716151413121110" \
+	"from register: "
+    test_one_register "vmovdqu" "ymm15" \
+	"0x1f1e1d1c1b1a19181716151413121110, 0x1f1e1d1c1b1a19181716151413121110" \
+	"from register: "
+    test_one_register "vmovdqu" "ymm0" \
+	"0x2f2e2d2c2b2a29282726252423222120, 0x2f2e2d2c2b2a29282726252423222120" \
+	"from register: "
+
+    test_one_memory "vmovdqu" "dyn_buf1" "0x0 .repeats 32 times" \
+	true "dynamic buffer: "
+    test_one_register "vmovdqu" "ymm0" \
+	"0x1f1e1d1c1b1a19181716151413121110, 0x1f1e1d1c1b1a19181716151413121110" \
+	"dynamic buffer: "
+
+    # Don't check the full buffer because that'd be too long
+    test_one_memory "vmovdqu" "global_buf1" \
+	"0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19" \
+	"global buffer: "
+    test_one_register "vmovdqu" "ymm0" \
+	"0x3f3e3d3c3b3a39383736353433323130, 0x3f3e3d3c3b3a39383736353433323130" \
+	"global buffer: "
+    test_one_memory "vmovdqa" "global_buf1" "0x0 .repeats 32 times"
+    test_one_register "vmovdqa" "ymm15" "0x0, 0x0"
+
+    test_one_memory "vmovdqu" "buf1" "0x0 .repeats 32 times"
+    test_one_register "vmovdqu" "ymm0" "0x2726252423222120, 0x0" "local buffer: "
 
     test_one_register "vmovq" "xmm15" "0x3736353433323130" "reg_reset: "
     test_one_register "vmovq" "xmm15" "0x0"
-- 
2.47.0


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

* [PATCH v5 7/7] gdb/record: add support to vzeroupper instruction
  2024-10-23 16:51 [PATCH v5 0/7] Support for recording some AVX instructions Guinevere Larsen
                   ` (5 preceding siblings ...)
  2024-10-23 16:51 ` [PATCH v5 6/7] gdb/record: support AVX instructions VMOVDQ(U|A) when recording Guinevere Larsen
@ 2024-10-23 16:51 ` Guinevere Larsen
  2024-10-25 15:46 ` [PATCH v5 0/7] Support for recording some AVX instructions Tom Tromey
  7 siblings, 0 replies; 10+ messages in thread
From: Guinevere Larsen @ 2024-10-23 16:51 UTC (permalink / raw)
  To: gdb-patches; +Cc: Guinevere Larsen

This commit adds recording support for the AVX instruction vzeroupper,
which zeroes the high bits of ymm registers 0..15.  In the programmer's
manual, it is explicitly states that ymm registers 16..31 won't be
affected if present, so we only need to record the first 16 registers.

We record ymm_h registers since only the higher bits are touched, and
that reduces the memory footprint of the instruction.

This instruction is tested differently as we want to confirm we're only
saving the relevant registers, and we want to ensure we're saving
all of them, so it makes use of "maint print record-instruction" to see
exactly what was recorded.
---
 gdb/i386-tdep.c                               | 17 +++++++++
 gdb/testsuite/gdb.reverse/i386-avx-reverse.c  | 19 ++++++++++
 .../gdb.reverse/i386-avx-reverse.exp          | 37 +++++++++++++++++++
 3 files changed, 73 insertions(+)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 938a654bf39..6a919bf6473 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -4964,6 +4964,23 @@ i386_record_vex (struct i386_record_s *ir, uint8_t vex_w, uint8_t vex_r,
       }
       break;
 
+    case 0x77:/* VZEROUPPER  */
+      {
+	int num_regs = tdep->num_ymm_regs;
+	/* This instruction only works on ymm0..15, even if 16..31 are
+	   available.  */
+	if (num_regs > 16)
+	  num_regs = 16;
+	for (int i = 0; i < num_regs; i++)
+	  {
+	    /* We only need to record ymm_h, because the low bits
+	       are not touched.  */
+	    record_full_arch_list_add_reg (ir->regcache,
+					   tdep->ymm0h_regnum + i);
+	  }
+	break;
+      }
+
     default:
       gdb_printf (gdb_stderr,
 		  _("Process record does not support VEX instruction 0x%02x "
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
index 87574983c8a..b36de10ec6f 100644
--- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.c
@@ -192,6 +192,24 @@ vpbroadcast_test ()
   return 0; /* end vpbroadcast_test */
 }
 
+int
+vzeroupper_test ()
+{
+  /* start vzeroupper_test.  */
+  /* Using GDB, load this value onto the register, for ease of testing.
+     ymm0.v2_int128  = {0x0, 0x12345}
+     ymm1.v2_int128  = {0x1f1e1d1c1b1a1918, 0x0}
+     ymm2.v2_int128  = {0x0, 0xbeef}
+     ymm15.v2_int128 = {0x0, 0xcafeface}
+     this way it's easy to confirm we're undoing things correctly.  */
+
+  asm volatile ("vzeroupper");
+
+  /* We have a return statement to deal with
+     epilogue in different compilers.  */
+  return 0; /* end vzeroupper_test  */
+}
+
 int
 main ()
 {
@@ -211,5 +229,6 @@ main ()
   vmov_test ();
   vpunpck_test ();
   vpbroadcast_test ();
+  vzeroupper_test ();
   return 0;	/* end of main */
 }
diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
index aea5e395cf8..4aefbcdbab3 100644
--- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
+++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
@@ -294,3 +294,40 @@ if {[record_full_function "vpbroadcast"] == true} {
 
 gdb_test "finish" "Run till exit from.*vpbroadcast_test.*" \
     "leaving vpbroadcast"
+
+# Preparation and testing of vzeroupper
+gdb_test_no_output "set \$ymm0.v2_int128 = {0x0, 0x12345}" "set ymm0 for vzeroupper"
+gdb_test_no_output "set \$ymm1.v2_int128 = {0x1f1e1d1c1b1a1918, 0x0}" \
+    "set ymm1 for vzeroupper"
+gdb_test_no_output "set \$ymm2.v2_int128 = {0x0, 0xbeef}" "set ymm2 for vzeroupper"
+gdb_test_no_output "set \$ymm15.v2_int128 = {0x0, 0xcafeface}" "set ymm15 for vpbroadcast"
+if {[record_full_function "vzeroupper"] == true} {
+    # Since vzeroupper needs to save 8 or 16 registers, let's check what was
+    # actually recorded, instead of just undoing an instruction.  Only
+    # really check the values of egisters 0, 1, 2 and 15 because those are
+    # the only ones we're setting.
+    gdb_test "maint print record-instruction" \
+	[multi_line "Register ymm0h changed: 74565" \
+	    "Register ymm1h changed: 0" \
+	    "Register ymm2h changed: 48879" \
+	    "Register ymm3h changed: ${decimal}" \
+	    "Register ymm4h changed: ${decimal}" \
+	    "Register ymm5h changed: ${decimal}" \
+	    "Register ymm6h changed: ${decimal}" \
+	    "Register ymm7h changed: ${decimal}" \
+	    "Register ymm8h changed: ${decimal}" \
+	    "Register ymm9h changed: ${decimal}" \
+	    "Register ymm10h changed: ${decimal}" \
+	    "Register ymm11h changed: ${decimal}" \
+	    "Register ymm12h changed: ${decimal}" \
+	    "Register ymm13h changed: ${decimal}" \
+	    "Register ymm14h changed: ${decimal}" \
+	    "Register ymm15h changed: 3405707982" \
+	    "Register rip changed: \[^\r\n\]+" ] \
+	"verify vzeroupper recording"
+} else {
+    untested "couldn't run vzeroupper tests"
+}
+
+gdb_test "finish" "Run till exit from.*vzeroupper_test.*" \
+    "leaving vzeroupper"
-- 
2.47.0


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

* Re: [PATCH v5 0/7] Support for recording some AVX instructions
  2024-10-23 16:51 [PATCH v5 0/7] Support for recording some AVX instructions Guinevere Larsen
                   ` (6 preceding siblings ...)
  2024-10-23 16:51 ` [PATCH v5 7/7] gdb/record: add support to vzeroupper instruction Guinevere Larsen
@ 2024-10-25 15:46 ` Tom Tromey
  2024-10-28 13:52   ` Guinevere Larsen
  7 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2024-10-25 15:46 UTC (permalink / raw)
  To: Guinevere Larsen; +Cc: gdb-patches

>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:

Guinevere> The first patch of the series makes GDB aware that it can read pseudo
Guinevere> registers freom executing and replaying threads, no user-visible
Guinevere> changes expected. The second adds the capability for the main i386 record
Guinevere> function to identify and read VEX prefixes, but doesn't add support for
Guinevere> any instruction yet, so minimal user visible changes (only a slight
Guinevere> tweak to an error message). The remaining patches add support to related
Guinevere> instruction, and tests those instructions.

FWIW these look fine to me.
I didn't really review the target-dependent parts in detail, like I
didn't look up instruction encodings or anything like that.

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

Tom

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

* Re: [PATCH v5 0/7] Support for recording some AVX instructions
  2024-10-25 15:46 ` [PATCH v5 0/7] Support for recording some AVX instructions Tom Tromey
@ 2024-10-28 13:52   ` Guinevere Larsen
  0 siblings, 0 replies; 10+ messages in thread
From: Guinevere Larsen @ 2024-10-28 13:52 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 10/25/24 12:46 PM, Tom Tromey wrote:
>>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:
> Guinevere> The first patch of the series makes GDB aware that it can read pseudo
> Guinevere> registers freom executing and replaying threads, no user-visible
> Guinevere> changes expected. The second adds the capability for the main i386 record
> Guinevere> function to identify and read VEX prefixes, but doesn't add support for
> Guinevere> any instruction yet, so minimal user visible changes (only a slight
> Guinevere> tweak to an error message). The remaining patches add support to related
> Guinevere> instruction, and tests those instructions.
>
> FWIW these look fine to me.
> I didn't really review the target-dependent parts in detail, like I
> didn't look up instruction encodings or anything like that.
>
> Approved-By: Tom Tromey <tom@tromey.com>

Thanks for the review!

I've pushed this! If there is something I missed related to the 
target-specific parts, the test should let us find it easily and figure 
out what's up.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> Tom
>


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

end of thread, other threads:[~2024-10-28 13:52 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-23 16:51 [PATCH v5 0/7] Support for recording some AVX instructions Guinevere Larsen
2024-10-23 16:51 ` [PATCH v5 1/7] gdb: Allow replayed threads to read and write pseudo registers Guinevere Larsen
2024-10-23 16:51 ` [PATCH v5 2/7] gdb: Start supporting AVX instruction Guinevere Larsen
2024-10-23 16:51 ` [PATCH v5 3/7] gdb/record: add support to vmovd and vmovq instructions Guinevere Larsen
2024-10-23 16:51 ` [PATCH v5 4/7] gdb/record: add support to AVX unpack instructions Guinevere Larsen
2024-10-23 16:51 ` [PATCH v5 5/7] gdb/record: Add recording support to vpbroadcast instructions Guinevere Larsen
2024-10-23 16:51 ` [PATCH v5 6/7] gdb/record: support AVX instructions VMOVDQ(U|A) when recording Guinevere Larsen
2024-10-23 16:51 ` [PATCH v5 7/7] gdb/record: add support to vzeroupper instruction Guinevere Larsen
2024-10-25 15:46 ` [PATCH v5 0/7] Support for recording some AVX instructions Tom Tromey
2024-10-28 13:52   ` Guinevere Larsen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).