Skip to content

Commit 497759e

Browse files
[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 #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.
1 parent 4f8e766 commit 497759e

File tree

3 files changed

+163
-7
lines changed

3 files changed

+163
-7
lines changed

lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,8 @@ void ABIAArch64::AugmentRegisterInfo(
136136

137137
std::array<std::optional<uint32_t>, 32> x_regs;
138138
std::array<std::optional<uint32_t>, 32> v_regs;
139+
std::array<std::optional<uint32_t>, 32> z_regs;
140+
std::optional<uint32_t> z_byte_size;
139141

140142
for (auto it : llvm::enumerate(regs)) {
141143
lldb_private::DynamicRegisterInfo::Register &info = it.value();
@@ -157,16 +159,44 @@ void ABIAArch64::AugmentRegisterInfo(
157159
x_regs[reg_num] = it.index();
158160
else if (get_reg("v"))
159161
v_regs[reg_num] = it.index();
162+
else if (get_reg("z")) {
163+
z_regs[reg_num] = it.index();
164+
if (!z_byte_size)
165+
z_byte_size = info.byte_size;
166+
}
160167
// if we have at least one subregister, abort
161168
else if (get_reg("w") || get_reg("s") || get_reg("d"))
162169
return;
163170
}
164171

165-
// Create aliases for partial registers: wN for xN, and sN/dN for vN.
172+
// Create aliases for partial registers.
173+
174+
// Wn for Xn.
166175
addPartialRegisters(regs, x_regs, 8, "w{0}", 4, lldb::eEncodingUint,
167176
lldb::eFormatHex);
168-
addPartialRegisters(regs, v_regs, 16, "s{0}", 4, lldb::eEncodingIEEE754,
169-
lldb::eFormatFloat);
170-
addPartialRegisters(regs, v_regs, 16, "d{0}", 8, lldb::eEncodingIEEE754,
171-
lldb::eFormatFloat);
177+
178+
auto bool_predicate = [](const auto &reg_num) { return bool(reg_num); };
179+
bool saw_v_regs = std::any_of(v_regs.begin(), v_regs.end(), bool_predicate);
180+
bool saw_z_regs = std::any_of(z_regs.begin(), z_regs.end(), bool_predicate);
181+
182+
// Sn/Dn for Vn.
183+
if (saw_v_regs) {
184+
addPartialRegisters(regs, v_regs, 16, "s{0}", 4, lldb::eEncodingIEEE754,
185+
lldb::eFormatFloat);
186+
addPartialRegisters(regs, v_regs, 16, "d{0}", 8, lldb::eEncodingIEEE754,
187+
lldb::eFormatFloat);
188+
} else if (saw_z_regs && z_byte_size) {
189+
// When SVE is enabled, some debug stubs will not describe the Neon V
190+
// registers because they can be read from the bottom 128 bits of the SVE
191+
// registers.
192+
193+
// The size used here is the one sent by the debug server. This only needs
194+
// to be correct right now. Later we will rely on the value of vg instead.
195+
addPartialRegisters(regs, z_regs, *z_byte_size, "v{0}", 16,
196+
lldb::eEncodingVector, lldb::eFormatVectorOfUInt8);
197+
addPartialRegisters(regs, z_regs, *z_byte_size, "s{0}", 4,
198+
lldb::eEncodingIEEE754, lldb::eFormatFloat);
199+
addPartialRegisters(regs, z_regs, *z_byte_size, "d{0}", 8,
200+
lldb::eEncodingIEEE754, lldb::eFormatFloat);
201+
}
172202
}

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4716,9 +4716,14 @@ bool ParseRegisters(
47164716
reg_info.encoding = eEncodingIEEE754;
47174717
} else if (gdb_type == "aarch64v" ||
47184718
llvm::StringRef(gdb_type).starts_with("vec") ||
4719-
gdb_type == "i387_ext" || gdb_type == "uint128") {
4719+
gdb_type == "i387_ext" || gdb_type == "uint128" ||
4720+
reg_info.byte_size > 16) {
47204721
// lldb doesn't handle 128-bit uints correctly (for ymm*h), so
4721-
// treat them as vector (similarly to xmm/ymm)
4722+
// treat them as vector (similarly to xmm/ymm).
4723+
// We can fall back to handling anything else <= 128 bit as an
4724+
// unsigned integer, more than that, call it a vector of bytes.
4725+
// This can happen if we don't recognise the type for AArc64 SVE
4726+
// registers.
47224727
reg_info.format = eFormatVectorOfUInt8;
47234728
reg_info.encoding = eEncodingVector;
47244729
} else {
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
""" Check that when a debug server provides XML that only defines SVE Z registers,
2+
and does not include Neon V registers, lldb creates sub-registers to represent
3+
the V registers as the bottom 128 bits of the Z registers.
4+
5+
qemu-aarch64 is one such debug server.
6+
7+
This also doubles as a test that lldb has a fallback path for registers of
8+
unknown type that are > 128 bits, as the SVE registers are here.
9+
"""
10+
11+
from textwrap import dedent
12+
import lldb
13+
from lldbsuite.test.lldbtest import *
14+
from lldbsuite.test.decorators import *
15+
from lldbsuite.test.gdbclientutils import *
16+
from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
17+
18+
19+
class Responder(MockGDBServerResponder):
20+
def __init__(self):
21+
super().__init__()
22+
self.vg = 4
23+
self.pc = 0xA0A0A0A0A0A0A0A0
24+
25+
def qXferRead(self, obj, annex, offset, length):
26+
if annex == "target.xml":
27+
# Note that QEMU sends the current SVE size in XML and the debugger
28+
# then reads vg to know the latest size.
29+
return (
30+
dedent(
31+
"""\
32+
<?xml version="1.0"?>
33+
<target version="1.0">
34+
<architecture>aarch64</architecture>
35+
<feature name="org.gnu.gdb.aarch64.core">
36+
<reg name="pc" regnum="0" bitsize="64"/>
37+
<reg name="vg" regnum="1" bitsize="64"/>
38+
<reg name="z0" regnum="2" bitsize="2048" type="not_a_type"/>
39+
</feature>
40+
</target>"""
41+
),
42+
False,
43+
)
44+
45+
return (None,)
46+
47+
def readRegister(self, regnum):
48+
return "E01"
49+
50+
def readRegisters(self):
51+
return "".join(
52+
[
53+
# 64 bit PC.
54+
f"{self.pc:x}",
55+
# 64 bit vg
56+
f"0{self.vg}00000000000000",
57+
# Enough data for 256 and 512 bit SVE.
58+
"".join([f"{n:02x}" * 4 for n in range(1, 17)]),
59+
]
60+
)
61+
62+
def cont(self):
63+
# vg is expedited so that lldb can resize the SVE registers.
64+
return f"T02thread:1ff0d;threads:1ff0d;thread-pcs:{self.pc};01:0{self.vg}00000000000000;"
65+
66+
def writeRegisters(self, registers_hex):
67+
# We get a block of data containing values in regnum order.
68+
self.vg = int(registers_hex[16:18])
69+
return "OK"
70+
71+
72+
class TestXMLRegisterFlags(GDBRemoteTestBase):
73+
def check_regs(self, vg):
74+
# Each 32 bit chunk repeats n.
75+
z0_value = " ".join(
76+
[" ".join([f"0x{n:02x}"] * 4) for n in range(1, (vg * 2) + 1)]
77+
)
78+
79+
self.expect(
80+
"register read vg z0 v0 s0 d0",
81+
substrs=[
82+
f" vg = 0x000000000000000{vg}\n"
83+
" z0 = {" + z0_value + "}\n"
84+
" v0 = {0x01 0x01 0x01 0x01 0x02 0x02 0x02 0x02 0x03 0x03 0x03 0x03 0x04 0x04 0x04 0x04}\n"
85+
" s0 = 2.36942783E-38\n"
86+
" d0 = 5.3779407333977203E-299\n"
87+
],
88+
)
89+
90+
self.expect("register read s0 --format uint32", substrs=["s0 = {0x01010101}"])
91+
self.expect(
92+
"register read d0 --format uint64",
93+
substrs=["d0 = {0x0202020201010101}"],
94+
)
95+
96+
@skipIfXmlSupportMissing
97+
@skipIfRemote
98+
@skipIfLLVMTargetMissing("AArch64")
99+
def test_v_sub_registers(self):
100+
self.server.responder = Responder()
101+
target = self.dbg.CreateTarget("")
102+
103+
if self.TraceOn():
104+
self.runCmd("log enable gdb-remote packets")
105+
self.addTearDownHook(lambda: self.runCmd("log disable gdb-remote packets"))
106+
107+
process = self.connect(target)
108+
lldbutil.expect_state_changes(
109+
self, self.dbg.GetListener(), process, [lldb.eStateStopped]
110+
)
111+
112+
self.check_regs(4)
113+
114+
# Now increase the SVE length and continue. The mock will respond with a new
115+
# vg and lldb will reconfigure the register defs. This should not break the
116+
# sub-registers.
117+
118+
self.runCmd("register write vg 8")
119+
self.expect("continue", substrs=["stop reason = signal SIGINT"])
120+
121+
self.check_regs(8)

0 commit comments

Comments
 (0)