Skip to content

Commit 644fd3b

Browse files
authored
[FastISel] Don't select a CallInst as a BasicBlock in the SelectionDAG fallback if it has bundled ops (#162895)
This was discovered while looking at the codegen for x64 when Control Flow Guard is enabled. When using `SelectionDAG`, LLVM would generate the following sequence for a CF guarded indirect call: ``` leaq target_func(%rip), %rax rex64 jmpq *__guard_dispatch_icall_fptr(%rip) # TAILCALL ``` However, when Fast ISel was used the following is generated: ``` leaq target_func(%rip), %rax movq __guard_dispatch_icall_fptr(%rip), %rcx rex64 jmpq *%rcx # TAILCALL ``` This was happening despite Fast ISel aborting and falling back to `SelectionDAG`. The root cause for this code gen is that `SelectionDAGISel` has a special case when Fast ISel aborts when lowering a `CallInst` where it tries to lower the instruction as its own basic block, which for such a CF Guard call means that it is lowering an indirect call to `__guard_dispatch_icall_fptr` without observing that the function was being loaded into a pointer in the preceding (and bundled) instruction. The fix for this is to not use the special case when a `CallInst` has bundled instructions: it's better to allow the call and its bundled instructions to be lowered together by `SelectionDAG` instead.
1 parent 83eea87 commit 644fd3b

File tree

2 files changed

+14
-3
lines changed

2 files changed

+14
-3
lines changed

llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1837,6 +1837,14 @@ void SelectionDAGISel::SelectAllBasicBlocks(const Function &Fn) {
18371837

18381838
reportFastISelFailure(*MF, *ORE, R, EnableFastISelAbort > 2);
18391839

1840+
// If the call has operand bundles, then it's best if they are handled
1841+
// together with the call instead of selecting the call as its own
1842+
// block.
1843+
if (cast<CallInst>(Inst)->hasOperandBundles()) {
1844+
NumFastIselFailures += NumFastIselRemaining;
1845+
break;
1846+
}
1847+
18401848
if (!Inst->getType()->isVoidTy() && !Inst->getType()->isTokenTy() &&
18411849
!Inst->use_empty()) {
18421850
Register &R = FuncInfo->ValueMap[Inst];

llvm/test/CodeGen/X86/cfguard-checks.ll

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
; RUN: llc < %s -mtriple=i686-pc-windows-msvc | FileCheck %s -check-prefix=X86
2-
; RUN: llc < %s -mtriple=x86_64-pc-windows-msvc | FileCheck %s -check-prefixes=X64,X64_MSVC
2+
; RUN: llc < %s -mtriple=x86_64-pc-windows-msvc | FileCheck %s -check-prefixes=X64,X64_MSVC,X64_SELDAG
3+
; RUN: llc < %s --fast-isel -mtriple=x86_64-pc-windows-msvc | FileCheck %s -check-prefixes=X64,X64_MSVC,X64_FISEL
34
; RUN: llc < %s -mtriple=i686-w64-windows-gnu | FileCheck %s -check-prefixes=X86,X86_MINGW
4-
; RUN: llc < %s -mtriple=x86_64-w64-windows-gnu | FileCheck %s -check-prefixes=X64,X64_MINGW
5+
; RUN: llc < %s -mtriple=x86_64-w64-windows-gnu | FileCheck %s -check-prefixes=X64,X64_MINGW,X64_SELDAG
6+
; RUN: llc < %s --fast-isel -mtriple=x86_64-w64-windows-gnu | FileCheck %s -check-prefixes=X64,X64_MINGW,X64_FISEL
57
; Control Flow Guard is currently only available on Windows
68

79
; Test that Control Flow Guard checks are correctly added when required.
@@ -27,7 +29,8 @@ entry:
2729
; X64-LABEL: func_guard_nocf
2830
; X64: leaq target_func(%rip), %rax
2931
; X64-NOT: __guard_dispatch_icall_fptr
30-
; X64: callq *%rax
32+
; X64_SELDAG: callq *%rax
33+
; X64_FISEL: callq *32(%rsp)
3134
}
3235
attributes #0 = { "guard_nocf" }
3336

0 commit comments

Comments
 (0)