Skip to content

[X86][FastISel] Restore support for struct returns#194586

Merged
nikic merged 2 commits intollvm:mainfrom
nikic:fastisel-struct
Apr 29, 2026
Merged

[X86][FastISel] Restore support for struct returns#194586
nikic merged 2 commits intollvm:mainfrom
nikic:fastisel-struct

Conversation

@nikic
Copy link
Copy Markdown
Contributor

@nikic nikic commented Apr 28, 2026

After #180322, X86 FastISel forces SDAG fallback for any call with a struct return. This caused major compile-time regressions for debug builds in Rust, where struct returns are very common.

The type legality check should work on the de-aggregated types, not on the return type directly.

After llvm#180322, X86 FastISel forces SDAG fallback for any call with
a struct return. This caused major compile-time regressions for
debug builds in Rust, where struct returns are very common.

The type legality check should work on the de-aggregated types,
not on the return type directly.
@llvmbot
Copy link
Copy Markdown
Member

llvmbot commented Apr 28, 2026

@llvm/pr-subscribers-backend-x86

Author: Nikita Popov (nikic)

Changes

After #180322, X86 FastISel forces SDAG fallback for any call with a struct return. This caused major compile-time regressions for debug builds in Rust, where struct returns are very common.

The type legality check should work on the de-aggregated types, not on the return type directly.


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

2 Files Affected:

  • (modified) llvm/lib/Target/X86/X86FastISel.cpp (+17-12)
  • (added) llvm/test/CodeGen/X86/fast-isel-struct-ret.ll (+58)
diff --git a/llvm/lib/Target/X86/X86FastISel.cpp b/llvm/lib/Target/X86/X86FastISel.cpp
index d36a9581a3638..f91ef4abbdf27 100644
--- a/llvm/lib/Target/X86/X86FastISel.cpp
+++ b/llvm/lib/Target/X86/X86FastISel.cpp
@@ -21,6 +21,7 @@
 #include "X86Subtarget.h"
 #include "X86TargetMachine.h"
 #include "llvm/Analysis/BranchProbabilityInfo.h"
+#include "llvm/CodeGen/Analysis.h"
 #include "llvm/CodeGen/FastISel.h"
 #include "llvm/CodeGen/FunctionLoweringInfo.h"
 #include "llvm/CodeGen/MachineConstantPool.h"
