[PATCH] D12992: invariant.group handling in GVN

[Richard Smith via llvm-commits]

rsmith added inline comments.

================
Comment at: include/llvm/Analysis/MemoryDependenceAnalysis.h:420
@@ +419,3 @@
+
+    /// getIncariantGroupPointerDependency - This analysis looks for other
+    /// loads and stores with invariant.group metadata and the same
----------------
Typo "Incariant"; use \brief insteaed of repeating the function name here.

================
Comment at: include/llvm/Analysis/MemoryDependenceAnalysis.h:422
@@ +421,3 @@
+    /// loads and stores with invariant.group metadata and the same
+    /// pointer operand. Returns Unknown if will not find anything, and Def
+    /// if it can be assumed that 2 instructions loads or stores the same value.
----------------
will -> does

================
Comment at: include/llvm/Analysis/MemoryDependenceAnalysis.h:423
@@ +422,3 @@
+    /// pointer operand. Returns Unknown if will not find anything, and Def
+    /// if it can be assumed that 2 instructions loads or stores the same value.
+    MemDepResult getInvariantGroupPointerDependency(LoadInst *LI);
----------------
loads or stores -> load or store

================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:379
@@ -378,2 +378,3 @@
 /// annotated to the query instruction to refine the result.
+
 MemDepResult MemoryDependenceAnalysis::getPointerDependencyFrom(
----------------
Revert this blank line insertion.

================
Comment at: lib/Transforms/Utils/Local.cpp:1340
@@ -1335,1 +1339,3 @@
   }
+  // Set !invariant.group from J if J has it. If both instruction has it
+  // then we will just pick it from J - even when they are different.
----------------
instruction has -> instructions have

================
Comment at: lib/Transforms/Utils/Local.cpp:1341
@@ +1340,3 @@
+  // Set !invariant.group from J if J has it. If both instruction has it
+  // then we will just pick it from J - even when they are different.
+  if (auto *JMD = J->getMetadata(LLVMContext::MD_invariant_group))
----------------
Maybe add a FIXME stating that we should try to preserve both groups in this case.


http://reviews.llvm.org/D12992