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

[silviu.baranga@arm.com via llvm-commits]

sbaranga added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombinePHI.cpp:969-983
@@ -965,1 +968,17 @@
+    } else {
+      // This is a PHI of PHIs. See if any one of the incoming values
+      // can be proven to be the equal with this PHI.
+      for (InValNo = 0; InValNo != NumIncomingVals; ++InValNo) {
+        PHINode *Val = cast<PHINode>(PN.getIncomingValue(InValNo));
+
+        // No need to check values that don't dominate the PHI.
+        if (!DT->dominates(Val, &PN))
+          continue;
+
+        // Scan to see if we can prove that this is a phi cycle equal to
+        // Val.
+        SmallPtrSet<PHINode*, 16> ValueEqualPHIs;
+        if (PHIsEqualValue(&PN, Val, ValueEqualPHIs))
+          return replaceInstUsesWith(PN, Val);
+      }
     }
----------------
majnemer wrote:
> sbaranga wrote:
> > majnemer wrote:
> > > Seeing as how you are not inventing a new instruction, couldn't this live in InstructionSimplify?
> > I suppose we could. Would we also move the logic that tries to find an incoming value that is not a PHI?
> > 
> > In that case this would get triggered a lot more often (and might have an impact on compile time).
> > 
> We already have similar logic in InstructionSimplify, this would make it more powerful. [1]
> 
> Would you mind benchmarking this between trunk vs trunk w/ InstructionSimplify amended? 
> 
> [1] https://github.com/llvm-mirror/llvm/blob/master/lib/Analysis/InstructionSimplify.cpp#L3684
Sure! I'll post the data once I have it.

Cheers,
Silviu


http://reviews.llvm.org/D20174