Skip to content

[BOLT][RISCV] Fix MCPlusBuilder instrumentation ifaces #136211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 17, 2025

Conversation

kazutakahirata
Copy link
Contributor

a) Due to the different capabilities of the functions implemented, rename the createCmpJE function
b) Refactor the convertIndirectCallToLoad function to override the interface.

Patch by WangJee, originally posted in #136129

a) Due to the different capabilities of the functions implemented, rename the createCmpJE function
b) Refactor the convertIndirectCallToLoad function to override the interface.

Patch by WangJee, originally posted in llvm#136129
@llvmbot
Copy link
Member

llvmbot commented Apr 17, 2025

@llvm/pr-subscribers-bolt

Author: Kazu Hirata (kazutakahirata)

Changes

a) Due to the different capabilities of the functions implemented, rename the createCmpJE function
b) Refactor the convertIndirectCallToLoad function to override the interface.

Patch by WangJee, originally posted in #136129


Full diff: https://github.com/llvm/llvm-project/pull/136211.diff

1 Files Affected:

  • (modified) bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp (+7-8)
diff --git a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
index 0e27d29019e95..391c1866c810a 100644
--- a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
+++ b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
@@ -555,9 +555,9 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
                .addReg(RegCnt);
   }
 
-  InstructionListType createCmpJE(MCPhysReg RegNo, MCPhysReg RegTmp,
-                                  const MCSymbol *Target,
-                                  MCContext *Ctx) const {
+  InstructionListType createRegCmpJE(MCPhysReg RegNo, MCPhysReg RegTmp,
+                                     const MCSymbol *Target,
+                                     MCContext *Ctx) const {
     InstructionListType Insts;
     Insts.emplace_back(
         MCInstBuilder(RISCV::SUB).addReg(RegTmp).addReg(RegNo).addReg(RegNo));
@@ -718,7 +718,7 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
     Insts.emplace_back();
     loadReg(Insts.back(), RISCV::X10, RISCV::X10, 0);
     InstructionListType cmpJmp =
-        createCmpJE(RISCV::X10, RISCV::X11, IndCallHandler, Ctx);
+        createRegCmpJE(RISCV::X10, RISCV::X11, IndCallHandler, Ctx);
     Insts.insert(Insts.end(), cmpJmp.begin(), cmpJmp.end());
     Insts.emplace_back();
     createStackPointerIncrement(Insts.back(), 16);
@@ -777,14 +777,13 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
     return createGetter(Ctx, "__bolt_instr_num_funcs");
   }
 
-  void convertIndirectCallToLoad(MCInst &Inst, MCPhysReg Reg,
-                                 MCPhysReg ZeroReg) const {
+  void convertIndirectCallToLoad(MCInst &Inst, MCPhysReg Reg) override {
     bool IsTailCall = isTailCall(Inst);
     if (IsTailCall)
       removeAnnotation(Inst, MCPlus::MCAnnotation::kTailCall);
     Inst.setOpcode(RISCV::ADD);
     Inst.insert(Inst.begin(), MCOperand::createReg(Reg));
-    Inst.insert(Inst.begin() + 1, MCOperand::createReg(ZeroReg));
+    Inst.insert(Inst.begin() + 1, MCOperand::createReg(RISCV::X0));
     return;
   }
 
@@ -845,7 +844,7 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
     InstructionListType Insts;
     spillRegs(Insts, {RISCV::X10, RISCV::X11});
     Insts.emplace_back(CallInst);
-    convertIndirectCallToLoad(Insts.back(), RISCV::X10, RISCV::X0);
+    convertIndirectCallToLoad(Insts.back(), RISCV::X10);
     InstructionListType LoadImm = createLoadImmediate(RISCV::X11, CallSiteID);
     Insts.insert(Insts.end(), LoadImm.begin(), LoadImm.end());
     spillRegs(Insts, {RISCV::X10, RISCV::X11});

@kazutakahirata kazutakahirata merged commit 2af5e01 into llvm:main Apr 17, 2025
12 checks passed
@kazutakahirata kazutakahirata deleted the bolt_fix branch April 17, 2025 22:27
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
a) Due to the different capabilities of the functions implemented,
rename the createCmpJE function
b) Refactor the convertIndirectCallToLoad function to override the
interface.

Patch by WangJee, originally posted in llvm#136129
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
a) Due to the different capabilities of the functions implemented,
rename the createCmpJE function
b) Refactor the convertIndirectCallToLoad function to override the
interface.

Patch by WangJee, originally posted in llvm#136129
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants