[PATCH] Simplify n-ary adds by reassociation

[Sanjoy Das]

Minor comments inline.


================
Comment at: lib/Transforms/Scalar/NaryReassociate.cpp:73
@@ +72,3 @@
+using namespace llvm;
+using namespace PatternMatch;
+
----------------
Since you're using `PatternMatch` in only one function, I think the namespace should be opened in just that scope.

================
Comment at: lib/Transforms/Scalar/NaryReassociate.cpp:140
@@ +139,3 @@
+  SeenExprs.clear();
+  for (auto node = GraphTraits<DominatorTree *>::nodes_begin(DT);
+       node != GraphTraits<DominatorTree *>::nodes_end(DT); ++node) {
----------------
I think LLVM convention is `Node`.  Also, I'd suggest using `auto *Node` to make it obvious that we have a pointer here (very minor).

================
Comment at: lib/Transforms/Scalar/NaryReassociate.cpp:199
@@ +198,3 @@
+       ++LHS, ++NumIterations) {
+    if (DT->dominates(*LHS, I)) {
+      Instruction *NewI = BinaryOperator::CreateAdd(*LHS, RHS, "", I);
----------------
Is it possible to switch to a traversal and expression gathering scheme that will always visit defs before uses?  Reverse post-order maybe?  I think that way we can avoid the `dominates` query (which can be expensive) and just turn it into an `assert`; and just use the earliest (latest?) element in `LHSCandidates`.

http://reviews.llvm.org/D8950

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/