<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 20 August 2015 at 19:31, Daniel Berlin via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">dberlin added a comment.<br>
<br>
I assume you are hitting this in switch statements or something similar.  I triggered the same in NewGVN.<br>
<br>
Note that the comment in dominates is simply wrong.<br>
I discussed with this Rafael (who added this). The callers can, in fact, do *nothing* different than what the function would have to do, and in fact, would have to duplicate a ton of complicated critical edge logic that Dominates handles.  That seems really bad to me.<br>
<br>
Nowhere else in LLVM do we *assert* because a very dumb caller may cause a performance problem.<br>
The right fix here is to  simply remove the assert, and make sure your caller is smart about not calling this unnecessarily.<br></blockquote><div><br></div><div>I'm not sure I follow.</div><div><br></div><div>The point of the function that's already there is to query "does this *edge* dominate that block"? When the edge is non-singular, it certainly doesn't (you can take the other edge between the same two blocks) and we would need to return false. The comment is correct on that point. If you removed the assert, you would need to replace it with "if (!isSingular()) return false;".</div><div><br></div><div>Which is not what we want as a caller. We want to query "does this block dominate that block" therefore we don't care about singular edges at all, we don't care if there's two edges from this block to that one. We want a different interface because we want to ask a different question.</div><div><br></div><div>Removing the assert may be the right thing to do, but it doesn't save us from needing this patch.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">(IE cache the value of edge domination in cases like this).<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
<a href="http://reviews.llvm.org/D12170" rel="noreferrer" target="_blank">http://reviews.llvm.org/D12170</a><br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>