[PATCH] D17191: [LoopVectorize] Annotate versioned loop with noalias metadata

[Michael Zolotukhin via llvm-commits]

mzolotukhin added a comment.

Hi Adam,

Please find some comments inline.

Thanks,
Michael


================
Comment at: include/llvm/Transforms/Utils/LoopVersioning.h:92-96
@@ +91,7 @@
+
+  /// \brief Add the noalias annotations to \p VersionedInst.
+  ////
+  ///\p OrigInst is the instruction corresponding to \p VersionedInst in the
+  /// original loop.  Initialize the aliasing scopes with
+  /// prepareNoAliasMetadata once before this can be called.
+  void annotateInstWithNoAlias(Instruction *VersionedInst,
----------------
Formatting looks weird here.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:449-455
@@ -447,2 +448,9 @@
 
+  /// \brief Propagate known metadata from one instruction to another.
+  void propagateMetadata(Instruction *To, const Instruction *From);
+
+  /// \brief Propagate known metadata from one instruction to a vector of
+  /// others.
+  void propagateMetadata(SmallVectorImpl<Value *> &To, const Instruction *From);
+
   /// This is a helper class that holds the vectorizer state. It maps scalar
----------------
These functions are generally useful, not only in LoopVectorizer - e.g. we have their duplicates in SLPVectorizer. While you're at it, could you please move them out to a common place (and commit as a separate change)?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:659-663
@@ -642,2 +658,7 @@
   }
+
+  // If the loop was versioned with memchecks, add the corresponding no-alias
+  // metadata.
+  if (LVer && (isa<LoadInst>(From) || isa<StoreInst>(From)))
+    LVer->annotateInstWithNoAlias(To, From);
 }
----------------
Does it belong here? I.e. should it really be a part of `propagateMetadata`? Probably that's fine with the current usage of `propagateMetadata` but it might become surprising if one decides to use this function somewhere else.


http://reviews.llvm.org/D17191