Skip to content

Commit

Permalink
[ISEL][BitTestBlock] omit additional bit test when default destinatio…
Browse files Browse the repository at this point in the history 
…n is unreachable

Otherwise we end up with an extra conditional jump, following by an
unconditional jump off the end of a function. ie.

  bb.0:
    BT32rr ..
    JCC_1 %bb.4 ...
  bb.1:
    BT32rr ..
    JCC_1 %bb.2 ...
    JMP_1 %bb.3
  bb.2:
    ...
  bb.3.unreachable:
  bb.4:
    ...

  Should be equivalent to:
  bb.0:
    BT32rr ..
    JCC_1 %bb.4 ...
    JMP_1 %bb.2
  bb.1:
  bb.2:
    ...
  bb.3.unreachable:
  bb.4:
    ...

This can occur since at the higher level IR (Instruction) SwitchInsts
are required to have BBs for default destinations, even when it can be
deduced that such BBs are unreachable.

For most programs, this isn't an issue, just wasted instructions since the
unreachable has been statically proven.

The x86_64 Linux kernel when built with CONFIG_LTO_CLANG_THIN=y fails to
boot though once D106056 is re-applied.  D106056 makes it more likely
that correlation-propagation (CVP) can deduce that the default case of
SwitchInsts are unreachable. The x86_64 kernel uses a binary post
processor called objtool, which emits this warning:

vmlinux.o: warning: objtool: cfg80211_edmg_chandef_valid()+0x169: can't
find jump dest instruction at .text.cfg80211_edmg_chandef_valid+0x17b

I haven't debugged precisely why this causes a failure at boot time, but
fixing this very obvious jump off the end of the function fixes the
warning and boot problem.

Link: https://bugs.llvm.org/show_bug.cgi?id=50080
Fixes: ClangBuiltLinux/linux#679
Fixes: ClangBuiltLinux/linux#1440

Reviewed By: hans

Differential Revision: https://reviews.llvm.org/D109103
  • Loading branch information
