[Lldb-commits] [lldb] [lldb][AArch64] Move register info reconfigure into architecture plugin (PR #70950)

David Spickett via lldb-commits lldb-commits at lists.llvm.org
Wed Nov 1 08:40:30 PDT 2023


https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/70950

This removes AArch64 specific code from the GDB* classes.

To do this I've added 2 new methods to Architecture:
* RegisterWriteCausesReconfigure to check if what you are about to do
  will trash the register info.
* ReconfigureRegisterInfo to do the reconfiguring. This tells you if
  anything changed so that we only invalidate registers when needed.

So that ProcessGDBRemote can call ReconfigureRegisterInfo in SetThreadStopInfo,
I've added forwarding calls to GDBRemoteRegisterContext and the base class
RegisterContext.

(which removes a slightly sketchy static cast as well)

RegisterContext defaults to doing nothing for both the methods
so anything other than GDBRemoteRegisterContext will do nothing.

>From f5c0e05769e4f9093d4477bec39c19afd0415604 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 31 Oct 2023 14:11:47 +0000
Subject: [PATCH 1/2] [lldb][AArch64] Simplify handing of scalable registers
 using vg and svg

This removes explicit invalidation of vg and svg that was done in
`GDBRemoteRegisterContext::AArch64Reconfigure`. This was in fact
covering up a bug elsehwere.

Register information says that a write to vg also invalidates svg
(it does not unless you are in streaming mode, but we decided to
keep it simple and say it always does).

This invalidation was not being applied until *after* AArch64Reconfigure
was called. This meant that without those manual invalidates this
happened:
* vg is written
* svg is not invalidated
* Reconfigure uses the written vg value
* Reconfigure uses the *old* svg value

I have moved the AArch64Reconfigure call to after we've processed
the invalidations caused by the register write, so we no longer
need the manual invalidates in AArch64Reconfigure.

In addition I have changed the order in which expedited registers
as parsed. These registers come with a stop notification and include,
amongst others, vg and svg.

So now we:
* Parse them and update register values (including vg and svg)
* AArch64Reconfigure, which uses those values, and invalidates every
  register, because offsets may have changed.
* Parse the expedited registers again, knowing that none of the
  values will have changed due to the scaling.

This means we use the expedited registers during the reconfigure,
but the invalidate does not mean we throw all of them away.

The cost is we parse them twice client side, but this is cheap
compared to a network packet, and is limited to AArch64 targets
only.

On a system with SVE and SME, these are the packets sent for a step:
```
(lldb) b-remote.async>  < 803> read packet:
$T05thread:p1f80.1f80;name:main.o;threads:1f80;thread-pcs:000000000040056c<...>a1:0800000000000000;d9:0400000000000000;reason:trace;#fc
intern-state     <  21> send packet: $xfffffffff200,200#5e
intern-state     < 516> read packet:
$e4f2ffffffff000000<...>#71
intern-state     <  15> send packet: $Z0,400568,4#4d
intern-state     <   6> read packet: $OK#9a
dbg.evt-handler  <  16> send packet: $jThreadsInfo#c1
dbg.evt-handler  < 224> read packet:
$[{"name":"main.o","reason":"trace","registers":{"161":"0800000000000000",<...>}],"signal":5,"tid":8064}]]#73
```

You can see there are no extra register reads which means we're using
the expedited registers.

For a write to vg:
```
(lldb) register write vg 4
lldb             <  37> send packet:
$Pa1=0400000000000000;thread:1f80;#4a
lldb             <   6> read packet: $OK#9a
lldb             <  20> send packet: $pa1;thread:1f80;#29
lldb             <  20> read packet: $0400000000000000#04
lldb             <  20> send packet: $pd9;thread:1f80;#34
lldb             <  20> read packet: $0400000000000000#04
```

There is the initial P write, and lldb correctly assumes that SVG is
invalidated by this also so we read back the new vg and svg values
afterwards.
---
 .../gdb-remote/GDBRemoteRegisterContext.cpp   | 17 ++----
 .../Process/gdb-remote/ProcessGDBRemote.cpp   | 55 +++++++++++++------
 .../Process/gdb-remote/ProcessGDBRemote.h     |  3 +
 3 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index 72280927471f883..013b2bbc0e67f27 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -434,11 +434,6 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const RegisterInfo *reg_info,
         } else {
           // This is an actual register, write it
           success = SetPrimordialRegister(reg_info, gdb_comm);
-
-          if (success && do_reconfigure_arm64_sve) {
-            AArch64Reconfigure();
-            InvalidateAllRegisters();
-          }
         }
 
         // Check if writing this register will invalidate any other register
