[PATCH] D22071: Correct ordering of loads/stores.

[Justin Lebar via llvm-commits]

jlebar added inline comments.

================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:354
@@ +353,3 @@
+        DEBUG(assert(IM->getParent() == IW->getParent() &&
+                     "Instructions to move should be in the same basic block"));
+      }
----------------
Don't need DEBUG around the assert().  assert() is a macro and only evaluates its args in debug builds.

================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:357
@@ -348,2 +356,3 @@
     }
   }
+
----------------
One of the things I'm bad at is evaluating the correctness of code that contains nits to be fixed.  So...now that you've fixed the nits, I have a correctness concern, which I'm sorry I didn't see earlier.

My concern is that we stop reordering when IM dominates IW.  But it seems to me that we should stop reordering when IM dominates I, no?  Because after we move IW up before I, it may no longer dominate its operands.


http://reviews.llvm.org/D22071