[PATCH] D8688: Update MergedLoadStoreMotion to use MemorySSA

Geoff Berry via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 16 11:52:29 PDT 2016


gberry added a comment.

A couple of minor comments I noticed when reading through this for my own edification:


================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:653
@@ -555,1 +652,3 @@
 
+// \brief DFS Number instructions in blocks in dominator order, tracking
+void MergedLoadStoreMotion::computeBarriers() {
----------------
It seems like you forgot to finish this sentence.

================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:1016
@@ +1015,3 @@
+  // will place in the sink location block.
+  // After handling the above, we create a new MemoryDefwith either the phi
+  // node, or the correct version, in the sink location block.
----------------
typo: "MemoryDefwith"

================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:1028
@@ +1027,3 @@
+    // This may create a phi node that has the same versions for everything
+    // after replaceALlUsesWith
+    MayCreateDegeneratePhi = true;
----------------
typo: "replaceALlUsesWith"


http://reviews.llvm.org/D8688





More information about the llvm-commits mailing list