<div dir="ltr"><div><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 19, 2017 at 11:00 AM, Davide Italiano <span dir="ltr"><<a href="mailto:davide@freebsd.org" target="_blank">davide@freebsd.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="">On Fri, May 19, 2017 at 10:00 AM, Sanjay Patel <<a href="mailto:spatel@rotateright.com">spatel@rotateright.com</a>> wrote:<br>
> Is "VRP" (value range propagation) in LLVM-speak "CVP" (correlated value<br>
> propagation)?<br>
><br>
> If so, we have this comment regarding compares:<br>
>   // As a policy choice, we choose not to waste compile time on anything<br>
> where<br>
>   // the comparison is testing local values.<br>
><br>
> Or this for div/rem:<br>
>   // As a policy choice, we choose not<br>
>   // to waste compile time on anything where the operands are local defs.<br>
><br>
> "Local" means in the same basic block from what I can tell by the code here.<br>
><br>
> I think this means that this pass explicitly defers handling simple cases<br>
> like:<br>
> <a href="https://reviews.llvm.org/D33342" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D33342</a><br>
> ...to another pass, and currently that pass is InstCombine (because the<br>
> patterns really can't be any simpler than the tests in that patch, right?).<br>
><br>
> I think that improving compile-time should not preclude improving<br>
> InstCombine. We should do both.<br>
><br>
<br>
</span>Just thoughts, feel free to ignore them.<br>
I didn't measure the impact in LLVM, but I'm sure you can do VRP<br>
relatively fast (GCC does that both interprocedurally and<br>
intraprocedurally and their pass is much faster in some cases than<br>
LLVM's), i.e. O(N) in practice, so, maybe we could re-evaluate this<br>
policy?<br></blockquote><div><br></div><div>Yeah, that's kind of silly, we can do much better.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I think replacing a pass in LLVM is not trivial (I'm learning it the<br>
hard way with NewGVN). OTOH, I'm still not entirely convinced<br>
`InstCombine` should handle these cases given it's already a<br>
compile-time sink?<br></blockquote><div><br></div><div><br></div><div>People seem intent on adding lots and lots of stuff to instcombine because it's easy.</div><div>This makes it harder to ever replace, while simultaneously making it slower.</div><div>It's not hard to see where that path ends up.</div><div>InstCombine does a lot of random crap that isn't even combining or graph rewrite, too, because well second and third order effects are hard.<br></div><div><br></div><div>If we want an optimal graph rewriter, we should actually just build one, with sane rewrite rules, verification that it fixpoints, and some sense of good ordering, etc,  instead of  slowly every adding possible pattern match to InstCombine.<br></div><div><br></div><div>(and then split out the crap that isn't rewriting, and if we need to understand how to get second and third order effects, let's sit down and think about it instead of the brute force approach we do now).</div><div><br></div><div>But that's just kibitzing.</div><div><br></div><div>--Dan</div></div></div></div>