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

Alina Sbirlea via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 8 17:03:33 PDT 2016


asbirlea added inline comments.

================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:357
@@ -348,2 +356,3 @@
     }
   }
+
----------------
jlebar wrote:
> 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.
Let me try to reason this.
The loop checks that all operands IM should dominate IW. If that's not the case, IW should be moved before I. If that happens, IW is added to the worklist, so all its operands are checked and possibly moved before as well in the next iterations.
Yes, all IM should implicitly dominate I as well, but that should be transitive through IW. Unless I missed something?


http://reviews.llvm.org/D22071





More information about the llvm-commits mailing list