[ISEL][BitTestBlock] omit additional bit test when default destination 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: https://github.com/ClangBuiltLinux/linux/issues/679
Fixes: https://github.com/ClangBuiltLinux/linux/issues/1440

Reviewed By: hans

Differential Revision: https://reviews.llvm.org/D109103
This commit is contained in:
Nick Desaulniers 2021-09-08 10:44:16 -07:00
parent 3f875134a7
commit 4331f19d8b
5 changed files with 29 additions and 58 deletions

View File

@ -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;
@ -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.

View File

@ -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) {

View File

@ -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.
@ -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;

View File

@ -62,10 +62,10 @@ 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
@ -73,16 +73,11 @@ define i32 @baz(i32 %0) {
; 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

View File

@ -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
@ -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 [