@nickdesaulniers
nickdesaulniers committed Sep 8, 2021
1 parent 3f87513 commit 4331f19
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 58 deletions.
4 changes: 2 additions & 2 deletions llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
View file
Expand Up @@ -3019,7 +3019,7 @@ void IRTranslator::finalizeBasicBlock() {
// test, and delete the last bit test.

MachineBasicBlock *NextMBB;
if (BTB.ContiguousRange && j + 2 == ej) {
if ((BTB.ContiguousRange || BTB.OmitRangeCheck) && j + 2 == ej) {
// Second-to-last bit-test with contiguous range: fall through to the
// target of the final bit test.
NextMBB = BTB.Cases[j + 1].TargetBB;
Expand All @@ -3033,7 +3033,7 @@ void IRTranslator::finalizeBasicBlock() {

emitBitTestCase(BTB, NextMBB, UnhandledProb, BTB.Reg, BTB.Cases[j], MBB);

if (BTB.ContiguousRange && j + 2 == ej) {
if ((BTB.ContiguousRange || BTB.OmitRangeCheck) && j + 2 == ej) {
// We need to record the replacement phi edge here that normally
// happens in emitBitTestCase before we delete the case, otherwise the
// phi edge will be lost.
Expand Down
5 changes: 2 additions & 3 deletions llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
View file
Expand Up @@ -10869,10 +10869,9 @@ void SelectionDAGBuilder::lowerWorkItem(SwitchWorkListItem W, Value *Cond,
BTB->DefaultProb -= DefaultProb / 2;
}

if (FallthroughUnreachable) {
// Skip the range check if the fallthrough block is unreachable.
// Skip the range check if the fallthrough block is unreachable.
if (FallthroughUnreachable)
BTB->OmitRangeCheck = true;
}

// If we're in the right place, emit the bit test header right now.
if (CurMBB == SwitchMBB) {
Expand Down
8 changes: 4 additions & 4 deletions llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
View file
Expand Up @@ -1864,9 +1864,9 @@ SelectionDAGISel::FinishBasicBlock() {
// test, and delete the last bit test.

MachineBasicBlock *NextMBB;
if (BTB.ContiguousRange && j + 2 == ej) {
// Second-to-last bit-test with contiguous range: fall through to the
// target of the final bit test.
if ((BTB.ContiguousRange || BTB.OmitRangeCheck) && j + 2 == ej) {
// Second-to-last bit-test with contiguous range or omitted range
// check: fall through to the target of the final bit test.
NextMBB = BTB.Cases[j + 1].TargetBB;
} else if (j + 1 == ej) {
// For the last bit test, fall through to Default.
Expand All @@ -1883,7 +1883,7 @@ SelectionDAGISel::FinishBasicBlock() {
SDB->clear();
CodeGenAndEmitDAG();

if (BTB.ContiguousRange && j + 2 == ej) {
if ((BTB.ContiguousRange || BTB.OmitRangeCheck) && j + 2 == ej) {
// Since we're not going to use the final bit test, remove it.
BTB.Cases.pop_back();
break;
Expand Down
15 changes: 5 additions & 10 deletions llvm/test/CodeGen/X86/SwitchLowering.ll
View file
Expand Up @@ -62,27 +62,22 @@ bb7: ; preds = %bb, %bb

declare void @foo(i8)

; PR50080
; The important part of this test is that we emit only 1 bit test rather than
; 2 since the default BB of the switch is unreachable.
define i32 @baz(i32 %0) {
; FIXME: Get rid of this conditional jump and bit test in .LBB1_1.
; FIXME: .LBB1_4 should not have .LBB1_1 as a predecessor, or be past the end
; FIXME: of the function.
; CHECK-LABEL: baz:
; CHECK: # %bb.0:
; CHECK-NEXT: xorl %eax, %eax
; CHECK-NEXT: movl {{[0-9]+}}(%esp), %ecx
; CHECK-NEXT: movl $13056, %edx # imm = 0x3300
; CHECK-NEXT: btl %ecx, %edx
; CHECK-NEXT: jae .LBB1_1
; CHECK-NEXT: # %bb.3: # %return
; CHECK-NEXT: # %bb.2: # %return
; CHECK-NEXT: retl
; CHECK-NEXT: .LBB1_1:
; CHECK-NEXT: movl $48, %eax
; CHECK-NEXT: btl %ecx, %eax
; CHECK-NEXT: jae .LBB1_4
; CHECK-NEXT: # %bb.2: # %sw.epilog8
; CHECK-NEXT: .LBB1_1: # %sw.epilog8
; CHECK-NEXT: movl $1, %eax
; CHECK-NEXT: retl
; CHECK-NEXT: .LBB1_4: # %if.then.unreachabledefault
switch i32 %0, label %if.then.unreachabledefault [
i32 4, label %sw.epilog8
i32 5, label %sw.epilog8
Expand Down
55 changes: 16 additions & 39 deletions llvm/test/CodeGen/X86/switch-bit-test-unreachable-default.ll
View file
Expand Up @@ -7,8 +7,6 @@

; PR50080
define i32 @baz(i32 %0) {
; FIXME: Get rid of this conditional jump and bit test in bb.5.
; FIXME: bb.2 should not have bb.5 as a predecessor.
; CHECK-SDISEL: bb.0 (%ir-block.1):
; CHECK-SDISEL: successors: %bb.4(0x80000000); %bb.4(100.00%)
; CHECK-SDISEL: liveins: $edi
Expand All @@ -17,77 +15,56 @@ define i32 @baz(i32 %0) {
; CHECK-SDISEL: %3:gr32 = COPY %1:gr32
; CHECK-SDISEL: bb.4 (%ir-block.1):
; CHECK-SDISEL: ; predecessors: %bb.0
; CHECK-SDISEL: successors: %bb.3(0x55555555), %bb.5(0x2aaaaaab); %bb.3(66.67%), %bb.5(33.33%)
; CHECK-SDISEL: successors: %bb.3(0x55555555), %bb.1(0x2aaaaaab); %bb.3(66.67%), %bb.1(33.33%)
; CHECK-SDISEL: %4:gr32 = MOV32ri 13056
; CHECK-SDISEL: BT32rr killed %4:gr32, %3:gr32, implicit-def $eflags
; CHECK-SDISEL: JCC_1 %bb.3, 2, implicit $eflags
; CHECK-SDISEL: JMP_1 %bb.1
; CHECK-SDISEL: bb.5 (%ir-block.1):
; CHECK-SDISEL: ; predecessors: %bb.4
; CHECK-SDISEL: successors: %bb.1(0x80000000), %bb.2(0x00000000); %bb.1(100.00%), %bb.2(0.00%)
; CHECK-SDISEL: %5:gr32 = MOV32ri 48
; CHECK-SDISEL: BT32rr killed %5:gr32, %3:gr32, implicit-def $eflags
; CHECK-SDISEL: JCC_1 %bb.1, 2, implicit $eflags
; CHECK-SDISEL: JMP_1 %bb.2
; CHECK-SDISEL: bb.1.sw.epilog8:
; CHECK-SDISEL: ; predecessors: %bb.5
; CHECK-SDISEL: ; predecessors: %bb.4
; CHECK-SDISEL: successors: %bb.3(0x80000000); %bb.3(100.00%)
; CHECK-SDISEL: %6:gr32 = MOV32ri 1
; CHECK-SDISEL: %5:gr32 = MOV32ri 1
; CHECK-SDISEL: JMP_1 %bb.3
; CHECK-SDISEL: bb.2.if.then.unreachabledefault:
; CHECK-SDISEL: ; predecessors: %bb.5
; CHECK-SDISEL: bb.3.return:
; CHECK-SDISEL: ; predecessors: %bb.4, %bb.1
; CHECK-SDISEL: %0:gr32 = PHI %2:gr32, %bb.4, %6:gr32, %bb.1
; CHECK-SDISEL: %0:gr32 = PHI %2:gr32, %bb.4, %5:gr32, %bb.1
; CHECK-SDISEL: $eax = COPY %0:gr32
; CHECK-SDISEL: RET 0, $eax


; FIXME: Get rid of this conditional jump and bit test in bb.6.
; FIXME: bb.3 should not have bb.6 as a predecessor.
; CHECK-GISEL: bb.1 (%ir-block.1):
; CHECK-GISEL: successors: %bb.5(0x80000000); %bb.5(100.00%)
; CHECK-GISEL: liveins: $edi
; CHECK-GISEL: %0:gr32 = COPY $edi
; CHECK-GISEL: %16:gr32 = MOV32ri 1
; CHECK-GISEL: %17:gr32 = MOV32r0 implicit-def $eflags
; CHECK-GISEL: %10:gr32 = MOV32ri 1
; CHECK-GISEL: %11:gr32 = MOV32r0 implicit-def $eflags
; CHECK-GISEL: %2:gr32 = SUB32ri8 %0:gr32(tied-def 0), 0, implicit-def $eflags
; CHECK-GISEL: bb.5 (%ir-block.1):
; CHECK-GISEL: ; predecessors: %bb.1
; CHECK-GISEL: successors: %bb.4(0x55555555), %bb.6(0x2aaaaaab); %bb.4(66.67%), %bb.6(33.33%)
; CHECK-GISEL: successors: %bb.4(0x55555555), %bb.2(0x2aaaaaab); %bb.4(66.67%), %bb.2(33.33%)
; CHECK-GISEL: %3:gr32 = MOV32ri 1
; CHECK-GISEL: %21:gr8 = COPY %2.sub_8bit:gr32
; CHECK-GISEL: $cl = COPY %21:gr8
; CHECK-GISEL: %13:gr8 = COPY %2.sub_8bit:gr32
; CHECK-GISEL: $cl = COPY %13:gr8
; CHECK-GISEL: %4:gr32 = SHL32rCL %3:gr32(tied-def 0), implicit-def $eflags, implicit $cl
; CHECK-GISEL: %6:gr32 = AND32ri %4:gr32(tied-def 0), 13056, implicit-def $eflags
; CHECK-GISEL: %7:gr32 = MOV32r0 implicit-def $eflags
; CHECK-GISEL: CMP32rr %6:gr32, %7:gr32, implicit-def $eflags
; CHECK-GISEL: %20:gr8 = SETCCr 5, implicit $eflags
; CHECK-GISEL: TEST8ri %20:gr8, 1, implicit-def $eflags
; CHECK-GISEL: %12:gr8 = SETCCr 5, implicit $eflags
; CHECK-GISEL: TEST8ri %12:gr8, 1, implicit-def $eflags
; CHECK-GISEL: JCC_1 %bb.4, 5, implicit $eflags
; CHECK-GISEL: JMP_1 %bb.2
; CHECK-GISEL: bb.6 (%ir-block.1):
; CHECK-GISEL: ; predecessors: %bb.5
; CHECK-GISEL: successors: %bb.2(0x80000000), %bb.3(0x00000000); %bb.2(100.00%), %bb.3(0.00%)
; CHECK-GISEL: %9:gr32 = MOV32ri 1
; CHECK-GISEL: %19:gr8 = COPY %2.sub_8bit:gr32
; CHECK-GISEL: $cl = COPY %19:gr8
; CHECK-GISEL: %10:gr32 = SHL32rCL %9:gr32(tied-def 0), implicit-def $eflags, implicit $cl
; CHECK-GISEL: %12:gr32 = AND32ri8 %10:gr32(tied-def 0), 48, implicit-def $eflags
; CHECK-GISEL: %13:gr32 = MOV32r0 implicit-def $eflags
; CHECK-GISEL: CMP32rr %12:gr32, %13:gr32, implicit-def $eflags
; CHECK-GISEL: %18:gr8 = SETCCr 5, implicit $eflags
; CHECK-GISEL: TEST8ri %18:gr8, 1, implicit-def $eflags
; CHECK-GISEL: JCC_1 %bb.2, 5, implicit $eflags
; CHECK-GISEL: JMP_1 %bb.3
; CHECK-GISEL: bb.2.sw.epilog8:
; CHECK-GISEL: ; predecessors: %bb.6
; CHECK-GISEL: ; predecessors: %bb.5
; CHECK-GISEL: successors: %bb.4(0x80000000); %bb.4(100.00%)
; CHECK-GISEL: JMP_1 %bb.4
; CHECK-GISEL: bb.3.if.then.unreachabledefault:
; CHECK-GISEL: ; predecessors: %bb.6
; CHECK-GISEL: bb.4.return:
; CHECK-GISEL: ; predecessors: %bb.5, %bb.2
; CHECK-GISEL: %15:gr32 = PHI %16:gr32, %bb.2, %17:gr32, %bb.5
; CHECK-GISEL: $eax = COPY %15:gr32
; CHECK-GISEL: %9:gr32 = PHI %10:gr32, %bb.2, %11:gr32, %bb.5
; CHECK-GISEL: $eax = COPY %9:gr32
; CHECK-GISEL: RET 0, implicit $eax

switch i32 %0, label %if.then.unreachabledefault [
Expand Down

0 comments on commit 4331f19

Please sign in to comment.