[RISCV][GISel] Minor refactoring of RISCVCallReturnHandler and RISCVIncomingValueHandler to match other targets

[topperc]

Forward assignValueToReg to the base class to make the copy. Add markPhysRegUsed to contain the differences between call handling and argument handling. Introduce RISCVFormalArgHandler.

This structure matches how AArch64, AMDGPU, and X86 are structured.

I've also added MIRBuilder.getMRI()->addLiveIn(PhysReg); to match the other targets.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: Craig Topper (topperc)

Changes

Forward assignValueToReg to the base class to make the copy. Add markPhysRegUsed to contain the differences between call handling and argument handling. Introduce RISCVFormalArgHandler.

This structure matches how AArch64, AMDGPU, and X86 are structured.

---

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

1 Files Affected:

• (modified) llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp (+21-10)

diff --git a/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp b/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
index 01b8e77252840f5..b605f2f621d040d 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
@@ -164,28 +164,39 @@ struct RISCVIncomingValueHandler : public CallLowering::IncomingValueHandler {
 
   void assignValueToReg(Register ValVReg, Register PhysReg,
                         CCValAssign VA) override {
-    // Copy argument received in physical register to desired VReg.
-    MIRBuilder.getMBB().addLiveIn(PhysReg);
-    MIRBuilder.buildCopy(ValVReg, PhysReg);
+    markPhysRegUsed(PhysReg);
+    IncomingValueHandler::assignValueToReg(ValVReg, PhysReg, VA);
   }
 
+  /// How the physical register gets marked varies between formal
+  /// parameters (it's a basic-block live-in), and a call instruction
+  /// (it's an implicit-def of the BL).
+  virtual void markPhysRegUsed(MCRegister PhysReg) = 0;
+
 private:
   const RISCVSubtarget &Subtarget;
 };
 
+struct RISCVFormalArgHandler : public RISCVIncomingValueHandler {
+  RISCVFormalArgHandler(MachineIRBuilder &B, MachineRegisterInfo &MRI)
+      : RISCVIncomingValueHandler(B, MRI) {}
+
+  void markPhysRegUsed(MCRegister PhysReg) override {
+    MIRBuilder.getMRI()->addLiveIn(PhysReg);
+    MIRBuilder.getMBB().addLiveIn(PhysReg);
+  }
+};
+
 struct RISCVCallReturnHandler : public RISCVIncomingValueHandler {
   RISCVCallReturnHandler(MachineIRBuilder &B, MachineRegisterInfo &MRI,
                          MachineInstrBuilder &MIB)
       : RISCVIncomingValueHandler(B, MRI), MIB(MIB) {}
 
-  MachineInstrBuilder MIB;
-
-  void assignValueToReg(Register ValVReg, Register PhysReg,
-                        CCValAssign VA) override {
-    // Copy argument received in physical register to desired VReg.
+  void markPhysRegUsed(MCRegister PhysReg) override {
     MIB.addDef(PhysReg, RegState::Implicit);
-    MIRBuilder.buildCopy(ValVReg, PhysReg);
   }
+
+  MachineInstrBuilder MIB;
 };
 
 } // namespace
@@ -312,7 +323,7 @@ bool RISCVCallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder,
   RISCVIncomingValueAssigner Assigner(
       CC == CallingConv::Fast ? RISCV::CC_RISCV_FastCC : RISCV::CC_RISCV,
       /*IsRet=*/false);
-  RISCVIncomingValueHandler Handler(MIRBuilder, MF.getRegInfo());
+  RISCVFormalArgHandler Handler(MIRBuilder, MF.getRegInfo());
 
   return determineAndHandleAssignments(Handler, Assigner, SplitArgInfos,
                                        MIRBuilder, CC, F.isVarArg());

[preames]

LGTM

[preames]

The switch to RISCVFormatArgHandler isn't quite nfc due to the addition of the getMRI()->getLiveIn line.

Not objecting, just making sure you're aware of it and it wasn't accidental.

[topperc]

Thanks. I'll put that in the commit. I copied it from one of the other targets.