Skip to content

Commit 37e8c6c

Browse files
authored
[BOLT] Do not return Def-ed registers from MCPlusBuilder::getUsedRegs (#129890)
Update the implementation of `MCPlusBuilder::getUsedRegs` to match its description in the header file, add unit tests.
1 parent 15d8b3c commit 37e8c6c

File tree

2 files changed

+125
-21
lines changed

2 files changed

+125
-21
lines changed

bolt/lib/Core/MCPlusBuilder.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -442,10 +442,10 @@ void MCPlusBuilder::getUsedRegs(const MCInst &Inst, BitVector &Regs) const {
442442
for (MCPhysReg ImplicitUse : InstInfo.implicit_uses())
443443
Regs |= getAliases(ImplicitUse, /*OnlySmaller=*/true);
444444

445-
for (unsigned I = 0, E = Inst.getNumOperands(); I != E; ++I) {
446-
if (!Inst.getOperand(I).isReg())
445+
for (const MCOperand &Operand : useOperands(Inst)) {
446+
if (!Operand.isReg())
447447
continue;
448-
Regs |= getAliases(Inst.getOperand(I).getReg(), /*OnlySmaller=*/true);
448+
Regs |= getAliases(Operand.getReg(), /*OnlySmaller=*/true);
449449
}
450450
}
451451

bolt/unittests/Core/MCPlusBuilder.cpp

Lines changed: 122 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#ifdef AARCH64_AVAILABLE
1010
#include "AArch64Subtarget.h"
11+
#include "MCTargetDesc/AArch64MCTargetDesc.h"
1112
#endif // AARCH64_AVAILABLE
1213

1314
#ifdef X86_AVAILABLE
@@ -19,6 +20,7 @@
1920
#include "bolt/Rewrite/RewriteInstance.h"
2021
#include "llvm/BinaryFormat/ELF.h"
2122
#include "llvm/DebugInfo/DWARF/DWARFContext.h"
23+
#include "llvm/MC/MCInstBuilder.h"
2224
#include "llvm/Support/TargetSelect.h"
2325
#include "gtest/gtest.h"
2426

@@ -70,16 +72,28 @@ struct MCPlusBuilderTester : public testing::TestWithParam<Triple::ArchType> {
7072
BC->MRI.get(), BC->STI.get())));
7173
}
7274

75+
void assertRegMask(const BitVector &RegMask,
76+
std::initializer_list<MCPhysReg> ExpectedRegs) {
77+
ASSERT_EQ(RegMask.count(), ExpectedRegs.size());
78+
for (MCPhysReg Reg : ExpectedRegs)
79+
ASSERT_TRUE(RegMask[Reg]) << "Expected " << BC->MRI->getName(Reg) << ".";
80+
}
81+
82+
void assertRegMask(std::function<void(BitVector &)> FillRegMask,
83+
std::initializer_list<MCPhysReg> ExpectedRegs) {
84+
BitVector RegMask(BC->MRI->getNumRegs());
85+
FillRegMask(RegMask);
86+
assertRegMask(RegMask, ExpectedRegs);
87+
}
88+
7389
void testRegAliases(Triple::ArchType Arch, uint64_t Register,
74-
uint64_t *Aliases, size_t Count,
90+
std::initializer_list<MCPhysReg> ExpectedAliases,
7591
bool OnlySmaller = false) {
7692
if (GetParam() != Arch)
7793
GTEST_SKIP();
7894

7995
const BitVector &BV = BC->MIB->getAliases(Register, OnlySmaller);
80-
ASSERT_EQ(BV.count(), Count);
81-
for (size_t I = 0; I < Count; ++I)
82-
ASSERT_TRUE(BV[Aliases[I]]);
96+
assertRegMask(BV, ExpectedAliases);
8397
}
8498

8599
char ElfBuf[sizeof(typename ELF64LE::Ehdr)] = {};
@@ -94,17 +108,15 @@ INSTANTIATE_TEST_SUITE_P(AArch64, MCPlusBuilderTester,
94108
::testing::Values(Triple::aarch64));
95109

