diff --git a/lib/Bytecode/Reader/InstructionReader.cpp b/lib/Bytecode/Reader/InstructionReader.cpp index 54ca8695115..bbf0a5d5a1b 100644 --- a/lib/Bytecode/Reader/InstructionReader.cpp +++ b/lib/Bytecode/Reader/InstructionReader.cpp @@ -104,24 +104,29 @@ bool BytecodeParser::ParseInstruction(const uchar *&Buf, const uchar *EndBuf, } else if (Raw.Opcode == Instruction::PHINode) { PHINode *PN = new PHINode(Raw.Ty); switch (Raw.NumOperands) { - case 0: cerr << "Invalid phi node encountered!\n"; + case 0: + case 1: + case 3: cerr << "Invalid phi node encountered!\n"; delete PN; return true; - case 1: PN->addIncoming(getValue(Raw.Ty, Raw.Arg1)); break; - case 2: PN->addIncoming(getValue(Raw.Ty, Raw.Arg1)); - PN->addIncoming(getValue(Raw.Ty, Raw.Arg2)); break; - case 3: PN->addIncoming(getValue(Raw.Ty, Raw.Arg1)); - PN->addIncoming(getValue(Raw.Ty, Raw.Arg2)); - PN->addIncoming(getValue(Raw.Ty, Raw.Arg3)); break; + case 2: PN->addIncoming(getValue(Raw.Ty, Raw.Arg1), + (BasicBlock*)getValue(Type::LabelTy, Raw.Arg2)); + break; default: - PN->addIncoming(getValue(Raw.Ty, Raw.Arg1)); - PN->addIncoming(getValue(Raw.Ty, Raw.Arg2)); - { + PN->addIncoming(getValue(Raw.Ty, Raw.Arg1), + (BasicBlock*)getValue(Type::LabelTy, Raw.Arg2)); + if (Raw.VarArgs->size() & 1) { + cerr << "PHI Node with ODD number of arguments!\n"; + delete PN; + return true; + } else { vector &args = *Raw.VarArgs; - for (unsigned i = 0; i < args.size(); i++) - PN->addIncoming(getValue(Raw.Ty, args[i])); + for (unsigned i = 0; i < args.size(); i+=2) + PN->addIncoming(getValue(Raw.Ty, args[i]), + (BasicBlock*)getValue(Type::LabelTy, args[i+1])); } - delete Raw.VarArgs; + delete Raw.VarArgs; + break; } Res = PN; return false; diff --git a/lib/Transforms/IPO/InlineSimple.cpp b/lib/Transforms/IPO/InlineSimple.cpp index a1e3156b3b4..0a191a508fd 100644 --- a/lib/Transforms/IPO/InlineSimple.cpp +++ b/lib/Transforms/IPO/InlineSimple.cpp @@ -158,7 +158,7 @@ bool InlineMethod(BasicBlock::InstListType::iterator CIIt) { assert(RI->getReturnValue() && "Ret should have value!"); assert(RI->getReturnValue()->getType() == PHI->getType() && "Ret value not consistent in method!"); - PHI->addIncoming((Value*)RI->getReturnValue()); + PHI->addIncoming((Value*)RI->getReturnValue(), (BasicBlock*)BB); } // Add a branch to the code that was after the original Call. diff --git a/lib/Transforms/Scalar/DCE.cpp b/lib/Transforms/Scalar/DCE.cpp index fbd852f0042..720354a4dfe 100644 --- a/lib/Transforms/Scalar/DCE.cpp +++ b/lib/Transforms/Scalar/DCE.cpp @@ -77,14 +77,14 @@ static bool RemoveSingularPHIs(BasicBlock *BB) { Instruction *I = BB->getInstList().front(); if (I->getInstType() != Instruction::PHINode) return false; // No PHI nodes - cerr << "Killing PHIs from " << BB; - cerr << "Pred #0 = " << *pred_begin(BB); + //cerr << "Killing PHIs from " << BB; + //cerr << "Pred #0 = " << *pred_begin(BB); - cerr << "Method == " << BB->getParent(); + //cerr << "Method == " << BB->getParent(); do { PHINode *PN = (PHINode*)I; - assert(PN->getOperand(1) == 0 && "PHI node should only have one value!"); + assert(PN->getOperand(2) == 0 && "PHI node should only have one value!"); Value *V = PN->getOperand(0); PN->replaceAllUsesWith(V); // Replace PHI node with its single value. @@ -134,29 +134,25 @@ static void ReplaceUsesWithConstant(Instruction *I) { // static void RemovePredecessorFromBlock(BasicBlock *BB, BasicBlock *Pred) { pred_iterator PI(pred_begin(BB)), EI(pred_end(BB)); - unsigned pred_idx = 0, max_idx; + unsigned max_idx; - cerr << "RPFB: " << Pred << "From Block: " << BB; + //cerr << "RPFB: " << Pred << "From Block: " << BB; - // Find out what index the predecessor is... - for (; *PI != BB; ++PI, ++pred_idx) { - assert(PI != EI && "Pred is not a predecessor of BB!"); - } - // Loop over the rest of the predecssors until we run out, or until we find // out that there are more than 2 predecessors. - for (max_idx = pred_idx; PI != EI && max_idx < 2; ++PI, ++max_idx) /*empty*/; + for (max_idx = 0; PI != EI && max_idx < 3; ++PI, ++max_idx) /*empty*/; // If there are exactly two predecessors, then we want to nuke the PHI nodes // altogether. - bool NukePHIs = max_idx == 1; + bool NukePHIs = max_idx == 2; + assert(max_idx != 0 && "PHI Node in block with 0 predecessors!?!?!"); // Okay, now we know that we need to remove predecessor #pred_idx from all // PHI nodes. Iterate over each PHI node fixing them up BasicBlock::InstListType::iterator II(BB->getInstList().begin()); for (; (*II)->getInstType() == Instruction::PHINode; ++II) { PHINode *PN = (PHINode*)*II; - PN->removeIncomingValue(pred_idx); + PN->removeIncomingValue(BB); if (NukePHIs) { // Destroy the PHI altogether?? assert(PN->getOperand(1) == 0 && "PHI node should only have one value!"); @@ -282,7 +278,7 @@ static bool DoDCEPass(Method *M) { BasicBlock::InstListType::iterator DI = Pred->getInstList().end(); assert(Pred->getTerminator() && "Degenerate basic block encountered!"); // Empty bb??? - delete Pred->getInstList().remove(--DI); + delete Pred->getInstList().remove(--DI); // Destroy uncond branch // Move all definitions in the succecessor to the predecessor... while (!BB->getInstList().empty()) { @@ -290,12 +286,16 @@ static bool DoDCEPass(Method *M) { Instruction *Def = BB->getInstList().remove(DI); // Remove from front Pred->getInstList().push_back(Def); // Add to end... } - + // Remove basic block from the method... and advance iterator to the // next valid block... BB = BBs.remove(BBIt); --BBIt; // remove puts us on the NEXT bb. We want the prev BB Changed = true; + + // Make all PHI nodes that refered to BB now refer to Pred as their + // source... + BB->replaceAllUsesWith(Pred); // Inherit predecessors name if it exists... if (BB->hasName() && !Pred->hasName()) Pred->setName(BB->getName()); diff --git a/lib/VMCore/InstrTypes.cpp b/lib/VMCore/InstrTypes.cpp index decb4ad3864..15db9e07a71 100644 --- a/lib/VMCore/InstrTypes.cpp +++ b/lib/VMCore/InstrTypes.cpp @@ -45,7 +45,9 @@ PHINode::PHINode(const PHINode &PN) : Instruction(PN.getType(), Instruction::PHINode) { for (unsigned i = 0; i < PN.IncomingValues.size(); i++) - IncomingValues.push_back(Use(PN.IncomingValues[i], this)); + IncomingValues.push_back( + make_pair(Use(PN.IncomingValues[i].first, this), + BasicBlockUse(PN.IncomingValues[i].second, this))); } void PHINode::dropAllReferences() { @@ -54,20 +56,37 @@ void PHINode::dropAllReferences() { bool PHINode::setOperand(unsigned i, Value *Val) { assert(Val && "PHI node must only reference nonnull definitions!"); - if (i >= IncomingValues.size()) return false; + if (i >= IncomingValues.size()*2) return false; - IncomingValues[i] = Val; + if (i & 1) { + assert(Val->getValueType() == BasicBlockVal && "Not a BB!"); + IncomingValues[i/2].second = (BasicBlock*)Val; + } else { + IncomingValues[i/2].first = Val; + } return true; } -void PHINode::addIncoming(Value *D) { - IncomingValues.push_back(Use(D, this)); +void PHINode::addIncoming(Value *D, BasicBlock *BB) { + IncomingValues.push_back(make_pair(Use(D, this), BasicBlockUse(BB, this))); } +struct FindBBEntry { + const BasicBlock *BB; + inline FindBBEntry(const BasicBlock *bb) : BB(bb) {} + inline bool operator()(const pair &Entry) { + return Entry.second == BB; + } +}; + + // removeIncomingValue - Remove an incoming value. This is useful if a // predecessor basic block is deleted. -Value *PHINode::removeIncomingValue(unsigned idx) { - Value *Removed = IncomingValues[idx]; - IncomingValues.erase(IncomingValues.begin()+idx); +Value *PHINode::removeIncomingValue(const BasicBlock *BB) { + vector::iterator Idx = find_if(IncomingValues.begin(), + IncomingValues.end(), FindBBEntry(BB)); + assert(Idx != IncomingValues.end() && "BB not in PHI node!"); + Value *Removed = Idx->first; + IncomingValues.erase(Idx); return Removed; } diff --git a/lib/VMCore/Value.cpp b/lib/VMCore/Value.cpp index 1c3f76bc4b6..ee642f69ce9 100644 --- a/lib/VMCore/Value.cpp +++ b/lib/VMCore/Value.cpp @@ -27,6 +27,12 @@ Value::Value(const Type *ty, ValueTy vty, const string &name = "") : Name(name){ Value::~Value() { #ifndef NDEBUG // Only in -g mode... + // Check to make sure that there are no uses of this value that are still + // around when the value is destroyed. If there are, then we have a dangling + // reference and something is wrong. This code is here to print out what is + // still being referenced. The value in question should be printed as + // a + // if (Uses.begin() != Uses.end()) { for (use_const_iterator I = Uses.begin(); I != Uses.end(); I++) cerr << "Use still stuck around after Def is destroyed:" << *I << endl;