[PATCH] D6995: Reassociate: reprocess RedoInsts after each instruction

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:2265
@@ +2264,3 @@
+  // processed, so we first perform a RPOT of the basic blocks so that
+  // when we process a basic block, all its dominators have ben processed
+  // before.
----------------
ben -> been ?

================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:2301-2302
@@ +2300,4 @@
+        // since these will be processed later.
+        if (I != II && (I->getParent() != BI || !Worklist.count(I)) &&
+            RankMap[I->getParent()] <= RankMap[BI]) {
+          if (isInstructionTriviallyDead(I))
----------------
Is it possible for `I` to equal `II` ?  Removing this check doesn't seem to make any tests fail.

================
Comment at: test/Transforms/Reassociate/prev_insts_canonicalized.ll:1
@@ +1,2 @@
+; RUN: opt -reassociate -o - -S %s | FileCheck %s
+
----------------
I'd follow the pattern used by the other tests:
  ; RUN: opt < %s -reassociate -S | FileCheck %s

This will ensure that the test's filename will not show up in the test's output.


http://reviews.llvm.org/D6995