Skip to content

[RISCV][GISel] Support passing arguments through the stack. #69289

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
Oct 19, 2023

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Oct 17, 2023

This is needed when we run out of registers.

@llvmbot
Copy link
Member

llvmbot commented Oct 17, 2023

@llvm/pr-subscribers-llvm-globalisel

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

Author: Craig Topper (topperc)

Changes

This is needed when we run out of registers.

Still need to add tests.


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

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp (+49-9)
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp b/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
index a362a709329d5df..215aa938e5dc484 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
@@ -17,6 +17,7 @@
 #include "RISCVSubtarget.h"
 #include "llvm/CodeGen/Analysis.h"
 #include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
+#include "llvm/CodeGen/MachineFrameInfo.h"
 
 using namespace llvm;
 
@@ -56,19 +57,38 @@ struct RISCVOutgoingValueAssigner : public CallLowering::OutgoingValueAssigner {
 struct RISCVOutgoingValueHandler : public CallLowering::OutgoingValueHandler {
   RISCVOutgoingValueHandler(MachineIRBuilder &B, MachineRegisterInfo &MRI,
                             MachineInstrBuilder MIB)
-      : OutgoingValueHandler(B, MRI), MIB(MIB) {}
-
-  MachineInstrBuilder MIB;
-
+      : OutgoingValueHandler(B, MRI), MIB(MIB),
+        Subtarget(MIRBuilder.getMF().getSubtarget<RISCVSubtarget>()) {}
   Register getStackAddress(uint64_t MemSize, int64_t Offset,
                            MachinePointerInfo &MPO,
                            ISD::ArgFlagsTy Flags) override {
-    llvm_unreachable("not implemented");
+    MachineFunction &MF = MIRBuilder.getMF();
+    LLT p0 = LLT::pointer(0, Subtarget.getXLen());
+    LLT sXLen = LLT::scalar(Subtarget.getXLen());
+
+    if (!SPReg)
+      SPReg = MIRBuilder.buildCopy(p0, Register(RISCV::X2)).getReg(0);
+
+    auto OffsetReg = MIRBuilder.buildConstant(sXLen, Offset);
+
+    auto AddrReg = MIRBuilder.buildPtrAdd(p0, SPReg, OffsetReg);
+
+    MPO = MachinePointerInfo::getStack(MF, Offset);
+    return AddrReg.getReg(0);
   }
 
   void assignValueToAddress(Register ValVReg, Register Addr, LLT MemTy,
                             MachinePointerInfo &MPO, CCValAssign &VA) override {
-    llvm_unreachable("not implemented");
+    MachineFunction &MF = MIRBuilder.getMF();
+    uint64_t LocMemOffset = VA.getLocMemOffset();
+
+    // TODO: Move StackAlignment to subtarget and share with FrameLowering.
+    auto MMO =
+        MF.getMachineMemOperand(MPO, MachineMemOperand::MOStore, MemTy,
+                                commonAlignment(Align(16), LocMemOffset));
+
+    Register ExtReg = extendRegister(ValVReg, VA);
+    MIRBuilder.buildStore(ExtReg, Addr, *MMO);
   }
 
   void assignValueToReg(Register ValVReg, Register PhysReg,
@@ -77,6 +97,14 @@ struct RISCVOutgoingValueHandler : public CallLowering::OutgoingValueHandler {
     MIRBuilder.buildCopy(PhysReg, ExtReg);
     MIB.addUse(PhysReg, RegState::Implicit);
   }
+
+private:
+  MachineInstrBuilder MIB;
+
+  // Cache the SP register vreg if we need it more than once in this call site.
+  Register SPReg;
+
+  const RISCVSubtarget &Subtarget;
 };
 
 struct RISCVIncomingValueAssigner : public CallLowering::IncomingValueAssigner {
@@ -112,17 +140,26 @@ struct RISCVIncomingValueAssigner : public CallLowering::IncomingValueAssigner {
 
 struct RISCVIncomingValueHandler : public CallLowering::IncomingValueHandler {
   RISCVIncomingValueHandler(MachineIRBuilder &B, MachineRegisterInfo &MRI)
-      : IncomingValueHandler(B, MRI) {}
+      : IncomingValueHandler(B, MRI),
+        Subtarget(MIRBuilder.getMF().getSubtarget<RISCVSubtarget>()) {}
 
   Register getStackAddress(uint64_t MemSize, int64_t Offset,
                            MachinePointerInfo &MPO,
                            ISD::ArgFlagsTy Flags) override {
-    llvm_unreachable("not implemented");
+    MachineFrameInfo &MFI = MIRBuilder.getMF().getFrameInfo();
+
+    int FI = MFI.CreateFixedObject(MemSize, Offset, /*Immutable=*/true);
+    MPO = MachinePointerInfo::getFixedStack(MIRBuilder.getMF(), FI);
+    return MIRBuilder.buildFrameIndex(LLT::pointer(0, Subtarget.getXLen()), FI)
+        .getReg(0);
   }
 
   void assignValueToAddress(Register ValVReg, Register Addr, LLT MemTy,
                             MachinePointerInfo &MPO, CCValAssign &VA) override {
-    llvm_unreachable("not implemented");
+    MachineFunction &MF = MIRBuilder.getMF();
+    auto MMO = MF.getMachineMemOperand(MPO, MachineMemOperand::MOLoad, MemTy,
+                                       inferAlignFromPtrInfo(MF, MPO));
+    MIRBuilder.buildLoad(ValVReg, Addr, *MMO);
   }
 
   void assignValueToReg(Register ValVReg, Register PhysReg,
@@ -131,6 +168,9 @@ struct RISCVIncomingValueHandler : public CallLowering::IncomingValueHandler {
     MIRBuilder.getMBB().addLiveIn(PhysReg);
     MIRBuilder.buildCopy(ValVReg, PhysReg);
   }
+
+private:
+  const RISCVSubtarget &Subtarget;
 };
 
 struct RISCVCallReturnHandler : public RISCVIncomingValueHandler {

This is needed when we run out of registers.
Copy link
Member

@mshockwave mshockwave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@michaelmaitland
Copy link
Contributor

I saw this was approved but WIP is in the title. Please change before commit.

@topperc topperc changed the title [RISCV][GISel][WIP] Support passing arguments through the stack. [RISCV][GISel] Support passing arguments through the stack. Oct 18, 2023
@mshockwave
Copy link
Member

I saw this was approved but WIP is in the title. Please change before commit.

Right, please also remember to update the commit message too as the tests have been added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants