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

[Adam Nemet via llvm-commits]

anemet marked an inline comment as done.

================
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
----------------
mzolotukhin wrote:
> 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)?
They are not really duplicates.  The one in LV copies from instruction to one/many.  The one in SLP copies many to one and merges them in the process in a metadata-specific way.

You can obviously still refactor the common parts or create a superset but I think that will probably be harder to read at the end.

What do you think?

================
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);
 }
----------------
mzolotukhin wrote:
> 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.
Well, that depends on what you respond to the above but semantically, I do think it belongs here.

Here we're propagating the metadata from the "versioned" scalar loop (which is never created) to the vector loop.  Let me know if this is not clear and I should add a comment.


http://reviews.llvm.org/D17191