DavidSpickett created this revision.
Herald added subscribers: ctetreau, kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Reading the SVE registers of streaming mode from non-streaming mode,
and vice versa, returns invalid data. As their state is reset each
time you switch between them.

However, the vector length is distinct between the two modes.

The existing register "vg" will always report the vector length for
the current mode and this change adds "svg" which will always return
the streaming vector length.

non-streaming mode: vg  = non-streaming vector length

                  svg = streaming vector length
  streaming mode: vg  = streaming vector length
                  svg = streaming vector length

The content of svg is read from the NT_ARM_SSVE header, even if
we are in non-streaming mode. Which we are allowed to do
(the result is just missing the register data in this situation).

It is read only for the moment. It may be made writeable in future
patches. It has been added to the SME register set and I've converted
that into a struct for easier handling.

The SVE dynamic size test has been updated to check the expected
svg values as it is already setup to break just after a mode switch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155269

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py

Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
===================================================================
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -96,6 +96,12 @@
 
         self.expect("register read ffr", substrs=[p_regs_value])
 
+    def build_for_mode(self, mode):
+        cflags = "-march=armv8-a+sve -lpthread"
+        if mode == Mode.SSVE:
+            cflags += " -DUSE_SSVE"
+        self.build(dictionary={"CFLAGS_EXTRAS": cflags})
+
     def run_sve_test(self, mode):
         if (mode == Mode.SVE) and not self.isAArch64SVE():
             self.skipTest("SVE registers must be supported.")
@@ -103,12 +109,8 @@
         if (mode == Mode.SSVE) and not self.isAArch64SME():
             self.skipTest("Streaming SVE registers must be supported.")
 
-        cflags = "-march=armv8-a+sve -lpthread"
-        if mode == Mode.SSVE:
-            cflags += " -DUSE_SSVE"
-        self.build(dictionary={"CFLAGS_EXTRAS": cflags})
+        self.build_for_mode(mode)
 
-        self.build()
         supported_vg = self.get_supported_vg()
 
         if not (2 in supported_vg and 4 in supported_vg):
@@ -198,3 +200,62 @@
     def test_ssve_registers_dynamic_config(self):
         """Test AArch64 SSVE registers multi-threaded dynamic resize."""
         self.run_sve_test(Mode.SSVE)
+
+    def setup_svg_test(self, mode):
+        # Even when running in SVE mode, we need access to SVG for these tests.
+        if not self.isAArch64SME():
+            self.skipTest("Streaming SVE registers must be supported.")
+
+        self.build_for_mode(mode)
+
+        supported_vg = self.get_supported_vg()
+
+        main_thread_stop_line = line_number("main.c", "// Break in main thread")
+        lldbutil.run_break_set_by_file_and_line(self, "main.c", main_thread_stop_line)
+
+        self.runCmd("run", RUN_SUCCEEDED)
+
+        process = self.dbg.GetSelectedTarget().GetProcess()
+
+        self.expect(
+            "thread info 1",
+            STOPPED_DUE_TO_BREAKPOINT,
+            substrs=["stop reason = breakpoint"],
+        )
+
+        target = self.dbg.GetSelectedTarget()
+        process = target.GetProcess()
+
+        return process, supported_vg
+
+    def read_vg(self, process):
+        registerSets = process.GetThreadAtIndex(0).GetFrameAtIndex(0).GetRegisters()
+        sve_registers = registerSets.GetFirstValueByName("Scalable Vector Extension Registers")
+        return sve_registers.GetChildMemberWithName("vg").GetValueAsUnsigned()
+
+    def read_svg(self, process):
+        registerSets = process.GetThreadAtIndex(0).GetFrameAtIndex(0).GetRegisters()
+        sve_registers = registerSets.GetFirstValueByName("Scalable Matrix Extension Registers")
+        return sve_registers.GetChildMemberWithName("svg").GetValueAsUnsigned()
+
+    def do_svg_test(self, process, vgs, expected_svgs):
+        for vg, svg in zip(vgs, expected_svgs):
+            self.runCmd("register write vg {}".format(vg))
+            self.assertEqual(svg, self.read_svg(process))
+
+    @no_debug_info_test
+    @skipIf(archs=no_match(["aarch64"]))
+    @skipIf(oslist=no_match(["linux"]))
+    def test_svg_sve_mode(self):
+        """ When in SVE mode, svg should remain constant as we change vg. """
+        process, supported_vg = self.setup_svg_test(Mode.SVE)
+        svg = self.read_svg(process)
+        self.do_svg_test(process, supported_vg, [svg]*len(supported_vg))
+
+    @no_debug_info_test
+    @skipIf(archs=no_match(["aarch64"]))
+    @skipIf(oslist=no_match(["linux"]))
+    def test_svg_ssve_mode(self):
+        """ When in SSVE mode, changing vg should change svg to the same value. """
+        process, supported_vg = self.setup_svg_test(Mode.SSVE)
+        self.do_svg_test(process, supported_vg, supported_vg)
\ No newline at end of file
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===================================================================
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -84,7 +84,7 @@
     DEFINE_EXTENSION_REG(tpidr2)};
 
 static lldb_private::RegisterInfo g_register_infos_sme[] = {
-    DEFINE_EXTENSION_REG(svcr)};
+    DEFINE_EXTENSION_REG(svcr), DEFINE_EXTENSION_REG(svg)};
 
 // Number of register sets provided by this context.
 enum {
@@ -94,7 +94,7 @@
   k_num_mte_register = 1,
   // Number of TLS registers is dynamic so it is not listed here.
   k_num_pauth_register = 2,
-  k_num_sme_register = 1,
+  k_num_sme_register = 2,
   k_num_register_sets_default = 2,
   k_num_register_sets = 3
 };
