[PATCH] D8688: Update MergedLoadStoreMotion to use MemorySSA

George Burgess IV via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 22 13:16:49 PDT 2016


george.burgess.iv added a comment.

Drive-by comments about comments (insert "we need to go deeper" meme here)


================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:791
@@ +790,3 @@
+  // Walk all accesses in first block, put them into a hash table.
+  // We can't just use a norm
+  for (auto &Access : *Succ0Accesses) {
----------------
Potentially unfinished thought?

================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:797
@@ +796,3 @@
+    Instruction *I = MU->getMemoryInst();
+    // Only move non-simple (atomic, volatile) loads.
+    LoadInst *Load0 = dyn_cast<LoadInst>(I);
----------------
Did you mean "only move simple (non-atomic, non-volatile)"?


http://reviews.llvm.org/D8688





More information about the llvm-commits mailing list