[PATCH] D20174: [InstCombine] Allow removal of PHI cycles which only contain PHIs

silviu.baranga@arm.com via llvm-commits llvm-commits at lists.llvm.org
Thu May 12 05:45:33 PDT 2016


sbaranga added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombinePHI.cpp:972
@@ +971,3 @@
+      InValNo = 0;
+      while (InValNo != NumIncomingVals) {
+        Value *Val = PN.getIncomingValue(InValNo);
----------------
reames wrote:
> Repeating this for each incoming value seems likely to be expensive.  Can we do better?  As one filter, we know that for the phi to be equal to some value, that value must dominate the phi or be a non-instruction value right?  If so, can we use that?  
> 
> Another option on how to approach this would be an inductive proof over the loop.  On iteration 1, we know that %p2 = %p0 and thus that on every future iteration all inputes to the phis equal %p0.  Framing this using a loop pass (possibly indvarsimplify?) might be another approach.  
> 
> To be clear, I'm okay with the patch in it's current form if we can't find a better approach.  I'm just asking to make sure we've explored all the options before eating the compile time.
Correct, this might be a bit expensive when triggered. But the problem only occurs when all incoming values are phis and the phi that we processing has lots of incoming values. This should mean that the really expensive wouldn't trigger that often.

Filtering with DT sounds like a very good idea. Do you think that this transformation should only be done when DT is available, or should we only filter when DT is available?

I think this case can be triggered without needing the a loop, but I don't have an example for that.




http://reviews.llvm.org/D20174





More information about the llvm-commits mailing list