Skip to content
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

[RISCV][GISel] Disable call lowering for integers larger than 2*XLen. #69144

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Oct 16, 2023

Types larger than 2*XLen are passed indirectly which is not supported yet. Currently, we will incorrectly pass X10 multiple times.

Types larger than 2*XLen are passed indirectly which is not supported yet.
Currently, we will incorrectly pass X10 multiple times.
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 16, 2023

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

Author: Craig Topper (topperc)

Changes

Types larger than 2*XLen are passed indirectly which is not supported yet. Currently, we will incorrectly pass X10 multiple times.


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

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp (+48-18)
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp b/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
index a362a709329d5df..74eb4cc8565e553 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp
@@ -153,6 +153,42 @@ struct RISCVCallReturnHandler : public RISCVIncomingValueHandler {
 RISCVCallLowering::RISCVCallLowering(const RISCVTargetLowering &TLI)
     : CallLowering(&TLI) {}
 
+// TODO: Support all argument types.
+static bool isSupportedArgumentType(Type *T, const RISCVSubtarget &Subtarget) {
+  // TODO: Integers larger than 2*XLen are passed indirectly which is not
+  // supported yet.
+  if (T->isIntegerTy())
+    return T->getIntegerBitWidth() <= Subtarget.getXLen() * 2;
+  if (T->isPointerTy())
+    return true;
+  return false;
+}
+
+// TODO: Only integer, pointer and aggregate types are supported now.
+static bool isSupportedReturnType(Type *T, const RISCVSubtarget &Subtarget) {
+  // TODO: Integers larger than 2*XLen are passed indirectly which is not
+  // supported yet.
+  if (T->isIntegerTy())
+    return T->getIntegerBitWidth() <= Subtarget.getXLen() * 2;
+  if (T->isPointerTy())
+    return true;
+
+  if (T->isArrayTy())
+    return isSupportedReturnType(T->getArrayElementType(), Subtarget);
+
+  if (T->isStructTy()) {
+    // For now we only allow homogeneous structs that we can manipulate with
+    // G_MERGE_VALUES and G_UNMERGE_VALUES
+    auto StructT = cast<StructType>(T);
+    for (unsigned i = 1, e = StructT->getNumElements(); i != e; ++i)
+      if (StructT->getElementType(i) != StructT->getElementType(0))
+        return false;
+    return isSupportedReturnType(StructT->getElementType(0), Subtarget);
+  }
+
+  return false;
+}
+
 bool RISCVCallLowering::lowerReturnVal(MachineIRBuilder &MIRBuilder,
                                        const Value *Val,
                                        ArrayRef<Register> VRegs,
@@ -160,8 +196,9 @@ bool RISCVCallLowering::lowerReturnVal(MachineIRBuilder &MIRBuilder,
   if (!Val)
     return true;
 
-  // TODO: Only integer, pointer and aggregate types are supported now.
-  if (!Val->getType()->isIntOrPtrTy() && !Val->getType()->isAggregateType())
+  const RISCVSubtarget &Subtarget =
+      MIRBuilder.getMF().getSubtarget<RISCVSubtarget>();
+  if (!isSupportedReturnType(Val->getType(), Subtarget))
     return false;
 
   MachineFunction &MF = MIRBuilder.getMF();
@@ -208,13 +245,11 @@ bool RISCVCallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder,
   if (F.isVarArg())
     return false;
 
-  // TODO: Support all argument types.
+  const RISCVSubtarget &Subtarget =
+      MIRBuilder.getMF().getSubtarget<RISCVSubtarget>();
   for (auto &Arg : F.args()) {
-    if (Arg.getType()->isIntegerTy())
-      continue;
-    if (Arg.getType()->isPointerTy())
-      continue;
-    return false;
+    if (!isSupportedArgumentType(Arg.getType(), Subtarget))
+      return false;
   }
 
   MachineFunction &MF = MIRBuilder.getMF();
@@ -252,15 +287,11 @@ bool RISCVCallLowering::lowerCall(MachineIRBuilder &MIRBuilder,
   const Function &F = MF.getFunction();
   CallingConv::ID CC = F.getCallingConv();
 
-  // TODO: Support all argument types.
+  const RISCVSubtarget &Subtarget =
+      MIRBuilder.getMF().getSubtarget<RISCVSubtarget>();
   for (auto &AInfo : Info.OrigArgs) {
-    if (AInfo.Ty->isIntegerTy())
-      continue;
-    if (AInfo.Ty->isPointerTy())
-      continue;
-    if (AInfo.Ty->isFloatingPointTy())
-      continue;
-    return false;
+    if (!isSupportedArgumentType(AInfo.Ty, Subtarget))
+      return false;
   }
 
   SmallVector<ArgInfo, 32> SplitArgInfos;
@@ -297,8 +328,7 @@ bool RISCVCallLowering::lowerCall(MachineIRBuilder &MIRBuilder,
   if (Info.OrigRet.Ty->isVoidTy())
     return true;
 
-  // TODO: Only integer, pointer and aggregate types are supported now.
-  if (!Info.OrigRet.Ty->isIntOrPtrTy() && !Info.OrigRet.Ty->isAggregateType())
+  if (!isSupportedReturnType(Info.OrigRet.Ty, Subtarget))
     return false;
 
   SmallVector<ArgInfo, 4> SplitRetInfos;

Copy link
Collaborator

@preames preames left a comment

Choose a reason for hiding this comment

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

LGTM

return true;

if (T->isArrayTy())
return isSupportedReturnType(T->getArrayElementType(), Subtarget);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we're going to need a restriction on array and struct size here as well, but that can be a follow up patch.

@topperc topperc merged commit 3a4b0e9 into llvm:main Oct 20, 2023
4 checks passed
@topperc topperc deleted the pr/gisel-2xlen branch October 20, 2023 18:05
topperc added a commit that referenced this pull request Oct 20, 2023
… 2*XLen. (#69144)"

This reverts commit 3a4b0e9.

Seems to be failing on the build bots.
topperc added a commit that referenced this pull request Oct 20, 2023
…an 2*XLen. (#69144)"

Remove bad test for >2x XLen scalar. Don't restrict struct returns if they aren't homogenous.

Original commit message:
Types larger than 2*XLen are passed indirectly which is not supported
yet. Currently, we will incorrectly pass X10 multiple times.
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.

None yet

3 participants