Skip to content

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
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