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

David Spickett via lldb-commits lldb-commits at lists.llvm.org
Thu Sep 12 04:01:46 PDT 2024


https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/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.

>From a4e984b8a5027ff189b113a292b016a2ab37688d Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 10 Sep 2024 13:36:05 +0000
Subject: [PATCH] [lldb][AArch64] Create Neon subregs when XML only includes
 SVE

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.
---
 .../source/Plugins/ABI/AArch64/ABIAArch64.cpp |  40 +++++-
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |   9 +-
 .../TestAArch64XMLRegistersSVEOnly.py         | 121 ++++++++++++++++++
 3 files changed, 163 insertions(+), 7 deletions(-)
 create mode 100644 lldb/test/API/functionalities/gdb_remote_client/TestAArch64XMLRegistersSVEOnly.py

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 5eaf9ce2a302aa..342a1ec090b3fa 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4715,9 +4715,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