[FastIsel] Get the right register type for call instruction #164565

LuoYuanke · 2025-10-22T07:12:06Z

When switch from fast isel to dag isel the input value is from llvm IR instruction.
If the instruction is call we should get the calling convention of the callee and
pass it to RegsForValue::getCopyFromRegs, so that it can deduce the right RegisterVT
of the returned value of the callee.

When switch from fast isel to dag isel the input value is from llvm IR instruction. If the instruction is call we should get the calling convention of the callee and pass it to RegsForValue::getCopyFromRegs, so that it can deduce the right RegisterVT of the returned value of the callee.

llvmbot · 2025-10-22T07:12:51Z

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-llvm-selectiondag

Author: Luo Yuanke (LuoYuanke)

Changes

When switch from fast isel to dag isel the input value is from llvm IR instruction.
If the instruction is call we should get the calling convention of the callee and
pass it to RegsForValue::getCopyFromRegs, so that it can deduce the right RegisterVT
of the returned value of the callee.

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

2 Files Affected:

(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+6-1)
(added) llvm/test/CodeGen/X86/bf16-fast-isel.ll (+19)

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 20a0efd3afa1c..14f9fdfddf658 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -1977,8 +1977,13 @@ SDValue SelectionDAGBuilder::getValueImpl(const Value *V) {
   if (const Instruction *Inst = dyn_cast<Instruction>(V)) {
     Register InReg = FuncInfo.InitializeRegForValue(Inst);
 
+    std::optional<CallingConv::ID> CallConv = std::nullopt;
+    auto *Callee = dyn_cast<CallInst>(Inst);
+    if (Callee && !Callee->isInlineAsm())
+      CallConv = Callee->getCallingConv();
+
     RegsForValue RFV(*DAG.getContext(), TLI, DAG.getDataLayout(), InReg,
-                     Inst->getType(), std::nullopt);
+                     Inst->getType(), CallConv);
     SDValue Chain = DAG.getEntryNode();
     return RFV.getCopyFromRegs(DAG, FuncInfo, getCurSDLoc(), Chain, nullptr, V);
   }
diff --git a/llvm/test/CodeGen/X86/bf16-fast-isel.ll b/llvm/test/CodeGen/X86/bf16-fast-isel.ll
new file mode 100644
index 0000000000000..43622566d2974
--- /dev/null
+++ b/llvm/test/CodeGen/X86/bf16-fast-isel.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown | FileCheck %s
+
+define i8 @test(ptr %f) nounwind {
+; CHECK-LABEL: test:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    callq foo@PLT
+; CHECK-NEXT:    callq bar@PLT
+; CHECK-NEXT:    popq %rcx
+; CHECK-NEXT:    retq
+entry:
+  %call = call bfloat @foo(ptr %f)
+  %call2 = call noundef zeroext i8 @bar(bfloat %call)
+  ret i8 %call2
+}
+
+declare bfloat @foo(ptr %f)
+declare zeroext i8 @bar(bfloat)

arsenm · 2025-10-22T07:16:32Z

llvm/test/CodeGen/X86/bf16-fast-isel.ll

+  %call2 = call noundef zeroext i8 @bar(bfloat %call)
+  ret i8 %call2
+}
+


Can you add the same testcase wit Han indirect call. Can you also test each case with a non-default calling convention

Added test case for indirect call and fast calling convention.