96110
TEST_P(MCPlusBuilderTester, AliasX0) {
97-
uint64_t AliasesX0[] = {AArch64::W0, AArch64::W0_HI,
98-
AArch64::X0, AArch64::W0_W1,
99-
AArch64::X0_X1, AArch64::X0_X1_X2_X3_X4_X5_X6_X7};
100-
size_t AliasesX0Count = sizeof(AliasesX0) / sizeof(*AliasesX0);
101-
testRegAliases(Triple::aarch64, AArch64::X0, AliasesX0, AliasesX0Count);
111+
testRegAliases(Triple::aarch64, AArch64::X0,
112+
{AArch64::W0, AArch64::W0_HI, AArch64::X0, AArch64::W0_W1,
113+
AArch64::X0_X1, AArch64::X0_X1_X2_X3_X4_X5_X6_X7});
102114
}
103115

104116
TEST_P(MCPlusBuilderTester, AliasSmallerX0) {
105-
uint64_t AliasesX0[] = {AArch64::W0, AArch64::W0_HI, AArch64::X0};
106-
size_t AliasesX0Count = sizeof(AliasesX0) / sizeof(*AliasesX0);
107-
testRegAliases(Triple::aarch64, AArch64::X0, AliasesX0, AliasesX0Count, true);
117+
testRegAliases(Triple::aarch64, AArch64::X0,
118+
{AArch64::W0, AArch64::W0_HI, AArch64::X0},
119+
/*OnlySmaller=*/true);
108120
}
109121

