[llvm-commits] [llvm] r100478 - /llvm/trunk/lib/Transforms/Scalar/SCCP.cpp

[Chris Lattner]

Author: lattner
Date: Mon Apr  5 17:14:48 2010
New Revision: 100478

URL: http://llvm.org/viewvc/llvm-project?rev=100478&view=rev
Log:
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.

Modified:
    llvm/trunk/lib/Transforms/Scalar/SCCP.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/SCCP.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SCCP.cpp?rev=100478&r1=100477&r2=100478&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/SCCP.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/SCCP.cpp Mon Apr  5 17:14:48 2010
@@ -1521,45 +1521,48 @@
       }
     }
   
+    // 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<BranchInst>(TI)) {
       if (!BI->isConditional()) continue;
       if (!getValueState(BI->getCondition()).isUndefined())
         continue;
-    } else if (SwitchInst *SI = dyn_cast<SwitchInst>(TI)) {
+    
+      // If the input to SCCP is actually branch on undef, fix the undef to
+      // false.
+      if (isa<UndefValue>(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<SwitchInst>(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<BranchInst>(TI)) {
-      BI->setCondition(ConstantInt::getFalse(BI->getContext()));
-    } else {
-      SwitchInst *SI = cast<SwitchInst>(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<UndefValue>(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;