Skip to content

Revert "[SimplifyCFG] switch: Do Not Transform the Default Case if the Condition is Too Wide" #78469

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.

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 2 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 7 additions & 33 deletions llvm/lib/Transforms/Utils/SimplifyCFG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5414,13 +5414,11 @@ static bool CasesAreContiguous(SmallVectorImpl<ConstantInt *> &Cases) {
}

static void createUnreachableSwitchDefault(SwitchInst *Switch,
DomTreeUpdater *DTU,
bool RemoveOrigDefaultBlock = true) {
DomTreeUpdater *DTU) {
LLVM_DEBUG(dbgs() << "SimplifyCFG: switch default is dead.\n");
auto *BB = Switch->getParent();
auto *OrigDefaultBlock = Switch->getDefaultDest();
if (RemoveOrigDefaultBlock)
OrigDefaultBlock->removePredecessor(BB);
OrigDefaultBlock->removePredecessor(BB);
BasicBlock *NewDefaultBlock = BasicBlock::Create(
BB->getContext(), BB->getName() + ".unreachabledefault", BB->getParent(),
OrigDefaultBlock);
Expand All @@ -5429,8 +5427,7 @@ static void createUnreachableSwitchDefault(SwitchInst *Switch,
if (DTU) {
SmallVector<DominatorTree::UpdateType, 2> Updates;
Updates.push_back({DominatorTree::Insert, BB, &*NewDefaultBlock});
if (RemoveOrigDefaultBlock &&
!is_contained(successors(BB), OrigDefaultBlock))
if (!is_contained(successors(BB), OrigDefaultBlock))
Updates.push_back({DominatorTree::Delete, BB, &*OrigDefaultBlock});
DTU->applyUpdates(Updates);
}
Expand Down Expand Up @@ -5612,33 +5609,10 @@ static bool eliminateDeadSwitchCases(SwitchInst *SI, DomTreeUpdater *DTU,
Known.getBitWidth() - (Known.Zero | Known.One).popcount();
assert(NumUnknownBits <= Known.getBitWidth());
if (HasDefault && DeadCases.empty() &&
NumUnknownBits < 64 /* avoid overflow */) {
uint64_t AllNumCases = 1ULL << NumUnknownBits;
if (SI->getNumCases() == AllNumCases) {
createUnreachableSwitchDefault(SI, DTU);
return true;
}
// When only one case value is missing, replace default with that case.
// Eliminating the default branch will provide more opportunities for
// optimization, such as lookup tables.
if (SI->getNumCases() == AllNumCases - 1) {
assert(NumUnknownBits > 1 && "Should be canonicalized to a branch");
IntegerType *CondTy = cast<IntegerType>(Cond->getType());
if (CondTy->getIntegerBitWidth() > 64 ||
!DL.fitsInLegalInteger(CondTy->getIntegerBitWidth()))
return false;

uint64_t MissingCaseVal = 0;
for (const auto &Case : SI->cases())
MissingCaseVal ^= Case.getCaseValue()->getValue().getLimitedValue();
auto *MissingCase =
cast<ConstantInt>(ConstantInt::get(Cond->getType(), MissingCaseVal));
SwitchInstProfUpdateWrapper SIW(*SI);
SIW.addCase(MissingCase, SI->getDefaultDest(), SIW.getSuccessorWeight(0));
createUnreachableSwitchDefault(SI, DTU, /*RemoveOrigDefaultBlock*/ false);
SIW.setSuccessorWeight(0, 0);
return true;
}
NumUnknownBits < 64 /* avoid overflow */ &&
SI->getNumCases() == (1ULL << NumUnknownBits)) {
createUnreachableSwitchDefault(SI, DTU);
return true;
}

if (DeadCases.empty())
Expand Down

This file was deleted.

89 changes: 3 additions & 86 deletions llvm/test/Transforms/SimplifyCFG/switch-dead-default.ll
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,15 @@ default:
ret void
}

; We can replace the default branch with case 3 since it is the only case that is missing.
; This one is a negative test - we know the value of the default,
; but that's about it
define void @test3(i2 %a) {
; CHECK-LABEL: define void @test3(
; CHECK-SAME: i2 [[A:%.*]]) {
; CHECK-NEXT: switch i2 [[A]], label [[DOTUNREACHABLEDEFAULT:%.*]] [
; CHECK-NEXT: switch i2 [[A]], label [[DEFAULT:%.*]] [
; CHECK-NEXT: i2 0, label [[CASE0:%.*]]
; CHECK-NEXT: i2 1, label [[CASE1:%.*]]
; CHECK-NEXT: i2 -2, label [[CASE2:%.*]]
; CHECK-NEXT: i2 -1, label [[DEFAULT:%.*]]
; CHECK-NEXT: ]
; CHECK: common.ret:
; CHECK-NEXT: ret void
Expand All @@ -100,8 +100,6 @@ define void @test3(i2 %a) {
; CHECK: case2:
; CHECK-NEXT: call void @foo(i32 2)
; CHECK-NEXT: br label [[COMMON_RET]]
; CHECK: .unreachabledefault:
; CHECK-NEXT: unreachable
; CHECK: default:
; CHECK-NEXT: call void @foo(i32 3)
; CHECK-NEXT: br label [[COMMON_RET]]
Expand All @@ -124,50 +122,6 @@ default:
ret void
}

define void @test3_prof(i2 %a) {
; CHECK-LABEL: define void @test3_prof(
; CHECK-SAME: i2 [[A:%.*]]) {
; CHECK-NEXT: switch i2 [[A]], label [[DOTUNREACHABLEDEFAULT:%.*]] [
; CHECK-NEXT: i2 0, label [[CASE0:%.*]]
; CHECK-NEXT: i2 1, label [[CASE1:%.*]]
; CHECK-NEXT: i2 -2, label [[CASE2:%.*]]
; CHECK-NEXT: i2 -1, label [[DEFAULT:%.*]]
; CHECK-NEXT: ], !prof [[PROF0:![0-9]+]]
; CHECK: common.ret:
; CHECK-NEXT: ret void
; CHECK: case0:
; CHECK-NEXT: call void @foo(i32 0)
; CHECK-NEXT: br label [[COMMON_RET:%.*]]
; CHECK: case1:
; CHECK-NEXT: call void @foo(i32 1)
; CHECK-NEXT: br label [[COMMON_RET]]
; CHECK: case2:
; CHECK-NEXT: call void @foo(i32 2)
; CHECK-NEXT: br label [[COMMON_RET]]
; CHECK: .unreachabledefault:
; CHECK-NEXT: unreachable
; CHECK: default:
; CHECK-NEXT: call void @foo(i32 3)
; CHECK-NEXT: br label [[COMMON_RET]]
;
switch i2 %a, label %default [i2 0, label %case0
i2 1, label %case1
i2 2, label %case2], !prof !0

case0:
call void @foo(i32 0)
ret void
case1:
call void @foo(i32 1)
ret void
case2:
call void @foo(i32 2)
ret void
default:
call void @foo(i32 3)
ret void
}

; Negative test - check for possible overflow when computing
; number of possible cases.
define void @test4(i128 %a) {
Expand Down Expand Up @@ -313,40 +267,3 @@ default:

declare void @llvm.assume(i1)

define zeroext i1 @test8(i128 %a) {
; We should not transform conditions wider than 64 bit.
; CHECK-LABEL: define zeroext i1 @test8(
; CHECK-SAME: i128 [[A:%.*]]) {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = and i128 [[A]], 3894222643901120721397872246915072
; CHECK-NEXT: switch i128 [[TMP0]], label [[LOR_RHS:%.*]] [
; CHECK-NEXT: i128 1298074214633706907132624082305024, label [[LOR_END:%.*]]
; CHECK-NEXT: i128 2596148429267413814265248164610048, label [[LOR_END]]
; CHECK-NEXT: i128 3894222643901120721397872246915072, label [[LOR_END]]
; CHECK-NEXT: ]
; CHECK: lor.rhs:
; CHECK-NEXT: br label [[LOR_END]]
; CHECK: lor.end:
; CHECK-NEXT: [[TMP1:%.*]] = phi i1 [ true, [[ENTRY:%.*]] ], [ false, [[LOR_RHS]] ], [ true, [[ENTRY]] ], [ true, [[ENTRY]] ]
; CHECK-NEXT: ret i1 [[TMP1]]
;
entry:
%0 = and i128 %a, 3894222643901120721397872246915072
switch i128 %0, label %lor.rhs [
i128 1298074214633706907132624082305024, label %lor.end
i128 2596148429267413814265248164610048, label %lor.end
i128 3894222643901120721397872246915072, label %lor.end
]

lor.rhs: ; preds = %entry
br label %lor.end

lor.end: ; preds = %entry, %entry, %entry, %lor.rhs
%1 = phi i1 [ true, %entry ], [ false, %lor.rhs ], [ true, %entry ], [ true, %entry ]
ret i1 %1
}

!0 = !{!"branch_weights", i32 8, i32 4, i32 2, i32 1}
;.
; CHECK: [[PROF0]] = !{!"branch_weights", i32 0, i32 4, i32 2, i32 1, i32 8}
;.