[PATCH] D9151: Loop Versioning for LICM

[Adam Nemet via llvm-commits]

anemet added inline comments.

================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:555-575
@@ +554,23 @@
+  MDNode *NewScope = MDB.createAnonymousAliasScope(NewDomain, Name);
+  // Iterate over each instruction of loop.
+  // set no-alias for all load & store instructions.
+  for (auto *Block : CurLoop->getBlocks()) {
+    for (auto &Inst : *Block) {
+      // Only interested in instruction that may modify or read memory.
+      if (!Inst.mayReadFromMemory() && !Inst.mayWriteToMemory())
+        continue;
+      Scopes.push_back(NewScope);
+      NoAliases.push_back(NewScope);
+      // Set no-alias for current instruction.
+      Inst.setMetadata(
+          LLVMContext::MD_noalias,
+          MDNode::concatenate(Inst.getMetadata(LLVMContext::MD_noalias),
+                              MDNode::get(Inst.getContext(), NoAliases)));
+      // set alias-scope for current instruction.
+      Inst.setMetadata(
+          LLVMContext::MD_alias_scope,
+          MDNode::concatenate(Inst.getMetadata(LLVMContext::MD_alias_scope),
+                              MDNode::get(Inst.getContext(), Scopes)));
+    }
+  }
+}
----------------
You probably want to add a comment saying that you can add no-alias between any pairs of memory operations because you ignore loops with must aliasing accesses.  Otherwise I don't think this would be valid.

================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:622-623
@@ +621,4 @@
+    setNoAliasToLoop(LVer.getVersionedLoop());
+    // Update Dominator tree with new changes.
+    DT->recalculate(*CurLoop->getHeader()->getParent());
+    Changed = true;
----------------
Why is this necessary?  LoopVersioning is supposed to update the DT.


Repository:
  rL LLVM

http://reviews.llvm.org/D9151