Skip to content

Commit 24b53fb

Browse files
LuoYuankeYuanke Luo
authored and
c-rhodes
committed
[X86] Improve illegal return type handling in FastISel (llvm#186723)
Previously, FastISel would fall back to DAG ISel for any illegal return type. This change adds a more precise check to determine if the ABI requires a type conversion that FastISel cannot handle. For example, bfloat is returned as f16 in XMM0, but FastISel would assign f32 register type and store it in FuncInfo.ValueMap, causing DAG to incorrectly perform type conversion from f32 to bfloat later. However, i1 is promoted to i8 and returned as i8 per the ABI, so FastISel can safely lower it without switching to DAGISel. This change enables FastISel to handle such cases properly. --------- Co-authored-by: Yuanke Luo <ykluo@birentech.com> (cherry picked from commit 140adc9)
1 parent cc87fcf commit 24b53fb

2 files changed

Lines changed: 127 additions & 5 deletions

File tree

‎llvm/lib/Target/X86/X86FastISel.cpp‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3265,11 +3265,27 @@ bool X86FastISel::fastLowerCall(CallLoweringInfo &CLI) {
32653265
bool Is64Bit = Subtarget->is64Bit();
32663266
bool IsWin64 = Subtarget->isCallingConvWin64(CC);
32673267

3268-
// If the return type is illegal, don't bother to promote it, just fall back
3269-
// to DAG ISel.
3270-
MVT RetVT;
3271-
if (!isTypeLegal(CLI.RetTy, RetVT) && !CLI.RetTy->isVoidTy())
3272-
return false;
3268+
// If the return type is illegal, check if the ABI requires a type conversion
3269+
// that FastISel cannot handle. Fall back to DAG ISel in such cases.
3270+
// For example, bfloat is returned as f16 in XMM0, however FastISel would
3271+
// assign f32 register type and store it in FuncInfo.ValueMap. This would
3272+
// cause DAG incorrectly perform type conversion from f32 to bfloat after get
3273+
// the value from FuncInfo.ValueMap.
3274+
// However, i1 is promoted to i8 and return i8 defined by ABI, so FastISel can
3275+
// lower it without switching to DAGISel.
3276+
MVT RetVT = MVT::Other;
3277+
if (!isTypeLegal(CLI.RetTy, RetVT) && !CLI.RetTy->isVoidTy()) {
3278+
if (RetVT == MVT::Other)
3279+
return false; // Unknown type, let DAG ISel handle it.
3280+
3281+
// RetVT is not MVT::Other, it must be simple now. It is something rely on
3282+
// the logic of isTypeLegal().
3283+
MVT ABIVT = TLI.getRegisterTypeForCallingConv(CLI.RetTy->getContext(),
3284+
CLI.CallConv, RetVT);
3285+
MVT RegVT = TLI.getRegisterType(CLI.RetTy->getContext(), RetVT);
3286+
if (ABIVT != RegVT)
3287+
return false;
3288+
}
32733289

32743290
// Call / invoke instructions with NoCfCheck attribute require special
32753291
// handling.

‎llvm/test/CodeGen/X86/i1-fast-isel.ll‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
2+
; RUN: llc --fast-isel < %s -mtriple=x86_64-unknown-unknown | FileCheck %s
3+
4+
define i8 @test_direct_call(ptr %f) nounwind {
5+
; CHECK-LABEL: test_direct_call:
6+
; CHECK: # %bb.0: # %entry
7+
; CHECK-NEXT: pushq %rax
8+
; CHECK-NEXT: callq foo@PLT
9+
; CHECK-NEXT: movzbl %al, %edi
10+
; CHECK-NEXT: callq bar@PLT
11+
; CHECK-NEXT: popq %rcx
12+
; CHECK-NEXT: retq
13+
entry:
14+
%call = call i1 @foo(ptr %f)
15+
%call2 = call zeroext i8 @bar(i1 %call)
16+
ret i8 %call2
17+
}
18+
19+
define i8 @test_fast_direct_call(ptr %f) nounwind {
20+
; CHECK-LABEL: test_fast_direct_call:
21+
; CHECK: # %bb.0: # %entry
22+
; CHECK-NEXT: pushq %rax
23+
; CHECK-NEXT: callq foo_fast@PLT
24+
; CHECK-NEXT: movzbl %al, %edi
25+
; CHECK-NEXT: callq bar@PLT
26+
; CHECK-NEXT: popq %rcx
27+
; CHECK-NEXT: retq
28+
entry:
29+
%call = call fastcc i1 @foo_fast(ptr %f)
30+
%call2 = call zeroext i8 @bar(i1 %call)
31+
ret i8 %call2
32+
}
33+
34+
define i8 @test_indirect_all(ptr %fptr, ptr %f) nounwind {
35+
; CHECK-LABEL: test_indirect_all:
36+
; CHECK: # %bb.0: # %entry
37+
; CHECK-NEXT: pushq %rbx
38+
; CHECK-NEXT: movq %rdi, %rbx
39+
; CHECK-NEXT: movq %rsi, %rdi
40+
; CHECK-NEXT: callq foo@PLT
41+
; CHECK-NEXT: movzbl %al, %edi
42+
; CHECK-NEXT: callq *%rbx
43+
; CHECK-NEXT: popq %rbx
44+
; CHECK-NEXT: retq
45+
entry:
46+
%call = call i1 @foo(ptr %f)
47+
%call2 = call zeroext i8 %fptr(i1 %call)
48+
ret i8 %call2
49+
}
50+
51+
define i8 @test_indirect_all2(ptr %fptr, ptr %f, i1 %cond) nounwind {
52+
; CHECK-LABEL: test_indirect_all2:
53+
; CHECK: # %bb.0: # %entry
54+
; CHECK-NEXT: pushq %rbp
55+
; CHECK-NEXT: pushq %rbx
56+
; CHECK-NEXT: pushq %rax
57+
; CHECK-NEXT: movl %edx, %ebp
58+
; CHECK-NEXT: movq %rdi, %rbx
59+
; CHECK-NEXT: movq %rsi, %rdi
60+
; CHECK-NEXT: callq foo@PLT
61+
; CHECK-NEXT: testb $1, %bpl
62+
; CHECK-NEXT: je .LBB3_2
63+
; CHECK-NEXT: # %bb.1: # %exit
64+
; CHECK-NEXT: movzbl %al, %edi
65+
; CHECK-NEXT: callq *%rbx
66+
; CHECK-NEXT: jmp .LBB3_3
67+
; CHECK-NEXT: .LBB3_2: # %exit2
68+
; CHECK-NEXT: movb $3, %al
69+
; CHECK-NEXT: .LBB3_3: # %exit2
70+
; CHECK-NEXT: addq $8, %rsp
71+
; CHECK-NEXT: popq %rbx
72+
; CHECK-NEXT: popq %rbp
73+
; CHECK-NEXT: retq
74+
entry:
75+
%call = call i1 @foo(ptr %f)
76+
br i1 %cond, label %exit, label %exit2
77+
78+
exit:
79+
%call2 = call zeroext i8 %fptr(i1 %call)
80+
ret i8 %call2
81+
82+
exit2:
83+
ret i8 3
84+
}
85+
86+
87+
define i8 @test_fast_indirect_all(ptr %fptr, ptr %f) nounwind {
88+
; CHECK-LABEL: test_fast_indirect_all:
89+
; CHECK: # %bb.0: # %entry
90+
; CHECK-NEXT: pushq %rbx
91+
; CHECK-NEXT: movq %rdi, %rbx
92+
; CHECK-NEXT: movq %rsi, %rdi
93+
; CHECK-NEXT: callq foo@PLT
94+
; CHECK-NEXT: movzbl %al, %edi
95+
; CHECK-NEXT: callq *%rbx
96+
; CHECK-NEXT: popq %rbx
97+
; CHECK-NEXT: retq
98+
entry:
99+
%call = call fastcc i1 @foo(ptr %f)
100+
%call2 = call zeroext i8 %fptr(i1 %call)
101+
ret i8 %call2
102+
}
103+
104+
declare i1 @foo(ptr %f)
105+
declare zeroext i8 @bar(i1)
106+
declare fastcc i1 @foo_fast(ptr %f)

0 commit comments

Comments
 (0)