<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, May 8, 2017 at 2:59 PM, Aditya Kumar via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">hiraditya added a comment.<br>
<br>
In <a href="https://reviews.llvm.org/D32614#740129" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32614#740129</a>, @dberlin wrote:<br>
<br>
> In <a href="https://reviews.llvm.org/D32614#740119" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32614#740119</a>, @hiraditya wrote:<br>
><br>
> > In <a href="https://reviews.llvm.org/D32614#739947" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32614#739947</a>, @dberlin wrote:<br>
> ><br>
> > > I'm at a loss to understand why you believe you need to compute joint post-dominance (which is what this is) to compute ANTIC.<br>
> ><br>
> ><br>
> > If the set of BasicBlocks (WL) do not joint-post-dominate the hoisting point (HoistBB), then the expression to be hoisted (from WL) cannot be ANTIC in HoistBB.<br>
><br>
><br>
> Sure.<br>
>  That is definitely a way of computing it.<br>
>  It is a generally slow and unused way of computing it, because there are only certain points in the SSA graph where ANTIC can actually change.<br>
><br>
> > It is a necessary condition what I'm checking here.<br>
><br>
> It is not necessary to perform graph reachability to do this.<br>
><br>
> <a href="https://pdfs.semanticscholar.org/6d0f/07ff330402b46e83d46142e202069d9aeceb.pdf" rel="noreferrer" target="_blank">https://pdfs.semanticscholar.<wbr>org/6d0f/<wbr>07ff330402b46e83d46142e202069d<wbr>9aeceb.pdf</a><br>
><br>
> Stare at the down-safety step.<br>
>  With a few bits, it is only actually necessary to compute and check antic at the possible phis you would insert to do your hoisting.<br>
<br>
<br>
</span>From the paper and the book (<a href="http://ssabook.gforge.inria.fr/latest/book.pdf" rel="noreferrer" target="_blank">http://ssabook.gforge.inria.<wbr>fr/latest/book.pdf</a> page: 151), it seems DownSafety algorithm does exactly what I'm doing in the function (anticReachable), IIUC.<br>
<br></blockquote><div><br></div><div>It does so only by walking the actual redundancy graph, actually.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
DownSafety:<br>
...<br>
10: for each f ∈ {Φ’s in the program} do<br>
11:   if ∃ path P to program exit or alteration of expression along which f is not used<br>
12:      downsafe (f ) ← false<br>
...<br>
<br>
It checks for each path from point P to the program exit. They have simplified the problem by assuming that end of the function is reachable from every node which is not the<br>
case with LLVM representation of CFG (and that caused the bug).<br></blockquote><div>They have exactly the same representation we do, actually (i've looked at the code :P)</div><div>;)</div><div> </div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
If I implement the ANTIC the way they have done, it would require iterating through all dominance frontiers to figure out PHI insertion points.<br></blockquote><div> <br>Errr, you already need to know the insertion points to do hoistnig/sinking anyway!</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Also, just ensuring downSafety at the PHI is not sufficient to guarantee downsafety at HoistPt because there might be safety issues between a definition of instruction and its dominance frontier.<br>
<br></blockquote><div><br></div><div>They mark this in the redundancy graph.</div><div><br></div><div>My point in all this is that repeatedly redoing computation over all possible expressions to be hoisted, instead of computing some sort of factored graph of relevant points like they do, means you are computing this repeatedly and expensively.</div><div><br></div><div><br></div></div></div></div>