From: Chris Lattner Date: Mon, 5 Apr 2010 22:14:48 +0000 (+0000) Subject: fix a really nasty bug that Evan was tracking in SCCP. When resolving X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=e597f00e1a88847677e04a70ecfc9f21d372fd9d;p=oota-llvm.git fix a really nasty bug that Evan was tracking in SCCP. When resolving undefs in branches/switches, we have two cases: a branch on a literal undef or a branch on a symbolic value which is undef. If we have a literal undef, the code was correct: forcing it to a constant is the right thing to do. If we have a branch on a symbolic value that is undef, we should force the symbolic value to a constant, which then makes the successor block live. Forcing the condition of the branch to being a constant isn't safe if later paths become live and the value becomes overdefined. This is the case that 'forcedconstant' is designed to handle, so just use it. This fixes rdar://7765019 but there is no good testcase for this, the one I have is too insane to be useful in the future. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@100478 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Transforms/Scalar/SCCP.cpp b/lib/Transforms/Scalar/SCCP.cpp index 546b7b6cc29..4f09bee53a8 100644 --- a/lib/Transforms/Scalar/SCCP.cpp +++ b/lib/Transforms/Scalar/SCCP.cpp @@ -1521,45 +1521,48 @@ bool SCCPSolver::ResolvedUndefsIn(Function &F) { } } + // Check to see if we have a branch or switch on an undefined value. If so + // we force the branch to go one way or the other to make the successor + // values live. It doesn't really matter which way we force it. TerminatorInst *TI = BB->getTerminator(); if (BranchInst *BI = dyn_cast(TI)) { if (!BI->isConditional()) continue; if (!getValueState(BI->getCondition()).isUndefined()) continue; - } else if (SwitchInst *SI = dyn_cast(TI)) { + + // If the input to SCCP is actually branch on undef, fix the undef to + // false. + if (isa(BI->getCondition())) { + BI->setCondition(ConstantInt::getFalse(BI->getContext())); + markEdgeExecutable(BB, TI->getSuccessor(1)); + return true; + } + + // Otherwise, it is a branch on a symbolic value which is currently + // considered to be undef. Handle this by forcing the input value to the + // branch to false. + markForcedConstant(BI->getCondition(), + ConstantInt::getFalse(TI->getContext())); + return true; + } + + if (SwitchInst *SI = dyn_cast(TI)) { if (SI->getNumSuccessors() < 2) // no cases continue; if (!getValueState(SI->getCondition()).isUndefined()) continue; - } else { - continue; - } - - // If the edge to the second successor isn't thought to be feasible yet, - // mark it so now. We pick the second one so that this goes to some - // enumerated value in a switch instead of going to the default destination. - if (KnownFeasibleEdges.count(Edge(BB, TI->getSuccessor(1)))) - continue; - - // Otherwise, it isn't already thought to be feasible. Mark it as such now - // and return. This will make other blocks reachable, which will allow new - // values to be discovered and existing ones to be moved in the lattice. - markEdgeExecutable(BB, TI->getSuccessor(1)); - - // This must be a conditional branch of switch on undef. At this point, - // force the old terminator to branch to the first successor. This is - // required because we are now influencing the dataflow of the function with - // the assumption that this edge is taken. If we leave the branch condition - // as undef, then further analysis could think the undef went another way - // leading to an inconsistent set of conclusions. - if (BranchInst *BI = dyn_cast(TI)) { - BI->setCondition(ConstantInt::getFalse(BI->getContext())); - } else { - SwitchInst *SI = cast(TI); - SI->setCondition(SI->getCaseValue(1)); + + // If the input to SCCP is actually switch on undef, fix the undef to + // the first constant. + if (isa(SI->getCondition())) { + SI->setCondition(SI->getCaseValue(1)); + markEdgeExecutable(BB, TI->getSuccessor(1)); + return true; + } + + markForcedConstant(SI->getCondition(), SI->getCaseValue(1)); + return true; } - - return true; } return false;