From: Sanjay Patel Date: Thu, 19 Aug 2021 12:43:51 +0000 (-0400) Subject: Revert "[CVP] processSwitch: Remove default case when switch cover all possible values." X-Git-Tag: upstream/15.0.7~33514 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=ec54e275f56cc042eb9c25acd76bff18b9ea8092;p=platform%2Fupstream%2Fllvm.git Revert "[CVP] processSwitch: Remove default case when switch cover all possible values." This reverts commit 9934a5b2ed5aa6e6bbb2e55c3cd98839722c226e. This patch may cause miscompiles because it missed a constraint as shown in the examples from: https://llvm.org/PR51531 --- diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h index f003615..97686d7 100644 --- a/llvm/include/llvm/Transforms/Utils/Local.h +++ b/llvm/include/llvm/Transforms/Utils/Local.h @@ -55,7 +55,6 @@ class MDNode; class MemorySSAUpdater; class PHINode; class StoreInst; -class SwitchInst; class TargetLibraryInfo; class TargetTransformInfo; @@ -237,10 +236,6 @@ CallInst *createCallMatchingInvoke(InvokeInst *II); /// This function converts the specified invoek into a normall call. void changeToCall(InvokeInst *II, DomTreeUpdater *DTU = nullptr); -/// This function removes the default destination from the specified switch. -void createUnreachableSwitchDefault(SwitchInst *Switch, - DomTreeUpdater *DTU = nullptr); - ///===---------------------------------------------------------------------===// /// Dbg Intrinsic utilities /// diff --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp index cd38ce9..36cbd42 100644 --- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp +++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp @@ -341,13 +341,7 @@ static bool processSwitch(SwitchInst *I, LazyValueInfo *LVI, // ConstantFoldTerminator() as the underlying SwitchInst can be changed. SwitchInstProfUpdateWrapper SI(*I); - APInt Low = - APInt::getSignedMaxValue(Cond->getType()->getScalarSizeInBits()); - APInt High = - APInt::getSignedMinValue(Cond->getType()->getScalarSizeInBits()); - - SwitchInst::CaseIt CI = SI->case_begin(); - for (auto CE = SI->case_end(); CI != CE;) { + for (auto CI = SI->case_begin(), CE = SI->case_end(); CI != CE;) { ConstantInt *Case = CI->getCaseValue(); LazyValueInfo::Tristate State = LVI->getPredicateAt(CmpInst::ICMP_EQ, Cond, Case, I, @@ -380,28 +374,9 @@ static bool processSwitch(SwitchInst *I, LazyValueInfo *LVI, break; } - // Get Lower/Upper bound from switch cases. - Low = APIntOps::smin(Case->getValue(), Low); - High = APIntOps::smax(Case->getValue(), High); - // Increment the case iterator since we didn't delete it. ++CI; } - - // Try to simplify default case as unreachable - if (CI == SI->case_end() && SI->getNumCases() != 0 && - !isa(SI->getDefaultDest()->getFirstNonPHIOrDbg())) { - const ConstantRange SIRange = - LVI->getConstantRange(SI->getCondition(), SI); - - // If the numbered switch cases cover the entire range of the condition, - // then the default case is not reachable. - if (SIRange.getSignedMin() == Low && SIRange.getSignedMax() == High && - SI->getNumCases() == High - Low + 1) { - createUnreachableSwitchDefault(SI, &DTU); - Changed = true; - } - } } if (Changed) diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp index 862e3c3..3d6ffde 100644 --- a/llvm/lib/Transforms/Utils/Local.cpp +++ b/llvm/lib/Transforms/Utils/Local.cpp @@ -2182,30 +2182,6 @@ void llvm::changeToCall(InvokeInst *II, DomTreeUpdater *DTU) { DTU->applyUpdates({{DominatorTree::Delete, BB, UnwindDestBB}}); } -void llvm::createUnreachableSwitchDefault(SwitchInst *Switch, - DomTreeUpdater *DTU) { - LLVM_DEBUG(dbgs() << "Switch default is dead.\n"); - auto *BB = Switch->getParent(); - BasicBlock *NewDefaultBlock = SplitBlockPredecessors( - Switch->getDefaultDest(), Switch->getParent(), "", DTU); - auto *OrigDefaultBlock = Switch->getDefaultDest(); - Switch->setDefaultDest(&*NewDefaultBlock); - if (DTU) - DTU->applyUpdates({{DominatorTree::Insert, BB, &*NewDefaultBlock}, - {DominatorTree::Delete, BB, OrigDefaultBlock}}); - - SplitBlock(&*NewDefaultBlock, &NewDefaultBlock->front(), DTU); - SmallVector Updates; - if (DTU) - for (auto *Successor : successors(NewDefaultBlock)) - Updates.push_back({DominatorTree::Delete, NewDefaultBlock, Successor}); - auto *NewTerminator = NewDefaultBlock->getTerminator(); - new UnreachableInst(Switch->getContext(), NewTerminator); - NewTerminator->eraseFromParent(); - if (DTU) - DTU->applyUpdates(Updates); -} - BasicBlock *llvm::changeToInvokeAndSplitBasicBlock(CallInst *CI, BasicBlock *UnwindEdge, DomTreeUpdater *DTU) { diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index 8f45996..847fdd7 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -4743,6 +4743,29 @@ static bool CasesAreContiguous(SmallVectorImpl &Cases) { return true; } +static void createUnreachableSwitchDefault(SwitchInst *Switch, + DomTreeUpdater *DTU) { + LLVM_DEBUG(dbgs() << "SimplifyCFG: switch default is dead.\n"); + auto *BB = Switch->getParent(); + BasicBlock *NewDefaultBlock = SplitBlockPredecessors( + Switch->getDefaultDest(), Switch->getParent(), "", DTU); + auto *OrigDefaultBlock = Switch->getDefaultDest(); + Switch->setDefaultDest(&*NewDefaultBlock); + if (DTU) + DTU->applyUpdates({{DominatorTree::Insert, BB, &*NewDefaultBlock}, + {DominatorTree::Delete, BB, OrigDefaultBlock}}); + SplitBlock(&*NewDefaultBlock, &NewDefaultBlock->front(), DTU); + SmallVector Updates; + if (DTU) + for (auto *Successor : successors(NewDefaultBlock)) + Updates.push_back({DominatorTree::Delete, NewDefaultBlock, Successor}); + auto *NewTerminator = NewDefaultBlock->getTerminator(); + new UnreachableInst(Switch->getContext(), NewTerminator); + EraseTerminatorAndDCECond(NewTerminator); + if (DTU) + DTU->applyUpdates(Updates); +} + /// Turn a switch with two reachable destinations into an integer range /// comparison and branch. bool SimplifyCFGOpt::TurnSwitchRangeIntoICmp(SwitchInst *SI, diff --git a/llvm/test/Transforms/CorrelatedValuePropagation/basic.ll b/llvm/test/Transforms/CorrelatedValuePropagation/basic.ll index cbb6e2b..42b4414 100644 --- a/llvm/test/Transforms/CorrelatedValuePropagation/basic.ll +++ b/llvm/test/Transforms/CorrelatedValuePropagation/basic.ll @@ -382,7 +382,7 @@ define i32 @switch_range(i32 %cond) { ; CHECK-NEXT: entry: ; CHECK-NEXT: [[S:%.*]] = urem i32 [[COND:%.*]], 3 ; CHECK-NEXT: [[S1:%.*]] = add nuw nsw i32 [[S]], 1 -; CHECK-NEXT: switch i32 [[S1]], label [[UNREACHABLE1:%.*]] [ +; CHECK-NEXT: switch i32 [[S1]], label [[UNREACHABLE:%.*]] [ ; CHECK-NEXT: i32 1, label [[EXIT1:%.*]] ; CHECK-NEXT: i32 2, label [[EXIT2:%.*]] ; CHECK-NEXT: i32 3, label [[EXIT1]] @@ -391,10 +391,6 @@ define i32 @switch_range(i32 %cond) { ; CHECK-NEXT: ret i32 1 ; CHECK: exit2: ; CHECK-NEXT: ret i32 2 -; CHECK: unreachable1: -; CHECK-NEXT: unreachable -; CHECK: unreachable1.split: -; CHECK-NEXT: br label [[UNREACHABLE:%.*]] ; CHECK: unreachable: ; CHECK-NEXT: ret i32 0 ; @@ -415,38 +411,6 @@ unreachable: ret i32 0 } -define i32 @switch_range_not_full(i32 %cond) { -; CHECK-LABEL: @switch_range_not_full( -; CHECK-NEXT: entry: -; CHECK-NEXT: [[S:%.*]] = urem i32 [[COND:%.*]], 3 -; CHECK-NEXT: [[S1:%.*]] = add nuw nsw i32 [[S]], 1 -; CHECK-NEXT: switch i32 [[S1]], label [[UNREACHABLE:%.*]] [ -; CHECK-NEXT: i32 1, label [[EXIT1:%.*]] -; CHECK-NEXT: i32 3, label [[EXIT2:%.*]] -; CHECK-NEXT: ] -; CHECK: exit1: -; CHECK-NEXT: ret i32 1 -; CHECK: exit2: -; CHECK-NEXT: ret i32 2 -; CHECK: unreachable: -; CHECK-NEXT: ret i32 0 -; -entry: - %s = urem i32 %cond, 3 - %s1 = add i32 %s, 1 - switch i32 %s1, label %unreachable [ - i32 1, label %exit1 - i32 3, label %exit2 - ] - -exit1: - ret i32 1 -exit2: - ret i32 2 -unreachable: - ret i32 0 -} - define i1 @arg_attribute(i8* nonnull %a) { ; CHECK-LABEL: @arg_attribute( ; CHECK-NEXT: ret i1 false