110122
TEST_P(MCPlusBuilderTester, AArch64_CmpJE) {
@@ -155,6 +167,100 @@ TEST_P(MCPlusBuilderTester, AArch64_CmpJNE) {
155167
ASSERT_EQ(Label, BB->getLabel());
156168
}
157169

170+
TEST_P(MCPlusBuilderTester, testAccessedRegsImplicitDef) {
171+
if (GetParam() != Triple::aarch64)
172+
GTEST_SKIP();
173+
174+
// adds x0, x5, #42
175+
MCInst Inst = MCInstBuilder(AArch64::ADDSXri)
176+
.addReg(AArch64::X0)
177+
.addReg(AArch64::X5)
178+
.addImm(42)
179+
.addImm(0);
180+
181+
assertRegMask([&](BitVector &BV) { BC->MIB->getClobberedRegs(Inst, BV); },
182+
{AArch64::NZCV, AArch64::W0, AArch64::X0, AArch64::W0_HI,
183+
AArch64::X0_X1_X2_X3_X4_X5_X6_X7, AArch64::W0_W1,
184+
AArch64::X0_X1});
185+
186+
assertRegMask(
187+
[&](BitVector &BV) { BC->MIB->getTouchedRegs(Inst, BV); },
188+
{AArch64::NZCV, AArch64::W0, AArch64::W5, AArch64::X0, AArch64::X5,
189+
AArch64::W0_HI, AArch64::W5_HI, AArch64::X0_X1_X2_X3_X4_X5_X6_X7,
190+
AArch64::X2_X3_X4_X5_X6_X7_X8_X9, AArch64::X4_X5_X6_X7_X8_X9_X10_X11,
191+
AArch64::W0_W1, AArch64::W4_W5, AArch64::X0_X1, AArch64::X4_X5});
192+
193+
assertRegMask([&](BitVector &BV) { BC->MIB->getWrittenRegs(Inst, BV); },
194+
{AArch64::NZCV, AArch64::W0, AArch64::X0, AArch64::W0_HI});
195+
196+
assertRegMask([&](BitVector &BV) { BC->MIB->getUsedRegs(Inst, BV); },
197+
{AArch64::W5, AArch64::X5, AArch64::W5_HI});
198+
199+
assertRegMask([&](BitVector &BV) { BC->MIB->getSrcRegs(Inst, BV); },
200+
{AArch64::W5, AArch64::X5, AArch64::W5_HI});
201+
}
202+
203+
TEST_P(MCPlusBuilderTester, testAccessedRegsImplicitUse) {
204+
if (GetParam() != Triple::aarch64)
205+
GTEST_SKIP();
206+
207+
// b.eq <label>
208+
MCInst Inst =
209+
MCInstBuilder(AArch64::Bcc)
210+
.addImm(AArch64CC::EQ)
211+
.addImm(0); // <label> - should be Expr, but immediate 0 works too.
212+
213+
assertRegMask([&](BitVector &BV) { BC->MIB->getClobberedRegs(Inst, BV); },
214+
{});
215+
216+
assertRegMask([&](BitVector &BV) { BC->MIB->getTouchedRegs(Inst, BV); },
217+
{AArch64::NZCV});
218+
219+
assertRegMask([&](BitVector &BV) { BC->MIB->getWrittenRegs(Inst, BV); }, {});
220+
221+
assertRegMask([&](BitVector &BV) { BC->MIB->getUsedRegs(Inst, BV); },
222+
{AArch64::NZCV});
223+
224+
assertRegMask([&](BitVector &BV) { BC->MIB->getSrcRegs(Inst, BV); },
225+
{AArch64::NZCV});
226+
}
227+
228+
TEST_P(MCPlusBuilderTester, testAccessedRegsMultipleDefs) {
229+
if (GetParam() != Triple::aarch64)
230+
GTEST_SKIP();
231+
232+
// ldr x0, [x5], #16
233+
MCInst Inst = MCInstBuilder(AArch64::LDRXpost)
234+
.addReg(AArch64::X5)
235+
.addReg(AArch64::X0)
236+
.addReg(AArch64::X5)
237+
.addImm(16);
238+
239+
assertRegMask(
240+
[&](BitVector &BV) { BC->MIB->getClobberedRegs(Inst, BV); },
241+
{AArch64::W0, AArch64::W5, AArch64::X0, AArch64::X5, AArch64::W0_HI,
242+
AArch64::W5_HI, AArch64::X0_X1_X2_X3_X4_X5_X6_X7,
243+
AArch64::X2_X3_X4_X5_X6_X7_X8_X9, AArch64::X4_X5_X6_X7_X8_X9_X10_X11,
244+
AArch64::W0_W1, AArch64::W4_W5, AArch64::X0_X1, AArch64::X4_X5});
245+
246+
assertRegMask(
247+
[&](BitVector &BV) { BC->MIB->getTouchedRegs(Inst, BV); },
248+
{AArch64::W0, AArch64::W5, AArch64::X0, AArch64::X5, AArch64::W0_HI,
249+
AArch64::W5_HI, AArch64::X0_X1_X2_X3_X4_X5_X6_X7,
250+
AArch64::X2_X3_X4_X5_X6_X7_X8_X9, AArch64::X4_X5_X6_X7_X8_X9_X10_X11,
251+
AArch64::W0_W1, AArch64::W4_W5, AArch64::X0_X1, AArch64::X4_X5});
252+
253+
assertRegMask([&](BitVector &BV) { BC->MIB->getWrittenRegs(Inst, BV); },
254+
{AArch64::W0, AArch64::X0, AArch64::W0_HI, AArch64::W5,
255+
AArch64::X5, AArch64::W5_HI});
256+
257+
assertRegMask([&](BitVector &BV) { BC->MIB->getUsedRegs(Inst, BV); },
258+
{AArch64::W5, AArch64::X5, AArch64::W5_HI});
259+
260+
assertRegMask([&](BitVector &BV) { BC->MIB->getSrcRegs(Inst, BV); },
261+
{AArch64::W5, AArch64::X5, AArch64::W5_HI});
262+
}
263+
158264
#endif // AARCH64_AVAILABLE
159265

160266
#ifdef X86_AVAILABLE
@@ -163,15 +269,13 @@ INSTANTIATE_TEST_SUITE_P(X86, MCPlusBuilderTester,
163269
::testing::Values(Triple::x86_64));
164270

165271
TEST_P(MCPlusBuilderTester, AliasAX) {
166-
uint64_t AliasesAX[] = {X86::RAX, X86::EAX, X86::AX, X86::AL, X86::AH};
167-
size_t AliasesAXCount = sizeof(AliasesAX) / sizeof(*AliasesAX);
168-
testRegAliases(Triple::x86_64, X86::AX, AliasesAX, AliasesAXCount);
272+
testRegAliases(Triple::x86_64, X86::AX,
273+
{X86::RAX, X86::EAX, X86::AX, X86::AL, X86::AH});
169274
}
170275

171276
TEST_P(MCPlusBuilderTester, AliasSmallerAX) {
172-
uint64_t AliasesAX[] = {X86::AX, X86::AL, X86::AH};
173-
size_t AliasesAXCount = sizeof(AliasesAX) / sizeof(*AliasesAX);
174-
testRegAliases(Triple::x86_64, X86::AX, AliasesAX, AliasesAXCount, true);
277+
testRegAliases(Triple::x86_64, X86::AX, {X86::AX, X86::AL, X86::AH},
278+
/*OnlySmaller=*/true);
175279
}
176280

177281
TEST_P(MCPlusBuilderTester, ReplaceRegWithImm) {

0 commit comments

Comments
 (0)