<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Aug 24, 2016 at 8:17 PM, Sebastian Pop <span dir="ltr"><<a href="mailto:sebpop@gmail.com" target="_blank">sebpop@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">sebpop added inline comments.<br>
<span class=""><br>
================<br>
Comment at: llvm/lib/Transforms/Scalar/<wbr>GVNHoist.cpp:315<br>
@@ -303,1 +314,3 @@<br>
+        return false;<br>
+<br>
       // Increment DFS traversal when not skipping children.<br>
----------------<br>
</span><span class="">dberlin wrote:<br>
> How is this different than just checking whether one of the successors is a loop backedge?<br>
> (and if so, why not just compute and store that set once :P)<br>
><br>
> If it's different, how is this not basically a dominance frontier check?<br>
><br>
> dominance frontier of BB = blocks where BB's dominance ends.<br>
><br>
> So this is a check if any of the blocks are in the dominance frontier of BB, no?<br>
><br>
><br>
</span>I think checking for loop latch is in part correct: there may be irreducible loops that would not be in LoopInfo and there may still exit backedges that would violate the property of execution on all paths.<br></blockquote><div><br></div><div><br>Okay.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Yes, the check can be rewritten as whether a successor of a block in the dominance frontier of BB dominates BB.  So we would not have to compute this property for all blocks dominated by BB, only for the blocks on the dominance frontier.<br>
<br></blockquote><div><br>See if it's worth it. Storing the DF is N^2, even though it's linear time to compute, so if doing it your current way is faster, let's go with that.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I will update the patch.  Thanks for the suggestion.<br>
<br>
<br>
<br>
<a href="https://reviews.llvm.org/D23843" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D23843</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>