From 7ca8b3173a6453ad3dfdffe7581aa01561fb1814 Mon Sep 17 00:00:00 2001 From: Rodrigo Rocha Date: Wed, 19 Mar 2025 01:22:30 +0000 Subject: [PATCH 1/3] [BOLT] Compare and Jump generation in MCPlusBuilder. --- bolt/include/bolt/Core/MCPlusBuilder.h | 10 +++ .../Target/AArch64/AArch64MCPlusBuilder.cpp | 34 ++++++- bolt/lib/Target/X86/X86MCPlusBuilder.cpp | 14 +++ bolt/unittests/Core/MCPlusBuilder.cpp | 88 +++++++++++++++++++ 4 files changed, 145 insertions(+), 1 deletion(-) diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h index 1d45a314a17b6..a9d79112b641a 100644 --- a/bolt/include/bolt/Core/MCPlusBuilder.h +++ b/bolt/include/bolt/Core/MCPlusBuilder.h @@ -1714,6 +1714,16 @@ class MCPlusBuilder { return {}; } + /// Create a sequence of instructions to compare contents of a register + /// \p RegNo to immediate \Imm and jump to \p Target if they are different. + virtual InstructionListType createCmpJNE(MCPhysReg RegNo, int64_t Imm, + const MCSymbol *Target, + MCContext *Ctx) const { + llvm_unreachable("not implemented"); + return {}; + } + + /// Creates inline memcpy instruction. If \p ReturnEnd is true, then return /// (dest + n) instead of dest. virtual InstructionListType createInlineMemcpy(bool ReturnEnd) const { diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp index 685b2279e5afb..bb627ce2586b0 100644 --- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp +++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp @@ -1321,17 +1321,49 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { int getUncondBranchEncodingSize() const override { return 28; } + //This helper function creates the snippet of code + //that compares a register RegNo with an immedaite Imm, + //and jumps to Target if they are equal. + //cmp RegNo, #Imm + //b.eq Target + //where cmp is an for subs, which results in the code below: + //subs xzr, RegNo, #Imm + //b.eq Target. InstructionListType createCmpJE(MCPhysReg RegNo, int64_t Imm, const MCSymbol *Target, MCContext *Ctx) const override { InstructionListType Code; Code.emplace_back(MCInstBuilder(AArch64::SUBSXri) - .addReg(RegNo) + .addReg(AArch64::XZR) .addReg(RegNo) .addImm(Imm) .addImm(0)); Code.emplace_back(MCInstBuilder(AArch64::Bcc) + .addImm(AArch64CC::EQ) + .addExpr(MCSymbolRefExpr::create( + Target, MCSymbolRefExpr::VK_None, *Ctx))); + return Code; + } + + //This helper function creates the snippet of code + //that compares a register RegNo with an immedaite Imm, + //and jumps to Target if they are not equal. + //cmp RegNo, #Imm + //b.ne Target + //where cmp is an for subs, which results in the code below: + //subs xzr, RegNo, #Imm + //b.ne Target. + InstructionListType createCmpJNE(MCPhysReg RegNo, int64_t Imm, + const MCSymbol *Target, + MCContext *Ctx) const override { + InstructionListType Code; + Code.emplace_back(MCInstBuilder(AArch64::SUBSXri) + .addReg(AArch64::XZR) + .addReg(RegNo) .addImm(Imm) + .addImm(0)); + Code.emplace_back(MCInstBuilder(AArch64::Bcc) + .addImm(AArch64CC::NE) .addExpr(MCSymbolRefExpr::create( Target, MCSymbolRefExpr::VK_None, *Ctx))); return Code; diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp index 465533ee71f2b..bc827e6fd6f1e 100644 --- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp +++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp @@ -2436,6 +2436,20 @@ class X86MCPlusBuilder : public MCPlusBuilder { return Code; } + InstructionListType createCmpJNE(MCPhysReg RegNo, int64_t Imm, + const MCSymbol *Target, + MCContext *Ctx) const override { + InstructionListType Code; + Code.emplace_back(MCInstBuilder(X86::CMP64ri8) + .addReg(RegNo) + .addImm(Imm)); + Code.emplace_back(MCInstBuilder(X86::JCC_1) + .addExpr(MCSymbolRefExpr::create( + Target, MCSymbolRefExpr::VK_None, *Ctx)) + .addImm(X86::COND_NE)); + return Code; + } + std::optional createRelocation(const MCFixup &Fixup, const MCAsmBackend &MAB) const override { diff --git a/bolt/unittests/Core/MCPlusBuilder.cpp b/bolt/unittests/Core/MCPlusBuilder.cpp index d367eb07f7767..5cd9ee2a6ff76 100644 --- a/bolt/unittests/Core/MCPlusBuilder.cpp +++ b/bolt/unittests/Core/MCPlusBuilder.cpp @@ -107,6 +107,52 @@ TEST_P(MCPlusBuilderTester, AliasSmallerX0) { testRegAliases(Triple::aarch64, AArch64::X0, AliasesX0, AliasesX0Count, true); } +TEST_P(MCPlusBuilderTester, AArch64_CmpJE) { + if (GetParam() != Triple::aarch64) + GTEST_SKIP(); + BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); + std::unique_ptr BB = BF->createBasicBlock(); + + InstructionListType Instrs = BC->MIB->createCmpJE(AArch64::X0, 2, BB->getLabel(), BC->Ctx.get()); + BB->addInstructions(Instrs.begin(), Instrs.end()); + BB->addSuccessor(BB.get()); + + auto II = BB->begin(); + ASSERT_EQ(II->getOpcode(), AArch64::SUBSXri); + ASSERT_EQ(II->getOperand(0).getReg(), AArch64::XZR); + ASSERT_EQ(II->getOperand(1).getReg(), AArch64::X0); + ASSERT_EQ(II->getOperand(2).getImm(), 2); + ASSERT_EQ(II->getOperand(3).getImm(), 0); + II++; + ASSERT_EQ(II->getOpcode(), AArch64::Bcc); + ASSERT_EQ(II->getOperand(0).getImm(), AArch64CC::EQ); + const MCSymbol *Label = BC->MIB->getTargetSymbol(*II,1); + ASSERT_EQ(Label, BB->getLabel()); +} + +TEST_P(MCPlusBuilderTester, AArch64_CmpJNE) { + if (GetParam() != Triple::aarch64) + GTEST_SKIP(); + BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); + std::unique_ptr BB = BF->createBasicBlock(); + + InstructionListType Instrs = BC->MIB->createCmpJNE(AArch64::X0, 2, BB->getLabel(), BC->Ctx.get()); + BB->addInstructions(Instrs.begin(), Instrs.end()); + BB->addSuccessor(BB.get()); + + auto II = BB->begin(); + ASSERT_EQ(II->getOpcode(), AArch64::SUBSXri); + ASSERT_EQ(II->getOperand(0).getReg(), AArch64::XZR); + ASSERT_EQ(II->getOperand(1).getReg(), AArch64::X0); + ASSERT_EQ(II->getOperand(2).getImm(), 2); + ASSERT_EQ(II->getOperand(3).getImm(), 0); + II++; + ASSERT_EQ(II->getOpcode(), AArch64::Bcc); + ASSERT_EQ(II->getOperand(0).getImm(), AArch64CC::NE); + const MCSymbol *Label = BC->MIB->getTargetSymbol(*II,1); + ASSERT_EQ(Label, BB->getLabel()); +} + #endif // AARCH64_AVAILABLE #ifdef X86_AVAILABLE @@ -143,6 +189,48 @@ TEST_P(MCPlusBuilderTester, ReplaceRegWithImm) { ASSERT_EQ(II->getOperand(1).getImm(), 1); } +TEST_P(MCPlusBuilderTester, X86_CmpJE) { + if (GetParam() != Triple::x86_64) + GTEST_SKIP(); + BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); + std::unique_ptr BB = BF->createBasicBlock(); + + InstructionListType Instrs = BC->MIB->createCmpJE(X86::EAX, 2, BB->getLabel(), BC->Ctx.get()); + BB->addInstructions(Instrs.begin(), Instrs.end()); + BB->addSuccessor(BB.get()); + + auto II = BB->begin(); + ASSERT_EQ(II->getOpcode(), X86::CMP64ri8); + ASSERT_EQ(II->getOperand(0).getReg(), X86::EAX); + ASSERT_EQ(II->getOperand(1).getImm(), 2); + II++; + ASSERT_EQ(II->getOpcode(), X86::JCC_1); + const MCSymbol *Label = BC->MIB->getTargetSymbol(*II,0); + ASSERT_EQ(Label, BB->getLabel()); + ASSERT_EQ(II->getOperand(1).getImm(), X86::COND_E); +} + +TEST_P(MCPlusBuilderTester, X86_CmpJNE) { + if (GetParam() != Triple::x86_64) + GTEST_SKIP(); + BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); + std::unique_ptr BB = BF->createBasicBlock(); + + InstructionListType Instrs = BC->MIB->createCmpJNE(X86::EAX, 2, BB->getLabel(), BC->Ctx.get()); + BB->addInstructions(Instrs.begin(), Instrs.end()); + BB->addSuccessor(BB.get()); + + auto II = BB->begin(); + ASSERT_EQ(II->getOpcode(), X86::CMP64ri8); + ASSERT_EQ(II->getOperand(0).getReg(), X86::EAX); + ASSERT_EQ(II->getOperand(1).getImm(), 2); + II++; + ASSERT_EQ(II->getOpcode(), X86::JCC_1); + const MCSymbol *Label = BC->MIB->getTargetSymbol(*II,0); + ASSERT_EQ(Label, BB->getLabel()); + ASSERT_EQ(II->getOperand(1).getImm(), X86::COND_NE); +} + #endif // X86_AVAILABLE TEST_P(MCPlusBuilderTester, Annotation) { From 92bd2e125c10762aaad360945d82b8b5d8c7a293 Mon Sep 17 00:00:00 2001 From: Rodrigo Rocha Date: Wed, 19 Mar 2025 18:15:17 +0000 Subject: [PATCH 2/3] [BOLT] Format files using LLVM style. --- bolt/include/bolt/Core/MCPlusBuilder.h | 5 ++- .../Target/AArch64/AArch64MCPlusBuilder.cpp | 36 +++++++++---------- bolt/lib/Target/X86/X86MCPlusBuilder.cpp | 8 ++--- bolt/unittests/Core/MCPlusBuilder.cpp | 26 ++++++++------ 4 files changed, 38 insertions(+), 37 deletions(-) diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h index a9d79112b641a..635f6d043a7d4 100644 --- a/bolt/include/bolt/Core/MCPlusBuilder.h +++ b/bolt/include/bolt/Core/MCPlusBuilder.h @@ -1717,13 +1717,12 @@ class MCPlusBuilder { /// Create a sequence of instructions to compare contents of a register /// \p RegNo to immediate \Imm and jump to \p Target if they are different. virtual InstructionListType createCmpJNE(MCPhysReg RegNo, int64_t Imm, - const MCSymbol *Target, - MCContext *Ctx) const { + const MCSymbol *Target, + MCContext *Ctx) const { llvm_unreachable("not implemented"); return {}; } - /// Creates inline memcpy instruction. If \p ReturnEnd is true, then return /// (dest + n) instead of dest. virtual InstructionListType createInlineMemcpy(bool ReturnEnd) const { diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp index bb627ce2586b0..f2ebbebdea299 100644 --- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp +++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp @@ -1321,14 +1321,14 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { int getUncondBranchEncodingSize() const override { return 28; } - //This helper function creates the snippet of code - //that compares a register RegNo with an immedaite Imm, - //and jumps to Target if they are equal. - //cmp RegNo, #Imm - //b.eq Target - //where cmp is an for subs, which results in the code below: - //subs xzr, RegNo, #Imm - //b.eq Target. + // This helper function creates the snippet of code + // that compares a register RegNo with an immedaite Imm, + // and jumps to Target if they are equal. + // cmp RegNo, #Imm + // b.eq Target + // where cmp is an for subs, which results in the code below: + // subs xzr, RegNo, #Imm + // b.eq Target. InstructionListType createCmpJE(MCPhysReg RegNo, int64_t Imm, const MCSymbol *Target, MCContext *Ctx) const override { @@ -1345,17 +1345,17 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { return Code; } - //This helper function creates the snippet of code - //that compares a register RegNo with an immedaite Imm, - //and jumps to Target if they are not equal. - //cmp RegNo, #Imm - //b.ne Target - //where cmp is an for subs, which results in the code below: - //subs xzr, RegNo, #Imm - //b.ne Target. + // This helper function creates the snippet of code + // that compares a register RegNo with an immedaite Imm, + // and jumps to Target if they are not equal. + // cmp RegNo, #Imm + // b.ne Target + // where cmp is an for subs, which results in the code below: + // subs xzr, RegNo, #Imm + // b.ne Target. InstructionListType createCmpJNE(MCPhysReg RegNo, int64_t Imm, - const MCSymbol *Target, - MCContext *Ctx) const override { + const MCSymbol *Target, + MCContext *Ctx) const override { InstructionListType Code; Code.emplace_back(MCInstBuilder(AArch64::SUBSXri) .addReg(AArch64::XZR) diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp index bc827e6fd6f1e..94428f715b51c 100644 --- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp +++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp @@ -2437,12 +2437,10 @@ class X86MCPlusBuilder : public MCPlusBuilder { } InstructionListType createCmpJNE(MCPhysReg RegNo, int64_t Imm, - const MCSymbol *Target, - MCContext *Ctx) const override { + const MCSymbol *Target, + MCContext *Ctx) const override { InstructionListType Code; - Code.emplace_back(MCInstBuilder(X86::CMP64ri8) - .addReg(RegNo) - .addImm(Imm)); + Code.emplace_back(MCInstBuilder(X86::CMP64ri8).addReg(RegNo).addImm(Imm)); Code.emplace_back(MCInstBuilder(X86::JCC_1) .addExpr(MCSymbolRefExpr::create( Target, MCSymbolRefExpr::VK_None, *Ctx)) diff --git a/bolt/unittests/Core/MCPlusBuilder.cpp b/bolt/unittests/Core/MCPlusBuilder.cpp index 5cd9ee2a6ff76..a3113cab3d334 100644 --- a/bolt/unittests/Core/MCPlusBuilder.cpp +++ b/bolt/unittests/Core/MCPlusBuilder.cpp @@ -113,10 +113,11 @@ TEST_P(MCPlusBuilderTester, AArch64_CmpJE) { BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); std::unique_ptr BB = BF->createBasicBlock(); - InstructionListType Instrs = BC->MIB->createCmpJE(AArch64::X0, 2, BB->getLabel(), BC->Ctx.get()); + InstructionListType Instrs = + BC->MIB->createCmpJE(AArch64::X0, 2, BB->getLabel(), BC->Ctx.get()); BB->addInstructions(Instrs.begin(), Instrs.end()); BB->addSuccessor(BB.get()); - + auto II = BB->begin(); ASSERT_EQ(II->getOpcode(), AArch64::SUBSXri); ASSERT_EQ(II->getOperand(0).getReg(), AArch64::XZR); @@ -126,7 +127,7 @@ TEST_P(MCPlusBuilderTester, AArch64_CmpJE) { II++; ASSERT_EQ(II->getOpcode(), AArch64::Bcc); ASSERT_EQ(II->getOperand(0).getImm(), AArch64CC::EQ); - const MCSymbol *Label = BC->MIB->getTargetSymbol(*II,1); + const MCSymbol *Label = BC->MIB->getTargetSymbol(*II, 1); ASSERT_EQ(Label, BB->getLabel()); } @@ -135,11 +136,12 @@ TEST_P(MCPlusBuilderTester, AArch64_CmpJNE) { GTEST_SKIP(); BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); std::unique_ptr BB = BF->createBasicBlock(); - - InstructionListType Instrs = BC->MIB->createCmpJNE(AArch64::X0, 2, BB->getLabel(), BC->Ctx.get()); + + InstructionListType Instrs = + BC->MIB->createCmpJNE(AArch64::X0, 2, BB->getLabel(), BC->Ctx.get()); BB->addInstructions(Instrs.begin(), Instrs.end()); BB->addSuccessor(BB.get()); - + auto II = BB->begin(); ASSERT_EQ(II->getOpcode(), AArch64::SUBSXri); ASSERT_EQ(II->getOperand(0).getReg(), AArch64::XZR); @@ -149,7 +151,7 @@ TEST_P(MCPlusBuilderTester, AArch64_CmpJNE) { II++; ASSERT_EQ(II->getOpcode(), AArch64::Bcc); ASSERT_EQ(II->getOperand(0).getImm(), AArch64CC::NE); - const MCSymbol *Label = BC->MIB->getTargetSymbol(*II,1); + const MCSymbol *Label = BC->MIB->getTargetSymbol(*II, 1); ASSERT_EQ(Label, BB->getLabel()); } @@ -195,7 +197,8 @@ TEST_P(MCPlusBuilderTester, X86_CmpJE) { BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); std::unique_ptr BB = BF->createBasicBlock(); - InstructionListType Instrs = BC->MIB->createCmpJE(X86::EAX, 2, BB->getLabel(), BC->Ctx.get()); + InstructionListType Instrs = + BC->MIB->createCmpJE(X86::EAX, 2, BB->getLabel(), BC->Ctx.get()); BB->addInstructions(Instrs.begin(), Instrs.end()); BB->addSuccessor(BB.get()); @@ -205,7 +208,7 @@ TEST_P(MCPlusBuilderTester, X86_CmpJE) { ASSERT_EQ(II->getOperand(1).getImm(), 2); II++; ASSERT_EQ(II->getOpcode(), X86::JCC_1); - const MCSymbol *Label = BC->MIB->getTargetSymbol(*II,0); + const MCSymbol *Label = BC->MIB->getTargetSymbol(*II, 0); ASSERT_EQ(Label, BB->getLabel()); ASSERT_EQ(II->getOperand(1).getImm(), X86::COND_E); } @@ -216,7 +219,8 @@ TEST_P(MCPlusBuilderTester, X86_CmpJNE) { BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); std::unique_ptr BB = BF->createBasicBlock(); - InstructionListType Instrs = BC->MIB->createCmpJNE(X86::EAX, 2, BB->getLabel(), BC->Ctx.get()); + InstructionListType Instrs = + BC->MIB->createCmpJNE(X86::EAX, 2, BB->getLabel(), BC->Ctx.get()); BB->addInstructions(Instrs.begin(), Instrs.end()); BB->addSuccessor(BB.get()); @@ -226,7 +230,7 @@ TEST_P(MCPlusBuilderTester, X86_CmpJNE) { ASSERT_EQ(II->getOperand(1).getImm(), 2); II++; ASSERT_EQ(II->getOpcode(), X86::JCC_1); - const MCSymbol *Label = BC->MIB->getTargetSymbol(*II,0); + const MCSymbol *Label = BC->MIB->getTargetSymbol(*II, 0); ASSERT_EQ(Label, BB->getLabel()); ASSERT_EQ(II->getOperand(1).getImm(), X86::COND_NE); } From 2e440b03f1828482b58a5361965f2967091e49a3 Mon Sep 17 00:00:00 2001 From: Rodrigo Rocha Date: Mon, 24 Mar 2025 23:55:17 +0000 Subject: [PATCH 3/3] Fixed typo and format of comments according to 80-column width. --- bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp index f2ebbebdea299..60c328de61907 100644 --- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp +++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp @@ -1321,12 +1321,11 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { int getUncondBranchEncodingSize() const override { return 28; } - // This helper function creates the snippet of code - // that compares a register RegNo with an immedaite Imm, - // and jumps to Target if they are equal. + // This helper function creates the snippet of code that compares a register + // RegNo with an immedaite Imm, and jumps to Target if they are equal. // cmp RegNo, #Imm // b.eq Target - // where cmp is an for subs, which results in the code below: + // where cmp is an alias for subs, which results in the code below: // subs xzr, RegNo, #Imm // b.eq Target. InstructionListType createCmpJE(MCPhysReg RegNo, int64_t Imm, @@ -1345,12 +1344,11 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { return Code; } - // This helper function creates the snippet of code - // that compares a register RegNo with an immedaite Imm, - // and jumps to Target if they are not equal. + // This helper function creates the snippet of code that compares a register + // RegNo with an immedaite Imm, and jumps to Target if they are not equal. // cmp RegNo, #Imm // b.ne Target - // where cmp is an for subs, which results in the code below: + // where cmp is an alias for subs, which results in the code below: // subs xzr, RegNo, #Imm // b.ne Target. InstructionListType createCmpJNE(MCPhysReg RegNo, int64_t Imm,