Respond to Chris' review:
[oota-llvm.git] / lib / Analysis / PHITransAddr.cpp
index 187924f45a849356e5bd0ab439bcbb03f805538c..334a188d12f7543269e5d9ab8601738a450ca156 100644 (file)
@@ -14,6 +14,7 @@
 #include "llvm/Analysis/PHITransAddr.h"
 #include "llvm/Analysis/Dominators.h"
 #include "llvm/Analysis/InstructionSimplify.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
@@ -35,12 +36,12 @@ static bool CanPHITrans(Instruction *Inst) {
 
 void PHITransAddr::dump() const {
   if (Addr == 0) {
-    errs() << "PHITransAddr: null\n";
+    dbgs() << "PHITransAddr: null\n";
     return;
   }
-  errs() << "PHITransAddr: " << *Addr << "\n";
+  dbgs() << "PHITransAddr: " << *Addr << "\n";
   for (unsigned i = 0, e = InstInputs.size(); i != e; ++i)
-    errs() << "  Input #" << i << " is " << *InstInputs[i] << "\n";
+    dbgs() << "  Input #" << i << " is " << *InstInputs[i] << "\n";
 }
 
 
@@ -110,8 +111,11 @@ bool PHITransAddr::IsPotentiallyPHITranslatable() const {
 }
 
 
-static void RemoveInstInputs(Instruction *I
+static void RemoveInstInputs(Value *V
                              SmallVectorImpl<Instruction*> &InstInputs) {
+  Instruction *I = dyn_cast<Instruction>(V);
+  if (I == 0) return;
+  
   // If the instruction is in the InstInputs list, remove it.
   SmallVectorImpl<Instruction*>::iterator Entry =
     std::find(InstInputs.begin(), InstInputs.end(), I);
@@ -120,73 +124,52 @@ static void RemoveInstInputs(Instruction *I,
     return;
   }
   
+  assert(!isa<PHINode>(I) && "Error, removing something that isn't an input");
+  
   // Otherwise, it must have instruction inputs itself.  Zap them recursively.
-  bool HadInstInputs = false;
   for (unsigned i = 0, e = I->getNumOperands(); i != e; ++i) {
-    if (Instruction *Op = dyn_cast<Instruction>(I->getOperand(i))) {
+    if (Instruction *Op = dyn_cast<Instruction>(I->getOperand(i)))
       RemoveInstInputs(Op, InstInputs);
-      HadInstInputs = true;
-    }
   }
-
-  // This instruction had to have operands in the instinputs list or it should
-  // have been in the list itself.  If not, the list is broken.
-  assert(HadInstInputs && "InstInputs list inconsistent!");
-}
-
-/// ReplaceInstWithValue - Remove any instruction inputs in the InstInputs
-/// array that are due to the specified instruction that is about to be
-/// removed from the address, and add any corresponding to V.  This returns V.
-Value *PHITransAddr::ReplaceInstWithValue(Instruction *I, Value *V) {
-  // Remove the old instruction from InstInputs.
-  RemoveInstInputs(I, InstInputs);
-  
-  // If V is an instruction, it is now an input.
-  if (Instruction *VI = dyn_cast<Instruction>(V))
-    InstInputs.push_back(VI);
-  return V;
 }
 
-
 Value *PHITransAddr::PHITranslateSubExpr(Value *V, BasicBlock *CurBB,
                                          BasicBlock *PredBB) {
   // If this is a non-instruction value, it can't require PHI translation.
   Instruction *Inst = dyn_cast<Instruction>(V);
   if (Inst == 0) return V;
   
-  // If 'Inst' is defined in this block, it must be an input that needs to be
-  // phi translated or an intermediate expression that needs to be incorporated
-  // into the expression.
-  if (Inst->getParent() == CurBB) {
-    assert(std::count(InstInputs.begin(), InstInputs.end(), Inst) &&
-           "Not an input?");
+  // Determine whether 'Inst' is an input to our PHI translatable expression.
+  bool isInput = std::count(InstInputs.begin(), InstInputs.end(), Inst);
+
+  // Handle inputs instructions if needed.
+  if (isInput) {
+    if (Inst->getParent() != CurBB) {
+      // If it is an input defined in a different block, then it remains an
+      // input.
+      return Inst;
+    }
+
+    // If 'Inst' is defined in this block and is an input that needs to be phi
+    // translated, we need to incorporate the value into the expression or fail.
+
+    // In either case, the instruction itself isn't an input any longer.
+    InstInputs.erase(std::find(InstInputs.begin(), InstInputs.end(), Inst));
     
     // If this is a PHI, go ahead and translate it.
     if (PHINode *PN = dyn_cast<PHINode>(Inst))
-      return PN->getIncomingValueForBlock(PredBB);
-
+      return AddAsInput(PN->getIncomingValueForBlock(PredBB));
     
     // If this is a non-phi value, and it is analyzable, we can incorporate it
     // into the expression by making all instruction operands be inputs.
     if (!CanPHITrans(Inst))
       return 0;
-    
-    // Okay, we can incorporate it, this instruction is no longer an input.
-    InstInputs.erase(std::find(InstInputs.begin(), InstInputs.end(), Inst));
-    
+   
     // All instruction operands are now inputs (and of course, they may also be
     // defined in this block, so they may need to be phi translated themselves.
     for (unsigned i = 0, e = Inst->getNumOperands(); i != e; ++i)
       if (Instruction *Op = dyn_cast<Instruction>(Inst->getOperand(i)))
         InstInputs.push_back(Op);
-    
-  } else {
-    // Determine whether 'Inst' is an input to our PHI translatable expression.
-    bool isInput = std::count(InstInputs.begin(), InstInputs.end(), Inst);
-    
-    // If it is an input defined in a different block, then it remains an input.
-    if (isInput)
-      return Inst;
   }
 
   // Ok, it must be an intermediate result (either because it started that way
@@ -203,8 +186,7 @@ Value *PHITransAddr::PHITranslateSubExpr(Value *V, BasicBlock *CurBB,
     
     // Constants are trivial to find.
     if (Constant *C = dyn_cast<Constant>(PHIIn))
-      return ReplaceInstWithValue(BC, ConstantExpr::getBitCast(C,
-                                                               BC->getType()));
+      return AddAsInput(ConstantExpr::getBitCast(C, BC->getType()));
     
     // Otherwise we have to see if a bitcasted version of the incoming pointer
     // is available.  If so, we can use it, otherwise we have to fail.
@@ -220,7 +202,6 @@ Value *PHITransAddr::PHITranslateSubExpr(Value *V, BasicBlock *CurBB,
   // Handle getelementptr with at least one PHI translatable operand.
   if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(Inst)) {
     SmallVector<Value*, 8> GEPOps;
-    BasicBlock *CurBB = GEP->getParent();
     bool AnyChanged = false;
     for (unsigned i = 0, e = GEP->getNumOperands(); i != e; ++i) {
       Value *GEPOp = PHITranslateSubExpr(GEP->getOperand(i), CurBB, PredBB);
@@ -234,8 +215,12 @@ Value *PHITransAddr::PHITranslateSubExpr(Value *V, BasicBlock *CurBB,
       return GEP;
     
     // Simplify the GEP to handle 'gep x, 0' -> x etc.
-    if (Value *V = SimplifyGEPInst(&GEPOps[0], GEPOps.size(), TD))
-      return ReplaceInstWithValue(GEP, V);
+    if (Value *V = SimplifyGEPInst(&GEPOps[0], GEPOps.size(), TD)) {
+      for (unsigned i = 0, e = GEPOps.size(); i != e; ++i)
+        RemoveInstInputs(GEPOps[i], InstInputs);
+      
+      return AddAsInput(V);
+    }
     
     // Scan to see if we have this GEP available.
     Value *APHIOp = GEPOps[0];
@@ -276,17 +261,32 @@ Value *PHITransAddr::PHITranslateSubExpr(Value *V, BasicBlock *CurBB,
           LHS = BOp->getOperand(0);
           RHS = ConstantExpr::getAdd(RHS, CI);
           isNSW = isNUW = false;
+          
+          // If the old 'LHS' was an input, add the new 'LHS' as an input.
+          if (std::count(InstInputs.begin(), InstInputs.end(), BOp)) {
+            RemoveInstInputs(BOp, InstInputs);
+            AddAsInput(LHS);
+          }
         }
     
     // See if the add simplifies away.
-    if (Value *Res = SimplifyAddInst(LHS, RHS, isNSW, isNUW, TD))
-      return ReplaceInstWithValue(Inst, Res);
+    if (Value *Res = SimplifyAddInst(LHS, RHS, isNSW, isNUW, TD)) {
+      // If we simplified the operands, the LHS is no longer an input, but Res
+      // is.
+      RemoveInstInputs(LHS, InstInputs);
+      return AddAsInput(Res);
+    }
+
+    // If we didn't modify the add, just return it.
+    if (LHS == Inst->getOperand(0) && RHS == Inst->getOperand(1))
+      return Inst;
     
     // Otherwise, see if we have this add available somewhere.
     for (Value::use_iterator UI = LHS->use_begin(), E = LHS->use_end();
          UI != E; ++UI) {
       if (BinaryOperator *BO = dyn_cast<BinaryOperator>(*UI))
-        if (BO->getOperand(0) == LHS && BO->getOperand(1) == RHS &&
+        if (BO->getOpcode() == Instruction::Add &&
+            BO->getOperand(0) == LHS && BO->getOperand(1) == RHS &&
             BO->getParent()->getParent() == CurBB->getParent())
           return BO;
     }