Skip to content

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

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.

Sign up for GitHub

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 4 commits into from
Oct 22, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6865dee
[FastIsel] Get the right register type for call instruction
Oct 22, 2025
8b59141
Address Matt's comments.
Oct 22, 2025
5620e4f
Add test case for fast calling convention
Oct 22, 2025
ad77fbe
Address Phoebe's comments
Oct 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
auto *CI = dyn_cast<CallInst>(Inst);
Copy link
Contributor

Choose a reason for hiding this comment

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

should also be CallBase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, let me create a PR to change it.

Copy link
Contributor Author

@LuoYuanke LuoYuanke Oct 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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)

if (CI && !CI->isInlineAsm())
CallConv = CI->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);
}
Expand Down
66 changes: 66 additions & 0 deletions llvm/test/CodeGen/X86/bf16-fast-isel.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
; RUN: llc --fast-isel < %s -mtriple=x86_64-unknown-unknown | FileCheck %s

define i8 @test_direct_call(ptr %f) nounwind {
; CHECK-LABEL: test_direct_call:
; 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 zeroext i8 @bar(bfloat %call)
ret i8 %call2
}

define i8 @test_fast_direct_call(ptr %f) nounwind {
; CHECK-LABEL: test_fast_direct_call:
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: pushq %rax
; CHECK-NEXT: callq foo_fast@PLT
; CHECK-NEXT: callq bar@PLT
; CHECK-NEXT: popq %rcx
; CHECK-NEXT: retq
entry:
%call = call fastcc bfloat @foo_fast(ptr %f)
%call2 = call zeroext i8 @bar(bfloat %call)
ret i8 %call2
}

define i8 @test_indirect_all(ptr %fptr, ptr %f) nounwind {
; CHECK-LABEL: test_indirect_all:
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: pushq %rbx
; CHECK-NEXT: movq %rdi, %rbx
; CHECK-NEXT: movq %rsi, %rdi
; CHECK-NEXT: callq foo@PLT
; CHECK-NEXT: callq *%rbx
; CHECK-NEXT: popq %rbx
; CHECK-NEXT: retq
entry:
%call = call bfloat @foo(ptr %f)
%call2 = call zeroext i8 %fptr(bfloat %call)
ret i8 %call2
}

define i8 @test_fast_indirect_all(ptr %fptr, ptr %f) nounwind {
; CHECK-LABEL: test_fast_indirect_all:
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: pushq %rbx
; CHECK-NEXT: movq %rdi, %rbx
; CHECK-NEXT: movq %rsi, %rdi
; CHECK-NEXT: callq foo@PLT
; CHECK-NEXT: callq *%rbx
; CHECK-NEXT: popq %rbx
; CHECK-NEXT: retq
entry:
%call = call fastcc bfloat @foo(ptr %f)
%call2 = call zeroext i8 %fptr(bfloat %call)
ret i8 %call2
}

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added test case for indirect call and fast calling convention.

declare bfloat @foo(ptr %f)
declare zeroext i8 @bar(bfloat)
declare fastcc bfloat @foo_fast(ptr %f)