[Lldb-commits] [lldb] 497759e - [lldb][AArch64] Create Neon subregs when XML only includes SVE (#108365)

via lldb-commits lldb-commits at lists.llvm.org
Tue Sep 24 04:40:45 PDT 2024


Author: David Spickett
Date: 2024-09-24T12:40:42+01:00
New Revision: 497759e872a53964a54db941f3a1ed74446c5ed4

URL: https://github.com/llvm/llvm-project/commit/497759e872a53964a54db941f3a1ed74446c5ed4
DIFF: https://github.com/llvm/llvm-project/commit/497759e872a53964a54db941f3a1ed74446c5ed4.diff

LOG: [lldb][AArch64] Create Neon subregs when XML only includes SVE (#108365)

Fixes #107864

QEMU decided that when SVE is enabled it will only tell us about SVE
registers in the XML, and not include Neon registers. On the grounds
that the Neon V registers can be read from the bottom 128 bits of a SVE
Z register (SVE's vector length is always >= 128 bits).

To support this we create sub-registers just as we do for S and D
registers of the V registers. Except this time we use part of the Z
registers.

This change also updates our fallback for registers with unknown types
that are > 128 bit. This is detailed in
https://github.com/llvm/llvm-project/issues/87471, though that covers
more than this change fixes.

We'll now treat any register of unknown type that is >= 128 bit as a
vector of bytes. So that the user gets to see something
even if the order might be wrong.

And until lldb supports vector and union types for registers, this is
also the only way we can get a value to apply the sub-reg to, to make
the V registers.

Added: 
    lldb/test/API/functionalities/gdb_remote_client/TestAArch64XMLRegistersSVEOnly.py

Modified: 
    lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
    lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp b/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
index 256c1f828feb38..7d8d0a4d3d6711 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
@@ -136,6 +136,8 @@ void ABIAArch64::AugmentRegisterInfo(
 
   std::array<std::optional<uint32_t>, 32> x_regs;
   std::array<std::optional<uint32_t>, 32> v_regs;
+  std::array<std::optional<uint32_t>, 32> z_regs;
+  std::optional<uint32_t> z_byte_size;
 
   for (auto it : llvm::enumerate(regs)) {
     lldb_private::DynamicRegisterInfo::Register &info = it.value();
@@ -157,16 +159,44 @@ void ABIAArch64::AugmentRegisterInfo(
       x_regs[reg_num] = it.index();
     else if (get_reg("v"))
       v_regs[reg_num] = it.index();
+    else if (get_reg("z")) {
+      z_regs[reg_num] = it.index();
+      if (!z_byte_size)
+        z_byte_size = info.byte_size;
+    }
     // if we have at least one subregister, abort
     else if (get_reg("w") || get_reg("s") || get_reg("d"))
       return;
   }
 
-  // Create aliases for partial registers: wN for xN, and sN/dN for vN.
+  // Create aliases for partial registers.
+
+  // Wn for Xn.
   addPartialRegisters(regs, x_regs, 8, "w{0}", 4, lldb::eEncodingUint,
                       lldb::eFormatHex);
-  addPartialRegisters(regs, v_regs, 16, "s{0}", 4, lldb::eEncodingIEEE754,
-                      lldb::eFormatFloat);
-  addPartialRegisters(regs, v_regs, 16, "d{0}", 8, lldb::eEncodingIEEE754,
-                      lldb::eFormatFloat);
+
+  auto bool_predicate = [](const auto &reg_num) { return bool(reg_num); };
+  bool saw_v_regs = std::any_of(v_regs.begin(), v_regs.end(), bool_predicate);
+  bool saw_z_regs = std::any_of(z_regs.begin(), z_regs.end(), bool_predicate);
+
+  // Sn/Dn for Vn.
+  if (saw_v_regs) {
+    addPartialRegisters(regs, v_regs, 16, "s{0}", 4, lldb::eEncodingIEEE754,
+                        lldb::eFormatFloat);
+    addPartialRegisters(regs, v_regs, 16, "d{0}", 8, lldb::eEncodingIEEE754,
+                        lldb::eFormatFloat);
+  } else if (saw_z_regs && z_byte_size) {
+    // When SVE is enabled, some debug stubs will not describe the Neon V
+    // registers because they can be read from the bottom 128 bits of the SVE
+    // registers.
+
+    // The size used here is the one sent by the debug server. This only needs
+    // to be correct right now. Later we will rely on the value of vg instead.
+    addPartialRegisters(regs, z_regs, *z_byte_size, "v{0}", 16,
+                        lldb::eEncodingVector, lldb::eFormatVectorOfUInt8);
+    addPartialRegisters(regs, z_regs, *z_byte_size, "s{0}", 4,
+                        lldb::eEncodingIEEE754, lldb::eFormatFloat);
+    addPartialRegisters(regs, z_regs, *z_byte_size, "d{0}", 8,
+                        lldb::eEncodingIEEE754, lldb::eFormatFloat);
+  }
 }

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 9e8c6046179631..3e09c316d74f44 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4716,9 +4716,14 @@ bool ParseRegisters(
               reg_info.encoding = eEncodingIEEE754;
             } else if (gdb_type == "aarch64v" ||
                        llvm::StringRef(gdb_type).starts_with("vec") ||
-                       gdb_type == "i387_ext" || gdb_type == "uint128") {
+                       gdb_type == "i387_ext" || gdb_type == "uint128" ||
+                       reg_info.byte_size > 16) {
               // lldb doesn't handle 128-bit uints correctly (for ymm*h), so
-              // treat them as vector (similarly to xmm/ymm)
+              // treat them as vector (similarly to xmm/ymm).
+              // We can fall back to handling anything else <= 128 bit as an
+              // unsigned integer, more than that, call it a vector of bytes.
+              // This can happen if we don't recognise the type for AArc64 SVE
+              // registers.
               reg_info.format = eFormatVectorOfUInt8;
               reg_info.encoding = eEncodingVector;
             } else {

diff  --git a/lldb/test/API/functionalities/gdb_remote_client/TestAArch64XMLRegistersSVEOnly.py b/lldb/test/API/functionalities/gdb_remote_client/TestAArch64XMLRegistersSVEOnly.py
new file mode 100644
index 00000000000000..e36013a11491b3
--- /dev/null
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestAArch64XMLRegistersSVEOnly.py
@@ -0,0 +1,121 @@
+""" Check that when a debug server provides XML that only defines SVE Z registers,
+    and does not include Neon V registers, lldb creates sub-registers to represent
+    the V registers as the bottom 128 bits of the Z registers.
+
+    qemu-aarch64 is one such debug server.
+
+    This also doubles as a test that lldb has a fallback path for registers of
+    unknown type that are > 128 bits, as the SVE registers are here.
+"""
+
+from textwrap import dedent
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class Responder(MockGDBServerResponder):
+    def __init__(self):
+        super().__init__()
+        self.vg = 4
+        self.pc = 0xA0A0A0A0A0A0A0A0
+
+    def qXferRead(self, obj, annex, offset, length):
+        if annex == "target.xml":
+            # Note that QEMU sends the current SVE size in XML and the debugger
+            # then reads vg to know the latest size.
+            return (
+                dedent(
+                    """\
+              <?xml version="1.0"?>
+              <target version="1.0">
+                <architecture>aarch64</architecture>
+                <feature name="org.gnu.gdb.aarch64.core">
+                  <reg name="pc" regnum="0" bitsize="64"/>
+                  <reg name="vg" regnum="1" bitsize="64"/>
+                  <reg name="z0" regnum="2" bitsize="2048" type="not_a_type"/>
+                </feature>
+              </target>"""
+                ),
+                False,
+            )
+
+        return (None,)
+
+    def readRegister(self, regnum):
+        return "E01"
+
+    def readRegisters(self):
+        return "".join(
+            [
+                # 64 bit PC.
+                f"{self.pc:x}",
+                # 64 bit vg
+                f"0{self.vg}00000000000000",
+                # Enough data for 256 and 512 bit SVE.
+                "".join([f"{n:02x}" * 4 for n in range(1, 17)]),
+            ]
+        )
+
+    def cont(self):
+        # vg is expedited so that lldb can resize the SVE registers.
+        return f"T02thread:1ff0d;threads:1ff0d;thread-pcs:{self.pc};01:0{self.vg}00000000000000;"
+
+    def writeRegisters(self, registers_hex):
+        # We get a block of data containing values in regnum order.
+        self.vg = int(registers_hex[16:18])
+        return "OK"
+
+
+class TestXMLRegisterFlags(GDBRemoteTestBase):
+    def check_regs(self, vg):
+        # Each 32 bit chunk repeats n.
+        z0_value = " ".join(
+            [" ".join([f"0x{n:02x}"] * 4) for n in range(1, (vg * 2) + 1)]
+        )
+
+        self.expect(
+            "register read vg z0 v0 s0 d0",
+            substrs=[
+                f"      vg = 0x000000000000000{vg}\n"
+                "      z0 = {" + z0_value + "}\n"
+                "      v0 = {0x01 0x01 0x01 0x01 0x02 0x02 0x02 0x02 0x03 0x03 0x03 0x03 0x04 0x04 0x04 0x04}\n"
+                "      s0 = 2.36942783E-38\n"
+                "      d0 = 5.3779407333977203E-299\n"
+            ],
+        )
+
+        self.expect("register read s0 --format uint32", substrs=["s0 = {0x01010101}"])
+        self.expect(
+            "register read d0 --format uint64",
+            substrs=["d0 = {0x0202020201010101}"],
+        )
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    @skipIfLLVMTargetMissing("AArch64")
+    def test_v_sub_registers(self):
+        self.server.responder = Responder()
+        target = self.dbg.CreateTarget("")
+
+        if self.TraceOn():
+            self.runCmd("log enable gdb-remote packets")
+            self.addTearDownHook(lambda: self.runCmd("log disable gdb-remote packets"))
+
+        process = self.connect(target)
+        lldbutil.expect_state_changes(
+            self, self.dbg.GetListener(), process, [lldb.eStateStopped]
+        )
+
+        self.check_regs(4)
+
+        # Now increase the SVE length and continue. The mock will respond with a new
+        # vg and lldb will reconfigure the register defs. This should not break the
+        # sub-registers.
+
+        self.runCmd("register write vg 8")
+        self.expect("continue", substrs=["stop reason = signal SIGINT"])
+
+        self.check_regs(8)


        


More information about the lldb-commits mailing list