@@ -452,6 +447,11 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const RegisterInfo *reg_info,
                                false);
         }
 
+        if (success && do_reconfigure_arm64_sve) {
+          AArch64Reconfigure();
+          InvalidateAllRegisters();
+        }
+
         return success;
       }
     } else {
@@ -772,8 +772,6 @@ void GDBRemoteRegisterContext::AArch64Reconfigure() {
   std::optional<uint64_t> vg_reg_value;
   const RegisterInfo *vg_reg_info = m_reg_info_sp->GetRegisterInfo("vg");
   if (vg_reg_info) {
-    // Make sure we get the latest value of vg from the remote.
-    SetRegisterIsValid(vg_reg_info, false);
     uint32_t vg_reg_num = vg_reg_info->kinds[eRegisterKindLLDB];
     uint64_t reg_value = ReadRegisterAsUnsigned(vg_reg_num, fail_value);
     if (reg_value != fail_value && reg_value <= 32)
@@ -783,11 +781,6 @@ void GDBRemoteRegisterContext::AArch64Reconfigure() {
   std::optional<uint64_t> svg_reg_value;
   const RegisterInfo *svg_reg_info = m_reg_info_sp->GetRegisterInfo("svg");
   if (svg_reg_info) {
-    // When vg is written it is automatically made invalid. Writing vg will also
-    // change svg if we're in streaming mode but it will not be made invalid
-    // so do this manually so the following read gets the latest svg value.
-    SetRegisterIsValid(svg_reg_info, false);
-
     uint32_t svg_reg_num = svg_reg_info->kinds[eRegisterKindLLDB];
     uint64_t reg_value = ReadRegisterAsUnsigned(svg_reg_num, fail_value);
     if (reg_value != fail_value && reg_value <= 32)
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 16b3ba661d07162..c50ac5de77904f6 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1612,6 +1612,22 @@ bool ProcessGDBRemote::CalculateThreadStopInfo(ThreadGDBRemote *thread) {
   return false;
 }
 
+void ProcessGDBRemote::ParseExpeditedRegisters(
+    ExpeditedRegisterMap &expedited_register_map, ThreadSP thread_sp) {
+  ThreadGDBRemote *gdb_thread = static_cast<ThreadGDBRemote *>(thread_sp.get());
+  RegisterContextSP gdb_reg_ctx_sp(gdb_thread->GetRegisterContext());
+
+  for (const auto &pair : expedited_register_map) {
+    StringExtractor reg_value_extractor(pair.second);
+    WritableDataBufferSP buffer_sp(
+        new DataBufferHeap(reg_value_extractor.GetStringRef().size() / 2, 0));
+    reg_value_extractor.GetHexBytes(buffer_sp->GetData(), '\xcc');
+    uint32_t lldb_regnum = gdb_reg_ctx_sp->ConvertRegisterKindToRegisterNumber(
+        eRegisterKindProcessPlugin, pair.first);
+    gdb_thread->PrivateSetRegisterValue(lldb_regnum, buffer_sp->GetData());
+  }
+}
+
 ThreadSP ProcessGDBRemote::SetThreadStopInfo(
     lldb::tid_t tid, ExpeditedRegisterMap &expedited_register_map,
     uint8_t signo, const std::string &thread_name, const std::string &reason,
@@ -1646,32 +1662,35 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
 
   reg_ctx_sp->InvalidateIfNeeded(true);
 
+  auto iter = std::find(m_thread_ids.begin(), m_thread_ids.end(), tid);
+  if (iter != m_thread_ids.end())
+    SetThreadPc(thread_sp, iter - m_thread_ids.begin());
+
+  ParseExpeditedRegisters(expedited_register_map, thread_sp);
+
   // AArch64 SVE/SME specific code below updates SVE and ZA register sizes and
   // offsets if value of VG or SVG registers has changed since last stop.
   const ArchSpec &arch = GetTarget().GetArchitecture();
   if (arch.IsValid() && arch.GetTriple().isAArch64()) {
-    GDBRemoteRegisterContext *gdb_remote_reg_ctx =
-        static_cast<GDBRemoteRegisterContext *>(reg_ctx_sp.get());
+    GDBRemoteRegisterContext *reg_ctx_sp =
+        static_cast<GDBRemoteRegisterContext *>(
+            gdb_thread->GetRegisterContext().get());
 
-    if (gdb_remote_reg_ctx) {
-      gdb_remote_reg_ctx->AArch64Reconfigure();
-      gdb_remote_reg_ctx->InvalidateAllRegisters();
+    if (reg_ctx_sp) {
+      reg_ctx_sp->AArch64Reconfigure();
+      // Now we have changed the offsets of all the registers, so the values
+      // will be corrupted.
+      reg_ctx_sp->InvalidateAllRegisters();
+
+      // Expedited registers values will never contain registers that would be
+      // resized by AArch64Reconfigure. So we are safe to continue using these
+      // values. These values include vg, svg and useful general purpose
+      // registers so this saves a few read packets each time we make use of
+      // them.
+      ParseExpeditedRegisters(expedited_register_map, thread_sp);
     }
   }
 
-  auto iter = std::find(m_thread_ids.begin(), m_thread_ids.end(), tid);
-  if (iter != m_thread_ids.end())
-    SetThreadPc(thread_sp, iter - m_thread_ids.begin());
-
-  for (const auto &pair : expedited_register_map) {
-    StringExtractor reg_value_extractor(pair.second);
-    WritableDataBufferSP buffer_sp(
-        new DataBufferHeap(reg_value_extractor.GetStringRef().size() / 2, 0));
-    reg_value_extractor.GetHexBytes(buffer_sp->GetData(), '\xcc');
-    uint32_t lldb_regnum = reg_ctx_sp->ConvertRegisterKindToRegisterNumber(
-        eRegisterKindProcessPlugin, pair.first);
-    gdb_thread->PrivateSetRegisterValue(lldb_regnum, buffer_sp->GetData());
-  }
   thread_sp->SetName(thread_name.empty() ? nullptr : thread_name.c_str());
 
   gdb_thread->SetThreadDispatchQAddr(thread_dispatch_qaddr);
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
index f0ead4c38c237ab..f3787e7169047e2 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -470,6 +470,9 @@ class ProcessGDBRemote : public Process,
   void DidForkSwitchSoftwareBreakpoints(bool enable);
   void DidForkSwitchHardwareTraps(bool enable);
 
+  void ParseExpeditedRegisters(ExpeditedRegisterMap &expedited_register_map,
+                               lldb::ThreadSP thread_sp);
+
   // Lists of register fields generated from the remote's target XML.
   // Pointers to these RegisterFlags will be set in the register info passed
   // back to the upper levels of lldb. Doing so is safe because this class will

>From 74c5179feb31d9f60f1d0c5b16829c75474163c7 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Wed, 1 Nov 2023 13:41:54 +0000
Subject: [PATCH 2/2] [lldb][AArch64] Move register info reconfigure into
 architecture plugin

This removes AArch64 specific code from the GDB* classes.

To do this I've added 2 new methods to Architecture:
* RegisterWriteCausesReconfigure to check if what you are about to do
  will trash the register info.
* ReconfigureRegisterInfo to do the reconfiguring. This tells you if
  anything changed so that we only invalidate registers when needed.

So that ProcessGDBRemote can call ReconfigureRegisterInfo in SetThreadStopInfo,
I've added forwarding calls to GDBRemoteRegisterContext and the base class
RegisterContext.

(which removes a slightly sketchy static cast as well)

RegisterContext defaults to doing nothing for both the methods
so anything other than GDBRemoteRegisterContext will do nothing.
---
 lldb/include/lldb/Core/Architecture.h         |  19 +++
 .../include/lldb/Target/DynamicRegisterInfo.h |   4 +
 lldb/include/lldb/Target/RegisterContext.h    |   6 +
 .../AArch64/ArchitectureAArch64.cpp           |  91 ++++++++++++++
 .../AArch64/ArchitectureAArch64.h             |  10 ++
 .../gdb-remote/GDBRemoteRegisterContext.cpp   | 115 +++++-------------
 .../gdb-remote/GDBRemoteRegisterContext.h     |   5 +-
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |  29 ++---
 8 files changed, 169 insertions(+), 110 deletions(-)

diff --git a/lldb/include/lldb/Core/Architecture.h b/lldb/include/lldb/Core/Architecture.h
index b68bf27ae0df888..d5294197fd119ba 100644
--- a/lldb/include/lldb/Core/Architecture.h
+++ b/lldb/include/lldb/Core/Architecture.h
@@ -10,6 +10,7 @@
 #define LLDB_CORE_ARCHITECTURE_H
 
 #include "lldb/Core/PluginInterface.h"
+#include "lldb/Target/DynamicRegisterInfo.h"
 #include "lldb/Target/MemoryTagManager.h"
 
 namespace lldb_private {
@@ -109,6 +110,24 @@ class Architecture : public PluginInterface {
   virtual const MemoryTagManager *GetMemoryTagManager() const {
     return nullptr;
   }
+
+  // This returns true if a write to the named register should cause lldb to
+  // reconfigure its register information. For example on AArch64 writing to vg
+  // to change the vector length means lldb has to change the size of registers.
+  virtual bool RegisterWriteCausesReconfigure(const char *name) const {
+    return false;
+  }
+
+  // Call this after writing a register for which RegisterWriteCausesReconfigure
+  // returns true. This method will update the layout of registers according to
+  // the new state e.g. the new length of scalable vector registers.
+  // Returns true if anything changed, which means existing register values must
+  // be invalidated.
+  virtual bool ReconfigureRegisterInfo(DynamicRegisterInfo &reg_info,
+                                       DataExtractor &reg_data,
+                                       RegisterContext &reg_context) const {
+    return false;
+  }
 };
 
 } // namespace lldb_private
diff --git a/lldb/include/lldb/Target/DynamicRegisterInfo.h b/lldb/include/lldb/Target/DynamicRegisterInfo.h
index fb22885e713d672..0e175a99eb7d58a 100644
--- a/lldb/include/lldb/Target/DynamicRegisterInfo.h
+++ b/lldb/include/lldb/Target/DynamicRegisterInfo.h
@@ -93,6 +93,10 @@ class DynamicRegisterInfo {
     return llvm::iterator_range<reg_collection::const_iterator>(m_regs);
   }
 
+  llvm::iterator_range<reg_collection::iterator> registers_mutable() {
+    return llvm::iterator_range<reg_collection::iterator>(m_regs);
+  }
+
   void ConfigureOffsets();
 
 protected:
diff --git a/lldb/include/lldb/Target/RegisterContext.h b/lldb/include/lldb/Target/RegisterContext.h
index 893569a98dbd8b3..921c25d215ade23 100644
--- a/lldb/include/lldb/Target/RegisterContext.h
+++ b/lldb/include/lldb/Target/RegisterContext.h
@@ -51,6 +51,12 @@ class RegisterContext : public std::enable_shared_from_this<RegisterContext>,
     return false;
   }
 
+  virtual bool RegisterWriteCausesReconfigure(const char *name) {
+    return false;
+  }
+
+  virtual bool ReconfigureRegisterInfo() { return false; }
+
   // These two functions are used to implement "push" and "pop" of register
   // states.  They are used primarily for expression evaluation, where we need
   // to push a new state (storing the old one in data_sp) and then restoring
diff --git a/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp b/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp
index 1b2b41ee8758758..2954eaa2083af08 100644
--- a/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp
+++ b/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp
@@ -8,7 +8,10 @@
 
 #include "Plugins/Architecture/AArch64/ArchitectureAArch64.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Target/RegisterContext.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/DataBufferHeap.h"
+#include "lldb/Utility/DataExtractor.h"
 
 using namespace lldb_private;
 using namespace lldb;
@@ -34,3 +37,91 @@ ArchitectureAArch64::Create(const ArchSpec &arch) {
   }
   return std::unique_ptr<Architecture>(new ArchitectureAArch64());
 }
+
+static void UpdateARM64SVERegistersInfos(
+    llvm::iterator_range<
+        lldb_private::DynamicRegisterInfo::reg_collection::iterator>
+        regs,
+    uint64_t vg) {
+  // SVE Z register size is vg x 8 bytes.
+  uint32_t z_reg_byte_size = vg * 8;
+
+  // SVE vector length has changed, accordingly set size of Z, P and FFR
+  // registers. Also invalidate register offsets it will be recalculated
+  // after SVE register size update.
+  for (auto &reg : regs) {
+    if (reg.value_regs == nullptr) {
+      if (reg.name[0] == 'z' && isdigit(reg.name[1]))
+        reg.byte_size = z_reg_byte_size;
+      else if (reg.name[0] == 'p' && isdigit(reg.name[1]))
+        reg.byte_size = vg;
+      else if (strcmp(reg.name, "ffr") == 0)
+        reg.byte_size = vg;
+    }
+    reg.byte_offset = LLDB_INVALID_INDEX32;
+  }
+}
+
+static void UpdateARM64SMERegistersInfos(
+    llvm::iterator_range<
+        lldb_private::DynamicRegisterInfo::reg_collection::iterator>
+        regs,
+    uint64_t svg) {
+  for (auto &reg : regs) {
+    if (strcmp(reg.name, "za") == 0) {
+      // ZA is a register with size (svg*8) * (svg*8). A square essentially.
+      reg.byte_size = (svg * 8) * (svg * 8);
+    }
+    reg.byte_offset = LLDB_INVALID_INDEX32;
+  }
+}
+
+bool ArchitectureAArch64::ReconfigureRegisterInfo(DynamicRegisterInfo &reg_info,
+                                                  DataExtractor &reg_data,
+                                                  RegisterContext &reg_context
+
+) const {
+  // Once we start to reconfigure registers, we cannot read any of them.
+  // So we must read VG and SVG up front.
+
+  const uint64_t fail_value = LLDB_INVALID_ADDRESS;
+  std::optional<uint64_t> vg_reg_value;
+  const RegisterInfo *vg_reg_info = reg_info.GetRegisterInfo("vg");
+  if (vg_reg_info) {
+    uint32_t vg_reg_num = vg_reg_info->kinds[eRegisterKindLLDB];
+    uint64_t reg_value =
+        reg_context.ReadRegisterAsUnsigned(vg_reg_num, fail_value);
+    if (reg_value != fail_value && reg_value <= 32)
+      vg_reg_value = reg_value;
+  }
+
+  std::optional<uint64_t> svg_reg_value;
+  const RegisterInfo *svg_reg_info = reg_info.GetRegisterInfo("svg");
+  if (svg_reg_info) {
+    uint32_t svg_reg_num = svg_reg_info->kinds[eRegisterKindLLDB];
+    uint64_t reg_value =
+        reg_context.ReadRegisterAsUnsigned(svg_reg_num, fail_value);
+    if (reg_value != fail_value && reg_value <= 32)
+      svg_reg_value = reg_value;
+  }
+
+  if (!vg_reg_value && !svg_reg_value)
+    return false;
+
+  if (vg_reg_value)
+    UpdateARM64SVERegistersInfos(reg_info.registers_mutable(), *vg_reg_value);
+  if (svg_reg_value)
+    UpdateARM64SMERegistersInfos(reg_info.registers_mutable(), *svg_reg_value);
+
+  // At this point if we have updated any registers, their offsets will all be
+  // invalid. If we did, we need to update them all.
+  reg_info.ConfigureOffsets();
+  // From here we are able to read registers again.
+
+  // Make a heap based buffer that is big enough to store all registers
+  reg_data.SetData(
+      std::make_shared<DataBufferHeap>(reg_info.GetRegisterDataByteSize(), 0));
+  reg_data.SetByteOrder(reg_context.GetByteOrder());
+
+  return true;
+}
diff --git a/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h b/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h
index da0b867fb1e9b22..895630bcd850f49 100644
--- a/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h
+++ b/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h
@@ -28,6 +28,16 @@ class ArchitectureAArch64 : public Architecture {
     return &m_memory_tag_manager;
   }
 
+  bool RegisterWriteCausesReconfigure(const char *name) const override {
+    // lldb treats svg as read only, so only vg can be written. This results in
+    // the SVE registers changing size.
+    return strcmp(name, "vg") == 0;
+  }
+
+  bool ReconfigureRegisterInfo(DynamicRegisterInfo &reg_info,
+                               DataExtractor &reg_data,
+                               RegisterContext &reg_context) const override;
+
 private:
   static std::unique_ptr<Architecture> Create(const ArchSpec &arch);
   ArchitectureAArch64() = default;
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index 013b2bbc0e67f27..bfc633bd0e65eaa 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -8,6 +8,12 @@
 
 #include "GDBRemoteRegisterContext.h"
 
+#include "ProcessGDBRemote.h"
+#include "ProcessGDBRemoteLog.h"
+#include "ThreadGDBRemote.h"
+#include "Utility/ARM_DWARF_Registers.h"
+#include "Utility/ARM_ehframe_Registers.h"
+#include "lldb/Core/Architecture.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
@@ -15,11 +21,6 @@
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/Scalar.h"
 #include "lldb/Utility/StreamString.h"
-#include "ProcessGDBRemote.h"
-#include "ProcessGDBRemoteLog.h"
-#include "ThreadGDBRemote.h"
-#include "Utility/ARM_DWARF_Registers.h"
-#include "Utility/ARM_ehframe_Registers.h"
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 #include <memory>
@@ -373,14 +374,8 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const RegisterInfo *reg_info,
   if (dst == nullptr)
     return false;
 
-  // Code below is specific to AArch64 target in SVE or SME state
-  // If vector granule (vg) register is being written then thread's
-  // register context reconfiguration is triggered on success.
-  // We do not allow writes to SVG so it is not mentioned here.
-  const ArchSpec &arch = process->GetTarget().GetArchitecture();
-  bool do_reconfigure_arm64_sve = arch.IsValid() &&
-                                  arch.GetTriple().isAArch64() &&
-                                  (strcmp(reg_info->name, "vg") == 0);
+  bool should_reconfigure_registers =
+      RegisterWriteCausesReconfigure(reg_info->name);
 
   if (data.CopyByteOrderedData(data_offset,                // src offset
                                reg_info->byte_size,        // src length
@@ -400,8 +395,8 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const RegisterInfo *reg_info,
                 {m_reg_data.GetDataStart(), size_t(m_reg_data.GetByteSize())}))
 
         {
-          if (do_reconfigure_arm64_sve)
-            AArch64Reconfigure();
+          if (should_reconfigure_registers)
+            ReconfigureRegisterInfo();
 
           InvalidateAllRegisters();
 
@@ -447,10 +442,9 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const RegisterInfo *reg_info,
                                false);
         }
 
-        if (success && do_reconfigure_arm64_sve) {
-          AArch64Reconfigure();
+        if (success && should_reconfigure_registers &&
+            ReconfigureRegisterInfo())
           InvalidateAllRegisters();
-        }
 
         return success;
       }
@@ -762,75 +756,22 @@ uint32_t GDBRemoteRegisterContext::ConvertRegisterKindToRegisterNumber(
   return m_reg_info_sp->ConvertRegisterKindToRegisterNumber(kind, num);
 }
 
-void GDBRemoteRegisterContext::AArch64Reconfigure() {
-  assert(m_reg_info_sp);
-
-  // Once we start to reconfigure registers, we cannot read any of them.
-  // So we must read VG and SVG up front.
-
-  const uint64_t fail_value = LLDB_INVALID_ADDRESS;
-  std::optional<uint64_t> vg_reg_value;
-  const RegisterInfo *vg_reg_info = m_reg_info_sp->GetRegisterInfo("vg");
-  if (vg_reg_info) {
-    uint32_t vg_reg_num = vg_reg_info->kinds[eRegisterKindLLDB];
-    uint64_t reg_value = ReadRegisterAsUnsigned(vg_reg_num, fail_value);
-    if (reg_value != fail_value && reg_value <= 32)
-      vg_reg_value = reg_value;
-  }
-
-  std::optional<uint64_t> svg_reg_value;
-  const RegisterInfo *svg_reg_info = m_reg_info_sp->GetRegisterInfo("svg");
-  if (svg_reg_info) {
-    uint32_t svg_reg_num = svg_reg_info->kinds[eRegisterKindLLDB];
-    uint64_t reg_value = ReadRegisterAsUnsigned(svg_reg_num, fail_value);
-    if (reg_value != fail_value && reg_value <= 32)
-      svg_reg_value = reg_value;
-  }
-
-  if (vg_reg_value)
-    m_reg_info_sp->UpdateARM64SVERegistersInfos(*vg_reg_value);
-  if (svg_reg_value)
-    m_reg_info_sp->UpdateARM64SMERegistersInfos(*svg_reg_value);
-
-  // At this point if we have updated any registers, their offsets will all be
-  // invalid. If we did, we need to update them all.
-  if (vg_reg_value || svg_reg_value) {
-    m_reg_info_sp->ConfigureOffsets();
-    // From here we are able to read registers again.
-
-    // Make a heap based buffer that is big enough to store all registers
-    m_reg_data.SetData(std::make_shared<DataBufferHeap>(
-        m_reg_info_sp->GetRegisterDataByteSize(), 0));
-    m_reg_data.SetByteOrder(GetByteOrder());
-  }
-}
-
-void GDBRemoteDynamicRegisterInfo::UpdateARM64SVERegistersInfos(uint64_t vg) {
-  // SVE Z register size is vg x 8 bytes.
-  uint32_t z_reg_byte_size = vg * 8;
-
-  // SVE vector length has changed, accordingly set size of Z, P and FFR
-  // registers. Also invalidate register offsets it will be recalculated
-  // after SVE register size update.
-  for (auto &reg : m_regs) {
-    if (reg.value_regs == nullptr) {
-      if (reg.name[0] == 'z' && isdigit(reg.name[1]))
-        reg.byte_size = z_reg_byte_size;
-      else if (reg.name[0] == 'p' && isdigit(reg.name[1]))
-        reg.byte_size = vg;
-      else if (strcmp(reg.name, "ffr") == 0)
-        reg.byte_size = vg;
-    }
-    reg.byte_offset = LLDB_INVALID_INDEX32;
-  }
+bool GDBRemoteRegisterContext::RegisterWriteCausesReconfigure(
+    const char *name) {
+  ExecutionContext exe_ctx(CalculateThread());
+  Process *process = exe_ctx.GetProcessPtr();
+  const Architecture *architecture =
+      process->GetTarget().GetArchitecturePlugin();
+  return architecture && architecture->RegisterWriteCausesReconfigure(name);
 }
 
-void GDBRemoteDynamicRegisterInfo::UpdateARM64SMERegistersInfos(uint64_t svg) {
-  for (auto &reg : m_regs) {
-    if (strcmp(reg.name, "za") == 0) {
-      // ZA is a register with size (svg*8) * (svg*8). A square essentially.
-      reg.byte_size = (svg * 8) * (svg * 8);
-    }
-    reg.byte_offset = LLDB_INVALID_INDEX32;
-  }
+bool GDBRemoteRegisterContext::ReconfigureRegisterInfo() {
+  ExecutionContext exe_ctx(CalculateThread());
+  Process *process = exe_ctx.GetProcessPtr();
+  const Architecture *architecture =
+      process->GetTarget().GetArchitecturePlugin();
+  if (architecture)
+    return architecture->ReconfigureRegisterInfo(*(m_reg_info_sp.get()),
+                                                 m_reg_data, *this);
+  return false;
 }
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
index 1b6b7e3ce1cb2c8..12c74e14fe544ab 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -78,8 +78,9 @@ class GDBRemoteRegisterContext : public RegisterContext {
   uint32_t ConvertRegisterKindToRegisterNumber(lldb::RegisterKind kind,
                                                uint32_t num) override;
 
-  // Reconfigure variable sized registers for AArch64 SVE and SME.
-  void AArch64Reconfigure();
+  bool RegisterWriteCausesReconfigure(const char *name) override;
+
+  bool ReconfigureRegisterInfo() override;
 
 protected:
   friend class ThreadGDBRemote;
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index c50ac5de77904f6..43e3a92c39df2cc 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1668,27 +1668,14 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
 
   ParseExpeditedRegisters(expedited_register_map, thread_sp);
 
-  // AArch64 SVE/SME specific code below updates SVE and ZA register sizes and
-  // offsets if value of VG or SVG registers has changed since last stop.
-  const ArchSpec &arch = GetTarget().GetArchitecture();
-  if (arch.IsValid() && arch.GetTriple().isAArch64()) {
-    GDBRemoteRegisterContext *reg_ctx_sp =
-        static_cast<GDBRemoteRegisterContext *>(
-            gdb_thread->GetRegisterContext().get());
-
-    if (reg_ctx_sp) {
-      reg_ctx_sp->AArch64Reconfigure();
-      // Now we have changed the offsets of all the registers, so the values
-      // will be corrupted.
-      reg_ctx_sp->InvalidateAllRegisters();
-
-      // Expedited registers values will never contain registers that would be
-      // resized by AArch64Reconfigure. So we are safe to continue using these
-      // values. These values include vg, svg and useful general purpose
-      // registers so this saves a few read packets each time we make use of
-      // them.
-      ParseExpeditedRegisters(expedited_register_map, thread_sp);
-    }
+  if (reg_ctx_sp->ReconfigureRegisterInfo()) {
+    // Now we have changed the offsets of all the registers, so the values
+    // will be corrupted.
+    reg_ctx_sp->InvalidateAllRegisters();
+    // Expedited registers values will never contain registers that would be
+    // resized by a reconfigure. So we are safe to continue using these
+    // values.
+    ParseExpeditedRegisters(expedited_register_map, thread_sp);
   }
 
   thread_sp->SetName(thread_name.empty() ? nullptr : thread_name.c_str());



More information about the lldb-commits mailing list