<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Very nice. Thanks Owen.<div><br></div><div>Here is one nitpick:</div><div><br></div><div><span class="Apple-style-span" style="font-family: -webkit-monospace; font-size: 11px; "> if (DTN->getIDom())<br>- localAvail.insert(std::make_pair(BB,<br>- localAvail[DTN->getIDom()->getBlock()]));<br>+ localAvail[BB] =<br>+ new ValueNumberScope(localAvail[DTN->getIDom()->getBlock()]);<br>+ else<br>+ localAvail[BB] = new ValueNumberScope(0);</span></div><div><font class="Apple-style-span" face="-webkit-monospace" size="3"><span class="Apple-style-span" style="font-size: 11px;"><br></span></font></div><div><font class="Apple-style-span" face="-webkit-monospace" size="3"><span class="Apple-style-span" style="font-size: 11px;">How about?</span></font></div><div><font class="Apple-style-span" face="-webkit-monospace" size="3"><span class="Apple-style-span" style="font-size: 11px;">ValueNumberScope* p = DTN->getIDom() ? localAvail[DTN->getIDom()->getBlock()] : 0;</span></font></div><div><font class="Apple-style-span" face="-webkit-monospace" size="3"><span class="Apple-style-span" style="font-size: 11px;">localAvail[BB] = p;<br></span></font><div><br></div><div>Evan</div><div><br><div><div>On Jun 19, 2008, at 6:15 PM, Owen Anderson wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>Author: resistor<br>Date: Thu Jun 19 20:15:47 2008<br>New Revision: 52521<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=52521&view=rev">http://llvm.org/viewvc/llvm-project?rev=52521&view=rev</a><br>Log:<br>Change around the data structures used to store availability sets, resulting in a GVN+PRE that is faster that GVN alone was before.<br><br>Modified:<br> llvm/trunk/lib/Transforms/Scalar/GVN.cpp<br><br>Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=52521&r1=52520&r2=52521&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=52521&r1=52520&r2=52521&view=diff</a><br><br>==============================================================================<br>--- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)<br>+++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Thu Jun 19 20:15:47 2008<br>@@ -693,6 +693,15 @@<br> }<br><br> namespace {<br>+ struct VISIBILITY_HIDDEN ValueNumberScope {<br>+ ValueNumberScope* parent;<br>+ DenseMap<uint32_t, Value*> table;<br>+ <br>+ ValueNumberScope(ValueNumberScope* p) : parent(p) { }<br>+ };<br>+}<br>+<br>+namespace {<br><br> class VISIBILITY_HIDDEN GVN : public FunctionPass {<br> bool runOnFunction(Function &F);<br>@@ -702,7 +711,7 @@<br><br> private:<br> ValueTable VN;<br>- DenseMap<BasicBlock*, DenseMap<uint32_t, Value*> > localAvail;<br>+ DenseMap<BasicBlock*, ValueNumberScope*> localAvail;<br><br> typedef DenseMap<Value*, SmallPtrSet<Instruction*, 4> > PhiMapType;<br> PhiMapType phiMap;<br>@@ -737,6 +746,7 @@<br> Value* CollapsePhi(PHINode* p);<br> bool isSafeReplacement(PHINode* p, Instruction* inst);<br> bool performPRE(Function& F);<br>+ Value* lookupNumber(BasicBlock* BB, uint32_t num);<br> };<br><br> char GVN::ID = 0;<br>@@ -1008,6 +1018,20 @@<br> return deletedLoad;<br> }<br><br>+Value* GVN::lookupNumber(BasicBlock* BB, uint32_t num) {<br>+ ValueNumberScope* locals = localAvail[BB];<br>+ <br>+ while (locals) {<br>+ DenseMap<uint32_t, Value*>::iterator I = locals->table.find(num);<br>+ if (I != locals->table.end())<br>+ return I->second;<br>+ else<br>+ locals = locals->parent;<br>+ }<br>+ <br>+ return 0;<br>+}<br>+<br> /// processInstruction - When calculating availability, handle an instruction<br> /// by inserting it into the appropriate sets<br> bool GVN::processInstruction(Instruction *I,<br>@@ -1018,7 +1042,7 @@<br><br> if (!changed) {<br> unsigned num = VN.lookup_or_add(L);<br>- localAvail[I->getParent()].insert(std::make_pair(num, L));<br>+ localAvail[I->getParent()]->table.insert(std::make_pair(num, L));<br> }<br><br> return changed;<br>@@ -1029,7 +1053,7 @@<br> // Allocations are always uniquely numbered, so we can save time and memory<br> // by fast failing them.<br> if (isa<AllocationInst>(I)) {<br>- localAvail[I->getParent()].insert(std::make_pair(num, I));<br>+ localAvail[I->getParent()]->table.insert(std::make_pair(num, I));<br> return false;<br> }<br><br>@@ -1046,12 +1070,10 @@<br> p->replaceAllUsesWith(constVal);<br> toErase.push_back(p);<br> } else {<br>- localAvail[I->getParent()].insert(std::make_pair(num, I));<br>+ localAvail[I->getParent()]->table.insert(std::make_pair(num, I));<br> }<br> // Perform value-number based elimination<br>- } else if (localAvail[I->getParent()].count(num)) {<br>- Value* repl = localAvail[I->getParent()][num];<br>- <br>+ } else if (Value* repl = lookupNumber(I->getParent(), num)) {<br> // Remove it!<br> MemoryDependenceAnalysis& MD = getAnalysis<MemoryDependenceAnalysis>();<br> MD.removeInstruction(I);<br>@@ -1061,7 +1083,7 @@<br> toErase.push_back(I);<br> return true;<br> } else if (!I->isTerminator()) {<br>- localAvail[I->getParent()].insert(std::make_pair(num, I));<br>+ localAvail[I->getParent()]->table.insert(std::make_pair(num, I));<br> }<br><br> return false;<br>@@ -1095,8 +1117,10 @@<br> bool changed_function = false;<br><br> if (DTN->getIDom())<br>- localAvail.insert(std::make_pair(BB,<br>- localAvail[DTN->getIDom()->getBlock()]));<br>+ localAvail[BB] =<br>+ new ValueNumberScope(localAvail[DTN->getIDom()->getBlock()]);<br>+ else<br>+ localAvail[BB] = new ValueNumberScope(0);<br><br> for (BasicBlock::iterator BI = BB->begin(), BE = BB->end();<br> BI != BE;) {<br>@@ -1161,19 +1185,29 @@<br> unsigned numWith = 0;<br> unsigned numWithout = 0;<br> BasicBlock* PREPred = 0;<br>+ DenseMap<BasicBlock*, Value*> predMap;<br> for (pred_iterator PI = pred_begin(CurrentBlock),<br> PE = pred_end(CurrentBlock); PI != PE; ++PI) {<br> // We're not interested in PRE where the block is its<br>- // own predecessor.<br>- if (*PI == CurrentBlock)<br>+ // own predecessor, on in blocks with predecessors<br>+ // that are not reachable.<br>+ if (*PI == CurrentBlock) {<br> numWithout = 2;<br>- <br>- if (!localAvail[*PI].count(valno)) {<br>+ break;<br>+ } else if (!localAvail.count(*PI)) {<br>+ numWithout = 2;<br>+ break;<br>+ }<br>+ <br>+ DenseMap<uint32_t, Value*>::iterator predV = <br>+ localAvail[*PI]->table.find(valno);<br>+ if (predV == localAvail[*PI]->table.end()) {<br> PREPred = *PI;<br> numWithout++;<br>- } else if (localAvail[*PI][valno] == BI) {<br>+ } else if (predV->second == BI) {<br> numWithout = 2;<br> } else {<br>+ predMap[*PI] = predV->second;<br> numWith++;<br> }<br> }<br>@@ -1214,11 +1248,11 @@<br> Value* op = BI->getOperand(i);<br> if (isa<Argument>(op) || isa<Constant>(op) || isa<GlobalValue>(op))<br> PREInstr->setOperand(i, op);<br>- else if (!localAvail[PREPred].count(VN.lookup(op))) {<br>+ else if (!lookupNumber(PREPred, VN.lookup(op))) {<br> success = false;<br> break;<br> } else<br>- PREInstr->setOperand(i, localAvail[PREPred][VN.lookup(op)]);<br>+ PREInstr->setOperand(i, lookupNumber(PREPred, VN.lookup(op)));<br> }<br><br> // Fail out if we encounter an operand that is not available in<br>@@ -1232,11 +1266,12 @@<br><br> PREInstr->insertBefore(PREPred->getTerminator());<br> PREInstr->setName(BI->getName() + ".pre");<br>+ predMap[PREPred] = PREInstr;<br> VN.add(PREInstr, valno);<br> NumGVNPRE++;<br><br> // Update the availability map to include the new instruction.<br>- localAvail[PREPred].insert(std::make_pair(valno, PREInstr));<br>+ localAvail[PREPred]->table.insert(std::make_pair(valno, PREInstr));<br><br> // Create a PHI to make the value available in this block.<br> PHINode* Phi = PHINode::Create(BI->getType(),<br>@@ -1244,18 +1279,17 @@<br> CurrentBlock->begin());<br> for (pred_iterator PI = pred_begin(CurrentBlock),<br> PE = pred_end(CurrentBlock); PI != PE; ++PI)<br>- Phi->addIncoming(localAvail[*PI][valno], *PI);<br>+ Phi->addIncoming(predMap[*PI], *PI);<br><br> VN.add(Phi, valno);<br><br> // The newly created PHI completely replaces the old instruction,<br> // so we need to update the maps to reflect this.<br>- for (DenseMap<BasicBlock*, DenseMap<uint32_t, Value*> >::iterator<br>- UI = localAvail.begin(), UE = localAvail.end(); UI != UE; ++UI)<br>- for (DenseMap<uint32_t, Value*>::iterator UUI = UI->second.begin(),<br>- UUE = UI->second.end(); UUI != UUE; ++UUI)<br>- if (UUI->second == BI)<br>- UUI->second = Phi;<br>+ DomTreeNode* DTN = getAnalysis<DominatorTree>()[CurrentBlock];<br>+ for (DomTreeNode::iterator UI = DTN->begin(), UE = DTN->end();<br>+ UI != UE; ++UI)<br>+ localAvail[(*UI)->getBlock()]->table[valno] = Phi;<br>+ localAvail[CurrentBlock]->table[valno] = Phi;<br><br> BI->replaceAllUsesWith(Phi);<br> VN.erase(BI);<br>@@ -1279,9 +1313,13 @@<br> bool GVN::iterateOnFunction(Function &F) {<br> // Clean out global sets from any previous functions<br> VN.clear();<br>- localAvail.clear();<br> phiMap.clear();<br><br>+ for (DenseMap<BasicBlock*, ValueNumberScope*>::iterator<br>+ I = localAvail.begin(), E = localAvail.end(); I != E; ++I)<br>+ delete I->second;<br>+ localAvail.clear();<br>+ <br> DominatorTree &DT = getAnalysis<DominatorTree>(); <br><br> // Top-down walk of the dominator tree<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></div></blockquote></div><br></div></div></body></html>