Skip to content

Commit b989171

Browse files
rcorcsrcorcs
authored
[BOLT] Handle generation of compare and jump sequences (#131949)
This patch fixes the following two issues with the createCmpJE for AArch64: 1. Avoids overwriting the value of the input register RegNo by use XZR as the destination register. subs xzr, RegNo, #Imm which is equivalent to a simple cmp RegNo, #Imm 2. The immediate operand to the Bcc instruction must be EQ instead of #Imm. This patch also adds a new function for createCmpJNE and unit tests for the both createCmpJE and createCmpJNE for X86 and AArch64.
1 parent ae5306f commit b989171

File tree

4 files changed

+144
-1
lines changed

4 files changed

+144
-1
lines changed

‎bolt/include/bolt/Core/MCPlusBuilder.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,6 +1751,15 @@ class MCPlusBuilder {
17511751
return {};
17521752
}
17531753

1754+
/// Create a sequence of instructions to compare contents of a register
1755+
/// \p RegNo to immediate \Imm and jump to \p Target if they are different.
1756+
virtual InstructionListType createCmpJNE(MCPhysReg RegNo, int64_t Imm,
1757+
const MCSymbol *Target,
1758+
MCContext *Ctx) const {
1759+
llvm_unreachable("not implemented");
1760+
return {};
1761+
}
1762+
17541763
/// Creates inline memcpy instruction. If \p ReturnEnd is true, then return
17551764
/// (dest + n) instead of dest.
17561765
virtual InstructionListType createInlineMemcpy(bool ReturnEnd) const {

‎bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1361,17 +1361,47 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
13611361

13621362
int getUncondBranchEncodingSize() const override { return 28; }
13631363

1364+
// This helper function creates the snippet of code that compares a register
1365+
// RegNo with an immedaite Imm, and jumps to Target if they are equal.
1366+
// cmp RegNo, #Imm
1367+
// b.eq Target
1368+
// where cmp is an alias for subs, which results in the code below:
1369+
// subs xzr, RegNo, #Imm
1370+
// b.eq Target.
13641371
InstructionListType createCmpJE(MCPhysReg RegNo, int64_t Imm,
13651372
const MCSymbol *Target,
13661373
MCContext *Ctx) const override {
13671374
InstructionListType Code;
13681375
Code.emplace_back(MCInstBuilder(AArch64::SUBSXri)
1369-
.addReg(RegNo)
1376+
.addReg(AArch64::XZR)
13701377
.addReg(RegNo)
13711378
.addImm(Imm)
13721379
.addImm(0));
13731380
Code.emplace_back(MCInstBuilder(AArch64::Bcc)
1381+
.addImm(AArch64CC::EQ)
1382+
.addExpr(MCSymbolRefExpr::create(
1383+
Target, MCSymbolRefExpr::VK_None, *Ctx)));
1384+
return Code;
1385+
}
1386+
1387+
// This helper function creates the snippet of code that compares a register
1388+
// RegNo with an immedaite Imm, and jumps to Target if they are not equal.
1389+
// cmp RegNo, #Imm
1390+
// b.ne Target
1391+
// where cmp is an alias for subs, which results in the code below:
1392+
// subs xzr, RegNo, #Imm
1393+
// b.ne Target.
1394+
InstructionListType createCmpJNE(MCPhysReg RegNo, int64_t Imm,
1395+
const MCSymbol *Target,
1396+
MCContext *Ctx) const override {
1397+
InstructionListType Code;
1398+
Code.emplace_back(MCInstBuilder(AArch64::SUBSXri)
1399+
.addReg(AArch64::XZR)
1400+
.addReg(RegNo)
13741401
.addImm(Imm)
1402+
.addImm(0));
1403+
Code.emplace_back(MCInstBuilder(AArch64::Bcc)
1404+
.addImm(AArch64CC::NE)
13751405
.addExpr(MCSymbolRefExpr::create(
13761406
Target, MCSymbolRefExpr::VK_None, *Ctx)));
13771407
return Code;

‎bolt/lib/Target/X86/X86MCPlusBuilder.cpp‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2426,6 +2426,18 @@ class X86MCPlusBuilder : public MCPlusBuilder {
24262426
return Code;
24272427
}
24282428

2429+
InstructionListType createCmpJNE(MCPhysReg RegNo, int64_t Imm,
2430+
const MCSymbol *Target,
2431+
MCContext *Ctx) const override {
2432+
InstructionListType Code;
2433+
Code.emplace_back(MCInstBuilder(X86::CMP64ri8).addReg(RegNo).addImm(Imm));
2434+
Code.emplace_back(MCInstBuilder(X86::JCC_1)
2435+
.addExpr(MCSymbolRefExpr::create(
2436+
Target, MCSymbolRefExpr::VK_None, *Ctx))
2437+
.addImm(X86::COND_NE));
2438+
return Code;
2439+
}
2440+
24292441
std::optional<Relocation>
24302442
createRelocation(const MCFixup &Fixup,
24312443
const MCAsmBackend &MAB) const override {

‎bolt/unittests/Core/MCPlusBuilder.cpp‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,54 @@ TEST_P(MCPlusBuilderTester, AliasSmallerX0) {
107107
testRegAliases(Triple::aarch64, AArch64::X0, AliasesX0, AliasesX0Count, true);
108108
}
109109

110+
TEST_P(MCPlusBuilderTester, AArch64_CmpJE) {
111+
if (GetParam() != Triple::aarch64)
112+
GTEST_SKIP();
113+
BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true);
114+
std::unique_ptr<BinaryBasicBlock> BB = BF->createBasicBlock();
115+
116+
InstructionListType Instrs =
117+
BC->MIB->createCmpJE(AArch64::X0, 2, BB->getLabel(), BC->Ctx.get());
118+
BB->addInstructions(Instrs.begin(), Instrs.end());
119+
BB->addSuccessor(BB.get());
120+
121+
auto II = BB->begin();
122+
ASSERT_EQ(II->getOpcode(), AArch64::SUBSXri);
123+
ASSERT_EQ(II->getOperand(0).getReg(), AArch64::XZR);
124+
ASSERT_EQ(II->getOperand(1).getReg(), AArch64::X0);
125+
ASSERT_EQ(II->getOperand(2).getImm(), 2);
126+
ASSERT_EQ(II->getOperand(3).getImm(), 0);
127+
II++;
128+
ASSERT_EQ(II->getOpcode(), AArch64::Bcc);
129+
ASSERT_EQ(II->getOperand(0).getImm(), AArch64CC::EQ);
130+
const MCSymbol *Label = BC->MIB->getTargetSymbol(*II, 1);
131+
ASSERT_EQ(Label, BB->getLabel());
132+
}
133+
134+
TEST_P(MCPlusBuilderTester, AArch64_CmpJNE) {
135+
if (GetParam() != Triple::aarch64)
136+
GTEST_SKIP();
137+
BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true);
138+
std::unique_ptr<BinaryBasicBlock> BB = BF->createBasicBlock();
139+
140+
InstructionListType Instrs =
141+
BC->MIB->createCmpJNE(AArch64::X0, 2, BB->getLabel(), BC->Ctx.get());
142+
BB->addInstructions(Instrs.begin(), Instrs.end());
143+
BB->addSuccessor(BB.get());
144+
145+
auto II = BB->begin();
146+
ASSERT_EQ(II->getOpcode(), AArch64::SUBSXri);
147+
ASSERT_EQ(II->getOperand(0).getReg(), AArch64::XZR);
148+
ASSERT_EQ(II->getOperand(1).getReg(), AArch64::X0);
149+
ASSERT_EQ(II->getOperand(2).getImm(), 2);
150+
ASSERT_EQ(II->getOperand(3).getImm(), 0);
151+
II++;
152+
ASSERT_EQ(II->getOpcode(), AArch64::Bcc);
153+
ASSERT_EQ(II->getOperand(0).getImm(), AArch64CC::NE);
154+
const MCSymbol *Label = BC->MIB->getTargetSymbol(*II, 1);
155+
ASSERT_EQ(Label, BB->getLabel());
156+
}
157+
110158
#endif // AARCH64_AVAILABLE
111159

112160
#ifdef X86_AVAILABLE
@@ -143,6 +191,50 @@ TEST_P(MCPlusBuilderTester, ReplaceRegWithImm) {
143191
ASSERT_EQ(II->getOperand(1).getImm(), 1);
144192
}
145193

194+
TEST_P(MCPlusBuilderTester, X86_CmpJE) {
195+
if (GetParam() != Triple::x86_64)
196+
GTEST_SKIP();
197+
BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true);
198+
std::unique_ptr<BinaryBasicBlock> BB = BF->createBasicBlock();
199+
200+
InstructionListType Instrs =
201+
BC->MIB->createCmpJE(X86::EAX, 2, BB->getLabel(), BC->Ctx.get());
202+
BB->addInstructions(Instrs.begin(), Instrs.end());
203+
BB->addSuccessor(BB.get());
204+
205+
auto II = BB->begin();
206+
ASSERT_EQ(II->getOpcode(), X86::CMP64ri8);
207+
ASSERT_EQ(II->getOperand(0).getReg(), X86::EAX);
208+
ASSERT_EQ(II->getOperand(1).getImm(), 2);
209+
II++;
210+
ASSERT_EQ(II->getOpcode(), X86::JCC_1);
211+
const MCSymbol *Label = BC->MIB->getTargetSymbol(*II, 0);
212+
ASSERT_EQ(Label, BB->getLabel());
213+
ASSERT_EQ(II->getOperand(1).getImm(), X86::COND_E);
214+
}
215+
216+
TEST_P(MCPlusBuilderTester, X86_CmpJNE) {
217+
if (GetParam() != Triple::x86_64)
218+
GTEST_SKIP();
219+
BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true);
220+
std::unique_ptr<BinaryBasicBlock> BB = BF->createBasicBlock();
221+
222+
InstructionListType Instrs =
223+
BC->MIB->createCmpJNE(X86::EAX, 2, BB->getLabel(), BC->Ctx.get());
224+
BB->addInstructions(Instrs.begin(), Instrs.end());
225+
BB->addSuccessor(BB.get());
226+
227+
auto II = BB->begin();
228+
ASSERT_EQ(II->getOpcode(), X86::CMP64ri8);
229+
ASSERT_EQ(II->getOperand(0).getReg(), X86::EAX);
230+
ASSERT_EQ(II->getOperand(1).getImm(), 2);
231+
II++;
232+
ASSERT_EQ(II->getOpcode(), X86::JCC_1);
233+
const MCSymbol *Label = BC->MIB->getTargetSymbol(*II, 0);
234+
ASSERT_EQ(Label, BB->getLabel());
235+
ASSERT_EQ(II->getOperand(1).getImm(), X86::COND_NE);
236+
}
237+
146238
#endif // X86_AVAILABLE
147239

148240
TEST_P(MCPlusBuilderTester, Annotation) {

0 commit comments

Comments
 (0)