arsenm · 2025-10-22T07:16:40Z

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

  if (const Instruction *Inst = dyn_cast<Instruction>(V)) {
    Register InReg = FuncInfo.InitializeRegForValue(Inst);

+    std::optional<CallingConv::ID> CallConv = std::nullopt;


Suggested change

std::optional<CallingConv::ID> CallConv = std::nullopt;

std::optional<CallingConv::ID> CallConv;

arsenm · 2025-10-22T07:16:58Z

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

+    if (Callee && !Callee->isInlineAsm())
+      CallConv = Callee->getCallingConv();


This is broken for indirect calls. You should take the calling convention directly from the callsite, not the callee

This is the callsite instruction. Let me change the variable name to CI.

phoebewang · 2025-10-22T08:09:59Z

Does this solve #134222?

phoebewang · 2025-10-22T08:10:50Z

llvm/test/CodeGen/X86/bf16-fast-isel.ll

@@ -0,0 +1,66 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown | FileCheck %s


Are you missing -O0 here?

Thank you! Let me add "--fast-isel" option for the test case. The issue happens when switching from fast isel to dag isel where fast isel fails to select call instruction.

LuoYuanke · 2025-10-22T08:13:36Z

Does this solve #134222?

Solve the same issue of https://godbolt.org/z/15K5Pb7GG which generate incorrect instruction "call __truncsfbf2@PLT". The callee return bf16 value through xmm0, so don't need to convert from f32 to bf16.

phoebewang · 2025-10-22T08:20:33Z

Does this solve #134222?

Solve the same issue of https://godbolt.org/z/15K5Pb7GG which generate incorrect instruction "call __truncsfbf2@PLT". The callee return bf16 value through xmm0, so don't need to convert from f32 to bf16.

Great! I spent some time on it, but didn't find a concise solution.

phoebewang

LGTM.

KanRobert

LGTM

phoebewang · 2025-10-22T09:19:48Z

@LuoYuanke Would you help verify the initial problems reported in #134222 and #151692 and close them if they are solved?

LuoYuanke · 2025-10-22T09:31:40Z

@LuoYuanke Would you help verify the initial problems reported in #134222 and #151692 and close them if they are solved?

Sure, I'll verify the two issues.

arsenm · 2025-10-22T09:53:06Z

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

    Register InReg = FuncInfo.InitializeRegForValue(Inst);

+    std::optional<CallingConv::ID> CallConv;
+    auto *CI = dyn_cast<CallInst>(Inst);


should also be CallBase?

Do you mean CallBase can cover invoke, callbr instructions which also have cconv, but CallInst only cover call instruction?

I try to create a test case for callbr, but I got llc crash with below case. It seems x86 don't support half or bfloat type in inline assembly.

define half @test1(float %x) { entry: %ret = callbr half asm "vxorps $1, $1, $0 jmp ${2:l}", "=x,x,!i"(float %x) to label %normal [label %abnormal] normal: ret half %ret abnormal: ret half 1.0 }

error: couldn't allocate output register for constraint 'x'

I also got llc crash when running below test case for invoke.

declare bfloat @pers(...) declare bfloat @foo(); define bfloat @test_lp() personality ptr @pers { entry: %1 = invoke bfloat @foo() to label %then unwind label %hotlp then: ret bfloat %1 hotlp: %2 = landingpad { ptr, bfloat } cleanup br label %lpret lpret: %3 = extractvalue { ptr, bfloat } %2, 1 ret bfloat %3 }

So I'd like to keep it as CallInst for now. When bfloat is better supported in callbr and invoke, we can review CallInst and support more call-alike instruction.

callbr is special but invoke is straightforward. You should also fix whatever the crash is

This absolutely needs to be CallBase, leaving this like this is just waiting for someone else to waste time debugging this in the future

OK, let me create a PR to change it.

I created a PR #164769, but it is difficult to create a test case. We need to have a call instruction to use the output of invoke instruction, besides, the call and invoke instruction should be in the same basic block to trigger the issue. Below code can be a base code for the test case.

declare i32 @pers(...) declare bfloat @foo(); define i32 @test_lp() personality ptr @pers { entry: %1 = invoke bfloat @foo() to label %then unwind label %hotlp then: %call = call i32 @bar2(bfloat %1) ret i32 %call hotlp: %2 = landingpad { ptr, i32 } cleanup br label %lpret lpret: %3 = extractvalue { ptr, i32 } %2, 1 ret i32 %3 } declare i32 @bar2(bfloat)

This is to follow the discussion in #164565 CallBase can cover more call-like instructions which carry caling convention flag. Co-authored-by: Yuanke Luo <ykluo@birentech.com>

This is to follow the discussion in llvm/llvm-project#164565 CallBase can cover more call-like instructions which carry caling convention flag. Co-authored-by: Yuanke Luo <ykluo@birentech.com>

) When switch from fast isel to dag isel the input value is from llvm IR instruction. If the instruction is call we should get the calling convention of the callee and pass it to RegsForValue::getCopyFromRegs, so that it can deduce the right RegisterVT of the returned value of the callee. --------- Co-authored-by: Yuanke Luo <ykluo@birentech.com>

This is to follow the discussion in llvm#164565 CallBase can cover more call-like instructions which carry caling convention flag. Co-authored-by: Yuanke Luo <ykluo@birentech.com>

llvmbot added backend:X86 llvm:SelectionDAG SelectionDAGISel as well labels Oct 22, 2025

LuoYuanke requested review from KanRobert, RKSimon and phoebewang October 22, 2025 07:13

arsenm reviewed Oct 22, 2025

View reviewed changes

Yuanke Luo added 2 commits October 22, 2025 15:40

Address Matt's comments.

8b59141

Add test case for fast calling convention

5620e4f

LuoYuanke requested a review from arsenm October 22, 2025 07:52

phoebewang reviewed Oct 22, 2025

View reviewed changes

Address Phoebe's comments

ad77fbe

LuoYuanke requested a review from phoebewang October 22, 2025 08:17

phoebewang approved these changes Oct 22, 2025

View reviewed changes

KanRobert approved these changes Oct 22, 2025

View reviewed changes

LuoYuanke merged commit 5afe29e into llvm:main Oct 22, 2025
10 checks passed

LuoYuanke deleted the fast-isel-bug branch October 22, 2025 09:11

LuoYuanke mentioned this pull request Oct 22, 2025

LLVM incorrectly emits a __truncsfbf2 call after a bfloat function call in unoptimized code on x86 #151692

Closed

arsenm reviewed Oct 22, 2025

View reviewed changes

This was referenced Oct 22, 2025

Incorrect float conversion via __bf16 at -O0 #134222

Closed

[ISel] Use CallBase instead of CallInst #164769

Merged

johnplatts mentioned this pull request Oct 24, 2025

Re-enable BF16 on x86 in debug builds with Clang 22 or later google/highway#2781

Merged

	std::optional<CallingConv::ID> CallConv = std::nullopt;
	std::optional<CallingConv::ID> CallConv;

		if (Callee && !Callee->isInlineAsm())
		CallConv = Callee->getCallingConv();

		@@ -0,0 +1,66 @@
		; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
		; RUN: llc < %s -mtriple=x86_64-unknown-unknown \| FileCheck %s

Uh oh!

[FastIsel] Get the right register type for call instruction #164565

[FastIsel] Get the right register type for call instruction #164565

Uh oh!

Conversation

LuoYuanke commented Oct 22, 2025

Uh oh!

llvmbot commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phoebewang commented Oct 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuoYuanke Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuoYuanke commented Oct 22, 2025

Uh oh!

phoebewang commented Oct 22, 2025

Uh oh!

phoebewang left a comment

Choose a reason for hiding this comment

Uh oh!

KanRobert left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phoebewang commented Oct 22, 2025

Uh oh!

LuoYuanke commented Oct 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuoYuanke Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

llvmbot commented Oct 22, 2025 •

edited

Loading

LuoYuanke Oct 22, 2025 •

edited

Loading

LuoYuanke Oct 23, 2025 •

edited

Loading