<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div><br><div><div>On Dec 20, 2012, at 8:03 AM, Manman Ren wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="content-type" content="text/html; charset=utf-8"><div dir="auto"><div>Ping</div><div><br>On Dec 19, 2012, at 10:05 AM, Manman Ren <<a href="mailto:mren@apple.com">mren@apple.com</a>> wrote:<br><br></div><blockquote type="cite"><div><div><br></div><div><br></div>Thanks for having the detailed algorithm.<div>According to Chandler (and I agree), </div><div><br></div><div><span class="Apple-style-span" style="font-family: arial, helvetica, sans-serif; font-size: 13px; "> We can't rely on LoopInfo to describe all backedges as LoopInfo doesn't represent unnatural loops -- essentially it isn't guaranteed to find all cycles.</span></div><div><font class="Apple-style-span" face="arial, helvetica, sans-serif"><span class="Apple-style-span" style="font-size: 13px;"><br></span></font></div><div><font class="Apple-style-span" face="arial, helvetica, sans-serif"><span class="Apple-style-span" style="font-size: 13px;">I now have a simple implementation of a very conservative reachablity analysis:</span></font></div><div><font class="Apple-style-span" face="arial, helvetica, sans-serif"><span class="Apple-style-span" style="font-size: 13px;"><div>+ // Conservatively return true. Return false, if there is a single path</div><div>+ // starting from "From" and the path does not reach "To".</div><div>+ static bool hasPath(const BasicBlock *From, const BasicBlock *To) {</div><div><div>+ const unsigned MaxCheck = 5;</div><div>+ const BasicBlock *Current = From;</div><div>+ for (unsigned I = 0; I < MaxCheck; I++) {</div><div>+ unsigned NumSuccs = Current->getTerminator()->getNumSuccessors();</div><div>+ if (NumSuccs > 1)</div><div>+ return true;</div><div>+ if (NumSuccs == 0)</div><div>+ return false;</div><div>+ Current = Current->getTerminator()->getSuccessor(0);</div><div>+ if (Current == To)</div><div>+ return true;</div><div>+ }</div><div>+ return true;</div><div>+ }</div></div><div><br></div></span></font></div><div><font class="Apple-style-span" face="arial, helvetica, sans-serif"><span class="Apple-style-span" style="font-size: 13px; ">And I will file a PR to request a more advanced reachability analysis.</span></font></div><div><font class="Apple-style-span" face="arial, helvetica, sans-serif"><span class="Apple-style-span" style="font-size: 13px;">Please review the attached patch,</span></font></div><div><font class="Apple-style-span" face="arial, helvetica, sans-serif"><span class="Apple-style-span" style="font-size: 13px;"><br></span></font></div><div><font class="Apple-style-span" face="arial, helvetica, sans-serif"><span class="Apple-style-span" style="font-size: 13px;">Thanks,</span></font></div><div><font class="Apple-style-span" face="arial, helvetica, sans-serif"><span class="Apple-style-span" style="font-size: 13px;">Manman</span></font></div><div><font class="Apple-style-span" face="arial, helvetica, sans-serif"><span class="Apple-style-span" style="font-size: 13px;"><br></span></font></div><div><font class="Apple-style-span" face="arial, helvetica, sans-serif"><span class="Apple-style-span" style="font-size: 13px;"></span></font></div></div></blockquote><blockquote type="cite"><div><mem_dep_2.patch></div></blockquote><blockquote type="cite"><div><div><font class="Apple-style-span" face="arial, helvetica, sans-serif"><span class="Apple-style-span" style="font-size: 13px;"><br></span></font><div><div>On Dec 19, 2012, at 2:07 AM, Nick Lewycky wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>Nick Lewycky wrote:<br><blockquote type="cite">Manman Ren wrote:<br></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">Given input:<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">while.body:<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">call void @RunInMode(i32 100) nounwind<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">%1 = load i32* %tasksIdle, align 4<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">%tobool = icmp eq i32 %1, 0<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">br i1 %tobool, label %while.cond.backedge, label %if.then<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">if.then:<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">store i32 0, i32* %tasksIdle, align 4<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">call void @TimerCreate(i32* %shouldExit) nounwind<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">br label %while.cond.backedge<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">while.cond.backedge:<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">%2 = load i32* %shouldExit, align 4<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">%cmp = icmp eq i32 %2, 0<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">br i1 %cmp, label %while.body, label %while.cond.while.end_crit_edge<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">Make sure we are not returning "call RunInMode" does not modify<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">%shouldExit.<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">Currently, we optimize this into<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">while.cond.backedge: ; preds = %if.then, %while.body<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">%1 = phi i32 [ %.pre, %if.then ], [ 0, %while.body ]<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">%cmp = icmp eq i32 %1, 0<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">br i1 %cmp, label %while.body, label %while.cond.while.end_crit_edge<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">The proper fix is to check all uses of %shouldExit as long as there is<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">a possible<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">path from the use to "call RunInMode". The proposed patch uses<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">LoopInfo to approximate the reachability analysis.<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">Suggestions on how to approximate this are welcome.<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">Please review, Thanks,<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Thanks for working on this.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">I think you should factor out logic for "is Inst2 reachable from Inst1"<br></blockquote><blockquote type="cite">and put it somewhere reusable (BasicBlockUtils?). Making it take<br></blockquote><blockquote type="cite">DominatorTree* and LoopInfo* is the right idea, but we may not have<br></blockquote><blockquote type="cite">those available. If the method requires one of those, it should take it<br></blockquote><blockquote type="cite">as a reference instead of a pointer.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">+ AU.addRequired<LoopInfo>();<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Sorry, I don't think we can make MemoryDependenceAnalysis depend on<br></blockquote><blockquote type="cite">LoopInfo.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">What we can do is check if it just happens to be available, due to the<br></blockquote><blockquote type="cite">pass ordering we happen to be running in. If it is,<br></blockquote><blockquote type="cite">getAnalysisIfAvailable<LoopInfo>() will return the pointer and we can<br></blockquote><blockquote type="cite">use it to speed up the search, or NULL.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">I spent a while thinking about whether LoopInfo ever actually helps us<br></blockquote><blockquote type="cite">prune the search, both in the case where we have a dominator tree<br></blockquote><blockquote type="cite">available and when we don't. I decided it does (in both cases), and I<br></blockquote><blockquote type="cite">wanted to quickly jot down what I think the algorithm is:<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">We're searching for whether there exists any path from A to B.<br></blockquote><blockquote type="cite">- if we have loop info and not dominator trees, set B = B's loop header<br></blockquote><blockquote type="cite">(this is an optimization only).<br></blockquote><blockquote type="cite">- push A on worklist.<br></blockquote><blockquote type="cite">- pop A' off the worklist and<br></blockquote><blockquote type="cite">- if A' and B are not both in the same loop, then A' is a dead end. Skip<br></blockquote><blockquote type="cite">to the next item on the worklist.<br></blockquote><blockquote type="cite">- if A' dominates B, we have found a path. Stop.<br></blockquote><blockquote type="cite">- push all of A''s successors onto the worklist, except the ones we've<br></blockquote><blockquote type="cite">visited before.<br></blockquote><blockquote type="cite">- if the worklist is empty, there is no path. Stop.<br></blockquote><br>Ignore this. It's both subtly wrong and rather inefficient.<br><br>- push A on the worklist.<br>- pop A' off the worklist and<br> - if there is a loop that contains both A' and B, then there is a path. Stop.<br> - if A' dominates B, there is a path. Stop.<br> - if A' is in a loop (and we have loop info), add the outermost loop's exit blocks to the worklist; else (if A' is not in a loop) add A''s successors to the worklist.<br> - if the worklist is empty, there is no path. Stop.<br><br>Nick<br><br><blockquote type="cite">Note that "in the same loop" can't be implemented by a simple<br></blockquote><blockquote type="cite">getLoopFor(A) == getLoopFor(B), because of nested loops. The test we<br></blockquote><blockquote type="cite">really want is "does there exist a Loop* that contains both blocks?".<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">If the dominator tree is not available then the test for "A' dominates<br></blockquote><blockquote type="cite">B" becomes "A' == B" with no change in precision. If the loop<br></blockquote><blockquote type="cite">information is not available, then we skip that test, again with loss of<br></blockquote><blockquote type="cite">precision. We should have some sort of visited-blocks limit to make sure<br></blockquote><blockquote type="cite">this doesn't take too much compile time.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Nick<br></blockquote><blockquote type="cite">_______________________________________________<br></blockquote><blockquote type="cite">llvm-commits mailing list<br></blockquote><blockquote type="cite"><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br></blockquote><blockquote type="cite"><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></blockquote><blockquote type="cite"><br></blockquote><br></div></blockquote></div><br></div></div></blockquote><blockquote type="cite"><div><span>_______________________________________________</span><br><span>llvm-commits mailing list</span><br><span><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a></span><br><span><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></span><br></div></blockquote></div></blockquote></div><br></div></div></body></html>