From 50c01fd2a32cd16216208c7d5f441f716aede22d Mon Sep 17 00:00:00 2001 From: Yingwei Zheng Date: Sun, 1 Jun 2025 12:35:14 +0800 Subject: [PATCH 1/3] [CVP] Add pre-commit tests. NFC. --- .../CorrelatedValuePropagation/switch.ll | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/llvm/test/Transforms/CorrelatedValuePropagation/switch.ll b/llvm/test/Transforms/CorrelatedValuePropagation/switch.ll index a0794d5efe932..456807bc499f8 100644 --- a/llvm/test/Transforms/CorrelatedValuePropagation/switch.ll +++ b/llvm/test/Transforms/CorrelatedValuePropagation/switch.ll @@ -294,6 +294,44 @@ cleanup: ret i32 %retval.0 } +; Make sure that we don't branch into unreachable. + +define void @pr142286() { +; CHECK-LABEL: define void @pr142286() { +; CHECK-NEXT: start: +; CHECK-NEXT: br label [[LOOP:%.*]] +; CHECK: loop: +; CHECK-NEXT: br label [[LOOP2:%.*]] +; CHECK: loop2: +; CHECK-NEXT: br label [[LOOP3:%.*]] +; CHECK: loop3: +; CHECK-NEXT: br label [[DEFAULT_UNREACHABLE:%.*]] +; CHECK: default.unreachable: +; CHECK-NEXT: unreachable +; CHECK: exit: +; CHECK-NEXT: ret void +; +start: + br label %loop + +loop: + %phi = phi i8 [ -1, %start ], [ 0, %loop3 ] + br label %loop2 + +loop2: + br label %loop3 + +loop3: + switch i8 %phi, label %exit [ + i8 0, label %loop3 + i8 1, label %loop2 + i8 2, label %loop + ] + +exit: + ret void +} + declare i32 @call0() declare i32 @call1() declare i32 @call2() From 4686664f341d5f5ea13b4bd625b69e612d6db1f9 Mon Sep 17 00:00:00 2001 From: Yingwei Zheng Date: Sun, 1 Jun 2025 12:39:11 +0800 Subject: [PATCH 2/3] [CVP] Keep ReachableCaseCount in sync with range of condition --- llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp | 8 +++++--- llvm/test/Transforms/CorrelatedValuePropagation/switch.ll | 4 +--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp index 5226aeb66f65a..4228d0529dc13 100644 --- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp +++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp @@ -370,7 +370,7 @@ static bool processSwitch(SwitchInst *I, LazyValueInfo *LVI, { // Scope for SwitchInstProfUpdateWrapper. It must not live during // ConstantFoldTerminator() as the underlying SwitchInst can be changed. SwitchInstProfUpdateWrapper SI(*I); - unsigned ReachableCaseCount = 0; + SmallVector ReachableCaseValues; for (auto CI = SI->case_begin(), CE = SI->case_end(); CI != CE;) { ConstantInt *Case = CI->getCaseValue(); @@ -407,13 +407,15 @@ static bool processSwitch(SwitchInst *I, LazyValueInfo *LVI, // Increment the case iterator since we didn't delete it. ++CI; - ++ReachableCaseCount; + ReachableCaseValues.push_back(Case->getValue()); } - if (ReachableCaseCount > 1 && !SI->defaultDestUnreachable()) { + if (ReachableCaseValues.size() > 1 && !SI->defaultDestUnreachable()) { BasicBlock *DefaultDest = SI->getDefaultDest(); ConstantRange CR = LVI->getConstantRangeAtUse(I->getOperandUse(0), /*UndefAllowed*/ false); + unsigned ReachableCaseCount = count_if( + ReachableCaseValues, [&](const APInt &V) { return CR.contains(V); }); // The default dest is unreachable if all cases are covered. if (!CR.isSizeLargerThan(ReachableCaseCount)) { BasicBlock *NewUnreachableBB = diff --git a/llvm/test/Transforms/CorrelatedValuePropagation/switch.ll b/llvm/test/Transforms/CorrelatedValuePropagation/switch.ll index 456807bc499f8..7e6aa3eeebe20 100644 --- a/llvm/test/Transforms/CorrelatedValuePropagation/switch.ll +++ b/llvm/test/Transforms/CorrelatedValuePropagation/switch.ll @@ -305,9 +305,7 @@ define void @pr142286() { ; CHECK: loop2: ; CHECK-NEXT: br label [[LOOP3:%.*]] ; CHECK: loop3: -; CHECK-NEXT: br label [[DEFAULT_UNREACHABLE:%.*]] -; CHECK: default.unreachable: -; CHECK-NEXT: unreachable +; CHECK-NEXT: br label [[EXIT:%.*]] ; CHECK: exit: ; CHECK-NEXT: ret void ; From e2f18f46e9a1f4b63d488c60350216e88fc3439b Mon Sep 17 00:00:00 2001 From: Yingwei Zheng Date: Sun, 1 Jun 2025 14:15:00 +0800 Subject: [PATCH 3/3] [CVP] Always compute CR --- .../Scalar/CorrelatedValuePropagation.cpp | 66 +++++++++++-------- 1 file changed, 38 insertions(+), 28 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp index 4228d0529dc13..b95a851c99b49 100644 --- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp +++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp @@ -370,15 +370,30 @@ static bool processSwitch(SwitchInst *I, LazyValueInfo *LVI, { // Scope for SwitchInstProfUpdateWrapper. It must not live during // ConstantFoldTerminator() as the underlying SwitchInst can be changed. SwitchInstProfUpdateWrapper SI(*I); - SmallVector ReachableCaseValues; + ConstantRange CR = + LVI->getConstantRangeAtUse(I->getOperandUse(0), /*UndefAllowed=*/false); + unsigned ReachableCaseCount = 0; for (auto CI = SI->case_begin(), CE = SI->case_end(); CI != CE;) { ConstantInt *Case = CI->getCaseValue(); - auto *Res = dyn_cast_or_null( - LVI->getPredicateAt(CmpInst::ICMP_EQ, Cond, Case, I, - /* UseBlockValue */ true)); + std::optional Predicate = std::nullopt; + if (!CR.contains(Case->getValue())) + Predicate = false; + else if (CR.isSingleElement() && + *CR.getSingleElement() == Case->getValue()) + Predicate = true; + if (!Predicate) { + // Handle missing cases, e.g., the range has a hole. + auto *Res = dyn_cast_or_null( + LVI->getPredicateAt(CmpInst::ICMP_EQ, Cond, Case, I, + /* UseBlockValue=*/true)); + if (Res && Res->isZero()) + Predicate = false; + else if (Res && Res->isOne()) + Predicate = true; + } - if (Res && Res->isZero()) { + if (Predicate && !*Predicate) { // This case never fires - remove it. BasicBlock *Succ = CI->getCaseSuccessor(); Succ->removePredecessor(BB); @@ -395,7 +410,7 @@ static bool processSwitch(SwitchInst *I, LazyValueInfo *LVI, DTU.applyUpdatesPermissive({{DominatorTree::Delete, BB, Succ}}); continue; } - if (Res && Res->isOne()) { + if (Predicate && *Predicate) { // This case always fires. Arrange for the switch to be turned into an // unconditional branch by replacing the switch condition with the case // value. @@ -407,32 +422,27 @@ static bool processSwitch(SwitchInst *I, LazyValueInfo *LVI, // Increment the case iterator since we didn't delete it. ++CI; - ReachableCaseValues.push_back(Case->getValue()); + ++ReachableCaseCount; } - if (ReachableCaseValues.size() > 1 && !SI->defaultDestUnreachable()) { + // The default dest is unreachable if all cases are covered. + if (!SI->defaultDestUnreachable() && + !CR.isSizeLargerThan(ReachableCaseCount)) { BasicBlock *DefaultDest = SI->getDefaultDest(); - ConstantRange CR = LVI->getConstantRangeAtUse(I->getOperandUse(0), - /*UndefAllowed*/ false); - unsigned ReachableCaseCount = count_if( - ReachableCaseValues, [&](const APInt &V) { return CR.contains(V); }); - // The default dest is unreachable if all cases are covered. - if (!CR.isSizeLargerThan(ReachableCaseCount)) { - BasicBlock *NewUnreachableBB = - BasicBlock::Create(BB->getContext(), "default.unreachable", - BB->getParent(), DefaultDest); - new UnreachableInst(BB->getContext(), NewUnreachableBB); - - DefaultDest->removePredecessor(BB); - SI->setDefaultDest(NewUnreachableBB); - - if (SuccessorsCount[DefaultDest] == 1) - DTU.applyUpdates({{DominatorTree::Delete, BB, DefaultDest}}); - DTU.applyUpdates({{DominatorTree::Insert, BB, NewUnreachableBB}}); + BasicBlock *NewUnreachableBB = + BasicBlock::Create(BB->getContext(), "default.unreachable", + BB->getParent(), DefaultDest); + new UnreachableInst(BB->getContext(), NewUnreachableBB); - ++NumDeadCases; - Changed = true; - } + DefaultDest->removePredecessor(BB); + SI->setDefaultDest(NewUnreachableBB); + + if (SuccessorsCount[DefaultDest] == 1) + DTU.applyUpdates({{DominatorTree::Delete, BB, DefaultDest}}); + DTU.applyUpdates({{DominatorTree::Insert, BB, NewUnreachableBB}}); + + ++NumDeadCases; + Changed = true; } }