@@ -3208,18 +3209,22 @@ bool X86FastISel::fastLowerCall(CallLoweringInfo &CLI) {
   // the value from FuncInfo.ValueMap.
   // However, i1 is promoted to i8 and return i8 defined by ABI, so FastISel can
   // lower it without switching to DAGISel.
-  MVT RetVT = MVT::Other;
-  if (!isTypeLegal(CLI.RetTy, RetVT) && !CLI.RetTy->isVoidTy()) {
-    if (RetVT == MVT::Other)
-      return false; // Unknown type, let DAG ISel handle it.
-
-    // RetVT is not MVT::Other, it must be simple now. It is something rely on
-    // the logic of isTypeLegal().
-    MVT ABIVT = TLI.getRegisterTypeForCallingConv(CLI.RetTy->getContext(),
-                                                  CLI.CallConv, RetVT);
-    MVT RegVT = TLI.getRegisterType(CLI.RetTy->getContext(), RetVT);
-    if (ABIVT != RegVT)
-      return false;
+  SmallVector<Type *> RetTys;
+  ComputeValueTypes(DL, CLI.RetTy, RetTys);
+  for (Type *RetTy : RetTys) {
+    MVT RetVT = MVT::Other;
+    if (!isTypeLegal(RetTy, RetVT)) {
+      if (RetVT == MVT::Other)
+        return false; // Unknown type, let DAG ISel handle it.
+
+      // RetVT is not MVT::Other, it must be simple now. It is something rely on
+      // the logic of isTypeLegal().
+      MVT ABIVT = TLI.getRegisterTypeForCallingConv(CLI.RetTy->getContext(),
+                                                    CLI.CallConv, RetVT);
+      MVT RegVT = TLI.getRegisterType(CLI.RetTy->getContext(), RetVT);
+      if (ABIVT != RegVT)
+        return false;
+    }
   }
 
   // Call / invoke instructions with NoCfCheck attribute require special
diff --git a/llvm/test/CodeGen/X86/fast-isel-struct-ret.ll b/llvm/test/CodeGen/X86/fast-isel-struct-ret.ll
new file mode 100644
index 0000000000000..34798ef5abe1f
--- /dev/null
+++ b/llvm/test/CodeGen/X86/fast-isel-struct-ret.ll
@@ -0,0 +1,58 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -fast-isel -fast-isel-abort=3 < %s | FileCheck %s
+
+declare { i32, i32 } @get_i32s()
+
+define i32 @call_get_i32s() nounwind {
+; CHECK-LABEL: call_get_i32s:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    callq get_i32s@PLT
+; CHECK-NEXT:    addl %edx, %eax
+; CHECK-NEXT:    popq %rcx
+; CHECK-NEXT:    retq
+  %res = call { i32, i32 } @get_i32s()
+  %res.0 = extractvalue { i32, i32 } %res, 0
+  %res.1 = extractvalue { i32, i32 } %res, 1
+  %add = add i32 %res.0, %res.1
+  ret i32 %add
+}
+
+declare { ptr, ptr } @get_ptrs()
+
+define i64 @call_get_ptrs() nounwind {
+; CHECK-LABEL: call_get_ptrs:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    callq get_ptrs@PLT
+; CHECK-NEXT:    subq %rdx, %rax
+; CHECK-NEXT:    popq %rcx
+; CHECK-NEXT:    retq
+  %res = call { ptr, ptr } @get_ptrs()
+  %res.0 = extractvalue { ptr, ptr } %res, 0
+  %res.1 = extractvalue { ptr, ptr } %res, 1
+  %res.0.addr = ptrtoaddr ptr %res.0 to i64
+  %res.1.addr = ptrtoaddr ptr %res.1 to i64
+  %sub = sub i64 %res.0.addr, %res.1.addr
+  ret i64 %sub
+}
+
+declare { i64, i1 } @get_i64_and_bool()
+
+define i64 @call_get_i64_and_bool() nounwind {
+; CHECK-LABEL: call_get_i64_and_bool:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    callq get_i64_and_bool@PLT
+; CHECK-NEXT:    andb $1, %dl
+; CHECK-NEXT:    movzbl %dl, %ecx
+; CHECK-NEXT:    addq %rcx, %rax
+; CHECK-NEXT:    popq %rcx
+; CHECK-NEXT:    retq
+  %res = call { i64, i1 } @get_i64_and_bool()
+  %res.0 = extractvalue { i64, i1 } %res, 0
+  %res.1 = extractvalue { i64, i1 } %res, 1
+  %res.1.ext = zext i1 %res.1 to i64
+  %add = add i64 %res.0, %res.1.ext
+  ret i64 %add
+}

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 28, 2026

🐧 Linux x64 Test Results

  • 194535 tests passed
  • 5163 tests skipped

✅ The build succeeded and all tests passed.

Copy link
Copy Markdown
Contributor

@phoebewang phoebewang left a comment

Choose a reason for hiding this comment

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

LGTM.

ret i64 %sub
}

declare { i64, i1 } @get_i64_and_bool()
Copy link
Copy Markdown
Contributor

@LuoYuanke LuoYuanke Apr 29, 2026

Choose a reason for hiding this comment

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

Could you add a test case that return { bf16, i32 }?

Copy link
Copy Markdown
Contributor

@LuoYuanke LuoYuanke left a comment

Choose a reason for hiding this comment

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

Thank you for fixing the compiling-time issue.
LGTM except a nit comment.

@nikic nikic merged commit 30fa415 into llvm:main Apr 29, 2026
10 checks passed
@nikic nikic deleted the fastisel-struct branch April 29, 2026 09:47
@nikic nikic added this to the LLVM 22.x Release milestone Apr 30, 2026
@github-project-automation github-project-automation Bot moved this from Needs Triage to Done in LLVM Release Status Apr 30, 2026
@github-project-automation github-project-automation Bot moved this to Needs Triage in LLVM Release Status Apr 30, 2026
@nikic
Copy link
Copy Markdown
Contributor Author

nikic commented Apr 30, 2026

/cherry-pick 30fa415

@llvmbot
Copy link
Copy Markdown
Member

llvmbot commented Apr 30, 2026

/pull-request #195029

kirthana14m pushed a commit to ROCm/llvm-project that referenced this pull request Apr 30, 2026
After llvm#180322, X86 FastISel forces SDAG fallback for any call with a
struct return. This caused major compile-time regressions for debug
builds in Rust, where struct returns are very common.

The type legality check should work on the de-aggregated types, not on
the return type directly.
dyung pushed a commit to llvmbot/llvm-project that referenced this pull request May 1, 2026
After llvm#180322, X86 FastISel forces SDAG fallback for any call with a
struct return. This caused major compile-time regressions for debug
builds in Rust, where struct returns are very common.

The type legality check should work on the de-aggregated types, not on
the return type directly.

(cherry picked from commit 30fa415)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

4 participants