[PATCH] [SROA] Simplify PHI nodes before promoting the alloca

[Chandler Carruth]

================
Comment at: lib/Transforms/Scalar/SROA.cpp:3097-3102
@@ -3096,1 +3096,8 @@
 
+/// \brief Return true if the value of PN originates from a value
+/// other than Source.
+///
+/// This function performs a standard depth-first search along the use-def
+/// chains of the PHI nodes. If the searching reaches a non-PHI other than
+/// Source, it stops and returns true.
+static bool
----------------
I think this is the wrong approach in several ways.

Depth first search when there is an early exit shortcut is usually more expensive than testing the predicate breadth first while adding the nodes to a worklist. Also you should use a worklist rather than recursion or you would easily blow out the stack. Finally, I agree that we shouldn't need a set for originating from other and just a visit....

But before we dive into the details to fix these issues -- why is this the right approach? Walking *up* from the PHI operands seems necessarily much more work than walking *down* from the alloca. In fact, we're already going to walk down from the alloca because we iterate on every non-promoted alloca. Why can't we fold no-op PHI nodes in the partition builder the same way we do no-op selects? I think that would solve the problem you have here with no extra use-list traversal.

For future reference, keep in mind that uselist traversal is a cache-hostile thing. We should minimize the number of traversals and merge traversals where possible.

================
Comment at: lib/Transforms/Scalar/SROA.cpp:3102
@@ +3101,3 @@
+/// chains of the PHI nodes. If the searching reaches a non-PHI other than
+/// Source, it stops and returns true.
+static bool
----------------
Eli Bendersky wrote:
> Please document all the arguments of this function
Wo don't actually insist on that usually. I'd rather the arguments have descriptive names than comments personally.

http://reviews.llvm.org/D4659