@@ -357,15 +357,18 @@
 
 void RegisterInfoPOSIX_arm64::AddRegSetSME() {
   uint32_t sme_regnum = m_dynamic_reg_infos.size();
-  m_sme_regnum_collection.push_back(sme_regnum);
-  m_dynamic_reg_infos.push_back(g_register_infos_sme[0]);
-  m_dynamic_reg_infos[sme_regnum].byte_offset =
-      m_dynamic_reg_infos[sme_regnum - 1].byte_offset +
-      m_dynamic_reg_infos[sme_regnum - 1].byte_size;
-  m_dynamic_reg_infos[sme_regnum].kinds[lldb::eRegisterKindLLDB] = sme_regnum;
+  for (uint32_t i = 0; i < k_num_sme_register; i++) {
+    m_sme_regnum_collection.push_back(sme_regnum + i);
+    m_dynamic_reg_infos.push_back(g_register_infos_sme[i]);
+    m_dynamic_reg_infos[sme_regnum + i].byte_offset =
+        m_dynamic_reg_infos[sme_regnum + i - 1].byte_offset +
+        m_dynamic_reg_infos[sme_regnum + i - 1].byte_size;
+    m_dynamic_reg_infos[sme_regnum + i].kinds[lldb::eRegisterKindLLDB] =
+        sme_regnum + i;
+  }
 
   m_per_regset_regnum_range[m_register_set_count] =
-      std::make_pair(sme_regnum, sme_regnum + 1);
+      std::make_pair(sme_regnum, m_dynamic_reg_infos.size());
   m_dynamic_reg_sets.push_back(g_reg_set_sme_arm64);
   m_dynamic_reg_sets.back().registers = m_sme_regnum_collection.data();
 }
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -127,7 +127,13 @@
   struct user_pac_mask m_pac_mask;
 
   uint64_t m_mte_ctrl_reg;
-  uint64_t m_sme_ctrl_reg;
+
+  struct sme_regs {
+    uint64_t ctrl_reg;
+    uint64_t svg_reg;
+  };
+
+  struct sme_regs m_sme_regs;
 
   struct tls_regs {
     uint64_t tpidr_reg;
@@ -168,6 +174,8 @@
   // SVCR is a pseudo register and we do not allow writes to it.
   Status ReadSMEControl();
 
+  Status ReadSMESVG();
+
   bool IsSVE(unsigned reg) const;
   bool IsPAuth(unsigned reg) const;
   bool IsMTE(unsigned reg) const;
@@ -184,7 +192,7 @@
 
   void *GetTLS() { return &m_tls_regs; }
 
-  void *GetSMEControl() { return &m_sme_ctrl_reg; }
+  void *GetSME() { return &m_sme_regs; }
 
   void *GetSVEBuffer() { return CurrentSVEStateData().m_buffer.data(); }
 
@@ -200,7 +208,7 @@
 
   size_t GetTLSSize() { return m_tls_size; }
 
-  size_t GetSMEControlSize() { return sizeof(m_sme_ctrl_reg); }
+  size_t GetSMESize() { return sizeof(m_sme_regs); }
 
   llvm::Error ReadHardwareDebugInfo() override;
 
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -133,8 +133,8 @@
   ::memset(&m_hbp_regs, 0, sizeof(m_hbp_regs));
   ::memset(&m_pac_mask, 0, sizeof(m_pac_mask));
   ::memset(&m_tls_regs, 0, sizeof(m_tls_regs));
+  ::memset(&m_sme_regs, 0, sizeof(m_sme_regs));
 
-  m_sme_ctrl_reg = 0;
   m_mte_ctrl_reg = 0;
 
   // 16 is just a maximum value, query hardware for actual watchpoint count
@@ -329,9 +329,13 @@
     // This is a pseduo so it never fails.
     ReadSMEControl();
 
+    error = ReadSMESVG();
+    if (error.Fail())
+      return error;
+
     offset = reg_info->byte_offset - GetRegisterInfo().GetSMEOffset();
-    assert(offset < GetSMEControlSize());
-    src = (uint8_t *)GetSMEControl() + offset;
+    assert(offset < GetSMESize());
+    src = (uint8_t *)GetSME() + offset;
   } else
     return Status("failed - register wasn't recognized to be a GPR or an FPR, "
                   "write strategy unknown");
@@ -534,7 +538,7 @@
 
     return WriteTLS();
   } else if (IsSME(reg)) {
-    return Status("Writing to SVCR is not supported.");
+    return Status("Writing to SVCR or SVG not supported.");
   }
 
   return Status("Failed to write register value");
@@ -1004,10 +1008,26 @@
   // what we know from ptrace.
   // Bit 1 indicates whether streaming mode is active.
   // Bit 2 indicates whether the array storage is active (not yet implemented).
-  m_sme_ctrl_reg = m_sve_state == SVEState::Streaming;
+  m_sme_regs.ctrl_reg = m_sve_state == SVEState::Streaming;
   return {};
 }
 
+Status NativeRegisterContextLinux_arm64::ReadSMESVG() {
+  // This register is the streaming vector length, which needs to be read from
+  // the NT_ARM_SSVE set regardless of the active mode.
+  Status error;
+
+  SVEState previous_sve_state = m_sve_state;
+  m_sve_state = SVEState::Streaming;
+
+  error = ReadSVEHeader();
+  if (error.Success())
+    m_sme_regs.svg_reg = CurrentSVEStateData().m_header.vl / 8;
+
+  m_sve_state = previous_sve_state;
+  return error;
+}
+
 Status NativeRegisterContextLinux_arm64::ReadMTEControl() {
   Status error;
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to