Skip to content

[RISCV][GlobalISel] Lower calls to variadic functions #68271

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 3 commits into from
Oct 10, 2023

Conversation

nitinjohnraj
Copy link
Contributor

Calls to variadic functions do not seem to require any special handling.

@llvmbot
Copy link
Member

llvmbot commented Oct 4, 2023

@llvm/pr-subscribers-llvm-globalisel

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

Changes

Calls to variadic functions do not seem to require any special handling.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp (-4)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/call-lowering/variadic-call.ll (+33)
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp b/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
index 5505f89a32f21be..a362a709329d5df 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
@@ -252,10 +252,6 @@ bool RISCVCallLowering::lowerCall(MachineIRBuilder &MIRBuilder,
   const Function &F = MF.getFunction();
   CallingConv::ID CC = F.getCallingConv();
 
-  // TODO: Support vararg functions.
-  if (Info.IsVarArg)
-    return false;
-
   // TODO: Support all argument types.
   for (auto &AInfo : Info.OrigArgs) {
     if (AInfo.Ty->isIntegerTy())
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/call-lowering/variadic-call.ll b/llvm/test/CodeGen/RISCV/GlobalISel/call-lowering/variadic-call.ll
new file mode 100644
index 000000000000000..232e7cf39ac634e
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/call-lowering/variadic-call.ll
@@ -0,0 +1,33 @@
+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+; RUN: llc -mtriple=riscv64 -global-isel --stop-before=legalizer < %s -o - \
+; RUN:   | FileCheck %s
+
+declare noundef signext i32 @foo(i32 noundef signext, ...)
+
+define dso_local noundef signext i32 @main() {
+  ; CHECK-LABEL: name: main
+  ; CHECK: bb.1.entry:
+  ; CHECK-NEXT:   [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
+  ; CHECK-NEXT:   [[FRAME_INDEX:%[0-9]+]]:_(p0) = G_FRAME_INDEX %stack.0.retval
+  ; CHECK-NEXT:   G_STORE [[C]](s32), [[FRAME_INDEX]](p0) :: (store (s32) into %ir.retval)
+  ; CHECK-NEXT:   [[C1:%[0-9]+]]:_(s64) = G_CONSTANT i64 0
+  ; CHECK-NEXT:   [[C2:%[0-9]+]]:_(s64) = G_CONSTANT i64 1
+  ; CHECK-NEXT:   [[C3:%[0-9]+]]:_(s64) = G_CONSTANT i64 2
+  ; CHECK-NEXT:   [[C4:%[0-9]+]]:_(s64) = G_CONSTANT i64 3
+  ; CHECK-NEXT:   $x10 = COPY [[C1]](s64)
+  ; CHECK-NEXT:   $x11 = COPY [[C2]](s64)
+  ; CHECK-NEXT:   $x12 = COPY [[C3]](s64)
+  ; CHECK-NEXT:   $x13 = COPY [[C4]](s64)
+  ; CHECK-NEXT:   PseudoCALL target-flags(riscv-call) @_Z1fiz, implicit-def $x1, implicit $x10, implicit $x11, implicit $x12, implicit $x13, implicit-def $x10
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:_(s64) = COPY $x10
+  ; CHECK-NEXT:   [[ASSERT_SEXT:%[0-9]+]]:_(s64) = G_ASSERT_SEXT [[COPY]], 32
+  ; CHECK-NEXT:   [[TRUNC:%[0-9]+]]:_(s32) = G_TRUNC [[ASSERT_SEXT]](s64)
+  ; CHECK-NEXT:   [[SEXT:%[0-9]+]]:_(s64) = G_SEXT [[TRUNC]](s32)
+  ; CHECK-NEXT:   $x10 = COPY [[SEXT]](s64)
+  ; CHECK-NEXT:   PseudoRET implicit $x10
+entry:
+  %retval = alloca i32, align 4
+  store i32 0, ptr %retval, align 4
+  %call = call noundef signext i32 (i32, ...) @(i32 noundef signext 0, i32 noundef signext 1, i32 noundef signext 2, i32 noundef signext 3)
+  ret i32 %call
+}

@michaelmaitland
Copy link
Contributor

In ISelLowering it looks like we do give some care to varargs.

  • If all registers are allocated, then all varargs must be passed on the stack and don't need to save any argregs
  • record the frame index of the first variable argument which is a value necessary to VASTART.
  • if saving an odd number of registers then create an extra stack slot to ensure that the frame pointer is correctly aligned
  • copy the integer register that may have been used for passing varargs to the vararg save area

Why don't we need this in GISel?

@michaelmaitland michaelmaitland self-requested a review October 4, 2023 23:13
@topperc
Copy link
Collaborator

topperc commented Oct 4, 2023

In ISelLowering it looks like we do give some care to varargs.

  • If all registers are allocated, then all varargs must be passed on the stack and don't need to save any argregs
  • record the frame index of the first variable argument which is a value necessary to VASTART.
  • if saving an odd number of registers then create an extra stack slot to ensure that the frame pointer is correctly aligned
  • copy the integer register that may have been used for passing varargs to the vararg save area

Why don't we need this in GISel?

I believe the code you mention is in RISCVTargetLowering::LowerFormalArguments in SelectionDAG. That is used to codegen the beginning of a variadic function. That will need to be done in RISCVCallLowering::lowerFormalArguments for gisel.

This patch is for calls to variadic functions. The equivalent is RISCVTargetLowering::LowerCall in SelectionDAG.

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

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

LGTM

@nitinjohnraj nitinjohnraj merged commit 9bb7122 into llvm:main Oct 10, 2023
@nitinjohnraj nitinjohnraj deleted the lower-variadic-call branch October 10, 2023 18:19
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