<div dir="rtl"><div dir="ltr">Hi Renato,</div><div dir="ltr"><br></div><div dir="ltr">Visual C++ 2013 does not like:</div><div dir="ltr"><br></div><div dir="ltr"><div dir="ltr">1>------ Build started: Project: LLVMVectorize, Configuration: RelWithDebInfo Win32 ------</div>

<div dir="ltr">1>  LoopVectorize.cpp</div><div dir="ltr">1>..\..\..\..\lib\Transforms\Vectorize\LoopVectorize.cpp(1016): error C2536: '`anonymous-namespace'::LoopVectorizeHints::`anonymous-namespace'::LoopVectorizeHints::Hints' : cannot specify explicit initializer for arrays</div>

<div dir="ltr">1>          ..\..\..\..\lib\Transforms\Vectorize\LoopVectorize.cpp(1016) : see declaration of '`anonymous-namespace'::LoopVectorizeHints::Hints'</div><div dir="ltr">1>          ..\..\..\..\lib\Transforms\Vectorize\LoopVectorize.cpp(1016) : see declaration of '`anonymous-namespace'::LoopVectorizeHints::Hints'</div>

<div dir="ltr">========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========</div><div><br></div><div>Yaron<br></div><div><br></div></div></div><div class="gmail_extra"><br><br><div class="gmail_quote"><div dir="ltr">

2014-08-19 20:30 GMT+03:00 Renato Golin <span dir="ltr"><<a href="mailto:renato.golin@linaro.org" target="_blank">renato.golin@linaro.org</a>></span>:</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

Author: rengolin<br>
Date: Tue Aug 19 12:30:43 2014<br>
New Revision: 215994<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=215994&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=215994&view=rev</a><br>
Log:<br>
Small refactor on VectorizerHint for deduplication<br>
<br>
Previously, the hint mechanism relied on clean up passes to remove redundant<br>
metadata, which still showed up if running opt at low levels of optimization.<br>
That also has shown that multiple nodes of the same type, but with different<br>
values could still coexist, even if temporary, and cause confusion if the<br>
next pass got the wrong value.<br>
<br>
This patch makes sure that, if metadata already exists in a loop, the hint<br>
mechanism will never append a new node, but always replace the existing one.<br>
It also enhances the algorithm to cope with more metadata types in the future<br>
by just adding a new type, not a lot of code.<br>
<br>
Added:<br>
    llvm/trunk/test/Transforms/LoopVectorize/duplicated-metadata.ll<br>
Modified:<br>
    llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp?rev=215994&r1=215993&r2=215994&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp?rev=215994&r1=215993&r2=215994&view=diff</a><br>


==============================================================================<br>
--- llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp Tue Aug 19 12:30:43 2014<br>
@@ -970,7 +970,54 @@ private:<br>
<br>
 /// Utility class for getting and setting loop vectorizer hints in the form<br>
 /// of loop metadata.<br>
+/// This class keeps a number of loop annotations locally (as member variables)<br>
+/// and can, upon request, write them back as metadata on the loop. It will<br>
+/// initially scan the loop for existing metadata, and will update the local<br>
+/// values based on information in the loop.<br>
+/// We cannot write all values to metadata, as the mere presence of some info,<br>
+/// for example 'force', means a decision has been made. So, we need to be<br>
+/// careful NOT to add them if the user hasn't specifically asked so.<br>
 class LoopVectorizeHints {<br>
+  enum HintKind {<br>
+    HK_WIDTH,<br>
+    HK_UNROLL,<br>
+    HK_FORCE<br>
+  };<br>
+<br>
+  /// Hint - associates name and validation with the hint value.<br>
+  struct Hint {<br>
+    const char * Name;<br>
+    unsigned Value; // This may have to change for non-numeric values.<br>
+    HintKind Kind;<br>
+<br>
+    Hint(const char * Name, unsigned Value, HintKind Kind)<br>
+      : Name(Name), Value(Value), Kind(Kind) { }<br>
+<br>
+    bool validate(unsigned Val) {<br>
+      switch (Kind) {<br>
+      case HK_WIDTH:<br>
+        return isPowerOf2_32(Val) && Val <= MaxVectorWidth;<br>
+      case HK_UNROLL:<br>
+        return isPowerOf2_32(Val) && Val <= MaxUnrollFactor;<br>
+      case HK_FORCE:<br>
+        return (Val <= 1);<br>
+      }<br>
+      return false;<br>
+    }<br>
+  };<br>
+<br>
+  /// Vectorization width.<br>
+  Hint Width;<br>
+  /// Vectorization unroll factor.<br>
+  Hint Unroll;<br>
+  /// Vectorization forced<br>
+  Hint Force;<br>
+  /// Array to help iterating through all hints.<br>
+  Hint *Hints[3] = { &Width, &Unroll, &Force };<br>
+<br>
+  /// Return the loop metadata prefix.<br>
+  static StringRef Prefix() { return "llvm.loop."; }<br>
+<br>
 public:<br>
   enum ForceKind {<br>
     FK_Undefined = -1, ///< Not selected.<br>
@@ -979,70 +1026,40 @@ public:<br>
   };<br>
<br>
   LoopVectorizeHints(const Loop *L, bool DisableUnrolling)<br>
-      : Width(VectorizationFactor),<br>
-        Unroll(DisableUnrolling),<br>
-        Force(FK_Undefined),<br>
-        LoopID(L->getLoopID()) {<br>
-    getHints(L);<br>
+      : Width("vectorize.width", VectorizationFactor, HK_WIDTH),<br>
+        Unroll("interleave.count", DisableUnrolling, HK_UNROLL),<br>
+        Force("vectorize.enable", FK_Undefined, HK_FORCE),<br>
+        TheLoop(L) {<br>
+    // Populate values with existing loop metadata.<br>
+    getHintsFromMetadata();<br>
+<br>
     // force-vector-unroll overrides DisableUnrolling.<br>
     if (VectorizationUnroll.getNumOccurrences() > 0)<br>
-      Unroll = VectorizationUnroll;<br>
+      Unroll.Value = VectorizationUnroll;<br>
<br>
-    DEBUG(if (DisableUnrolling && Unroll == 1) dbgs()<br>
+    DEBUG(if (DisableUnrolling && Unroll.Value == 1) dbgs()<br>
           << "LV: Unrolling disabled by the pass manager\n");<br>
   }<br>
<br>
-  /// Return the loop metadata prefix.<br>
-  static StringRef Prefix() { return "llvm.loop."; }<br>
-<br>
-  MDNode *createHint(LLVMContext &Context, StringRef Name, unsigned V) const {<br>
-    SmallVector<Value*, 2> Vals;<br>
-    Vals.push_back(MDString::get(Context, Name));<br>
-    Vals.push_back(ConstantInt::get(Type::getInt32Ty(Context), V));<br>
-    return MDNode::get(Context, Vals);<br>
-  }<br>
-<br>
   /// Mark the loop L as already vectorized by setting the width to 1.<br>
-  void setAlreadyVectorized(Loop *L) {<br>
-    LLVMContext &Context = L->getHeader()->getContext();<br>
-<br>
-    Width = 1;<br>
-<br>
-    // Create a new loop id with one more operand for the already_vectorized<br>
-    // hint. If the loop already has a loop id then copy the existing operands.<br>
-    SmallVector<Value*, 4> Vals(1);<br>
-    if (LoopID)<br>
-      for (unsigned i = 1, ie = LoopID->getNumOperands(); i < ie; ++i)<br>
-        Vals.push_back(LoopID->getOperand(i));<br>
-<br>
-    Vals.push_back(<br>
-        createHint(Context, Twine(Prefix(), "vectorize.width").str(), Width));<br>
-    Vals.push_back(<br>
-        createHint(Context, Twine(Prefix(), "interleave.count").str(), 1));<br>
-<br>
-    MDNode *NewLoopID = MDNode::get(Context, Vals);<br>
-    // Set operand 0 to refer to the loop id itself.<br>
-    NewLoopID->replaceOperandWith(0, NewLoopID);<br>
-<br>
-    L->setLoopID(NewLoopID);<br>
-    if (LoopID)<br>
-      LoopID->replaceAllUsesWith(NewLoopID);<br>
-<br>
-    LoopID = NewLoopID;<br>
+  void setAlreadyVectorized() {<br>
+    Width.Value = Unroll.Value = 1;<br>
+    writeHintsToMetadata({ Width, Unroll });<br>
   }<br>
<br>
+  /// Dumps all the hint information.<br>
   std::string emitRemark() const {<br>
     Report R;<br>
-    if (Force == LoopVectorizeHints::FK_Disabled)<br>
+    if (Force.Value == LoopVectorizeHints::FK_Disabled)<br>
       R << "vectorization is explicitly disabled";<br>
     else {<br>
       R << "use -Rpass-analysis=loop-vectorize for more info";<br>
-      if (Force == LoopVectorizeHints::FK_Enabled) {<br>
+      if (Force.Value == LoopVectorizeHints::FK_Enabled) {<br>
         R << " (Force=true";<br>
-        if (Width != 0)<br>
-          R << ", Vector Width=" << Width;<br>
-        if (Unroll != 0)<br>
-          R << ", Interleave Count=" << Unroll;<br>
+        if (Width.Value != 0)<br>
+          R << ", Vector Width=" << Width.Value;<br>
+        if (Unroll.Value != 0)<br>
+          R << ", Interleave Count=" << Unroll.Value;<br>
         R << ")";<br>
       }<br>
     }<br>
@@ -1050,14 +1067,14 @@ public:<br>
     return R.str();<br>
   }<br>
<br>
-  unsigned getWidth() const { return Width; }<br>
-  unsigned getUnroll() const { return Unroll; }<br>
-  enum ForceKind getForce() const { return Force; }<br>
-  MDNode *getLoopID() const { return LoopID; }<br>
+  unsigned getWidth() const { return Width.Value; }<br>
+  unsigned getUnroll() const { return Unroll.Value; }<br>
+  enum ForceKind getForce() const { return (ForceKind)Force.Value; }<br>
<br>
 private:<br>
-  /// Find hints specified in the loop metadata.<br>
-  void getHints(const Loop *L) {<br>
+  /// Find hints specified in the loop metadata and update local values.<br>
+  void getHintsFromMetadata() {<br>
+    MDNode *LoopID = TheLoop->getLoopID();<br>
     if (!LoopID)<br>
       return;<br>
<br>
@@ -1086,52 +1103,91 @@ private:<br>
         continue;<br>
<br>
       // Check if the hint starts with the loop metadata prefix.<br>
-      StringRef Hint = S->getString();<br>
-      if (!Hint.startswith(Prefix()))<br>
-        continue;<br>
-      // Remove the prefix.<br>
-      Hint = Hint.substr(Prefix().size(), StringRef::npos);<br>
-<br>
+      StringRef Name = S->getString();<br>
       if (Args.size() == 1)<br>
-        getHint(Hint, Args[0]);<br>
+        setHint(Name, Args[0]);<br>
     }<br>
   }<br>
<br>
-  // Check string hint with one operand.<br>
-  void getHint(StringRef Hint, Value *Arg) {<br>
+  /// Checks string hint with one operand and set value if valid.<br>
+  void setHint(StringRef Name, Value *Arg) {<br>
+    if (!Name.startswith(Prefix()))<br>
+      return;<br>
+    Name = Name.substr(Prefix().size(), StringRef::npos);<br>
+<br>
     const ConstantInt *C = dyn_cast<ConstantInt>(Arg);<br>
     if (!C) return;<br>
     unsigned Val = C->getZExtValue();<br>
<br>
-    if (Hint == "vectorize.width") {<br>
-      if (isPowerOf2_32(Val) && Val <= MaxVectorWidth)<br>
-        Width = Val;<br>
-      else<br>
-        DEBUG(dbgs() << "LV: ignoring invalid width hint metadata\n");<br>
-    } else if (Hint == "vectorize.enable") {<br>
-      if (C->getBitWidth() == 1)<br>
-        Force = Val == 1 ? LoopVectorizeHints::FK_Enabled<br>
-                         : LoopVectorizeHints::FK_Disabled;<br>
-      else<br>
-        DEBUG(dbgs() << "LV: ignoring invalid enable hint metadata\n");<br>
-    } else if (Hint == "interleave.count") {<br>
-      if (isPowerOf2_32(Val) && Val <= MaxUnrollFactor)<br>
-        Unroll = Val;<br>
-      else<br>
-        DEBUG(dbgs() << "LV: ignoring invalid unroll hint metadata\n");<br>
-    } else {<br>
-      DEBUG(dbgs() << "LV: ignoring unknown hint " << Hint << '\n');<br>
+    for (auto H : Hints) {<br>
+      if (Name == H->Name) {<br>
+        if (H->validate(Val))<br>
+          H->Value = Val;<br>
+        else<br>
+          DEBUG(dbgs() << "LV: ignoring invalid hint '" << Name << "'\n");<br>
+        break;<br>
+      }<br>
     }<br>
   }<br>
<br>
-  /// Vectorization width.<br>
-  unsigned Width;<br>
-  /// Vectorization unroll factor.<br>
-  unsigned Unroll;<br>
-  /// Vectorization forced<br>
-  enum ForceKind Force;<br>
+  /// Create a new hint from name / value pair.<br>
+  MDNode *createHintMetadata(StringRef Name, unsigned V) const {<br>
+    LLVMContext &Context = TheLoop->getHeader()->getContext();<br>
+    SmallVector<Value*, 2> Vals;<br>
+    Vals.push_back(MDString::get(Context, Name));<br>
+    Vals.push_back(ConstantInt::get(Type::getInt32Ty(Context), V));<br>
+    return MDNode::get(Context, Vals);<br>
+  }<br>
+<br>
+  /// Matches metadata with hint name.<br>
+  bool matchesHintMetadataName(MDNode *Node, std::vector<Hint> &HintTypes) {<br>
+    MDString* Name = dyn_cast<MDString>(Node->getOperand(0));<br>
+    if (!Name)<br>
+      return false;<br>
+<br>
+    for (auto H : HintTypes)<br>
+      if (Name->getName().endswith(H.Name))<br>
+        return true;<br>
+    return false;<br>
+  }<br>
+<br>
+  /// Sets current hints into loop metadata, keeping other values intact.<br>
+  void writeHintsToMetadata(std::vector<Hint> HintTypes) {<br>
+    if (HintTypes.size() == 0)<br>
+      return;<br>
+<br>
+    // Reserve the first element to LoopID (see below).<br>
+    SmallVector<Value*, 4> Vals(1);<br>
+    // If the loop already has metadata, then ignore the existing operands.<br>
+    MDNode *LoopID = TheLoop->getLoopID();<br>
+    if (LoopID) {<br>
+      for (unsigned i = 1, ie = LoopID->getNumOperands(); i < ie; ++i) {<br>
+        MDNode *Node = cast<MDNode>(LoopID->getOperand(i));<br>
+        // If node in update list, ignore old value.<br>
+        if (!matchesHintMetadataName(Node, HintTypes))<br>
+          Vals.push_back(Node);<br>
+      }<br>
+    }<br>
+<br>
+    // Now, add the missing hints.<br>
+    for (auto H : HintTypes)<br>
+      Vals.push_back(<br>
+          createHintMetadata(Twine(Prefix(), H.Name).str(), H.Value));<br>
+<br>
+    // Replace current metadata node with new one.<br>
+    LLVMContext &Context = TheLoop->getHeader()->getContext();<br>
+    MDNode *NewLoopID = MDNode::get(Context, Vals);<br>
+    // Set operand 0 to refer to the loop id itself.<br>
+    NewLoopID->replaceOperandWith(0, NewLoopID);<br>
+<br>
+    TheLoop->setLoopID(NewLoopID);<br>
+    if (LoopID)<br>
+      LoopID->replaceAllUsesWith(NewLoopID);<br>
+    LoopID = NewLoopID;<br>
+  }<br>
<br>
-  MDNode *LoopID;<br>
+  /// The loop these hints belong to.<br>
+  const Loop *TheLoop;<br>
 };<br>
<br>
 static void emitMissedWarning(Function *F, Loop *L,<br>
@@ -1393,7 +1449,7 @@ struct LoopVectorize : public FunctionPa<br>
     }<br>
<br>
     // Mark the loop as already vectorized to avoid vectorizing again.<br>
-    Hints.setAlreadyVectorized(L);<br>
+    Hints.setAlreadyVectorized();<br>
<br>
     DEBUG(verifyFunction(*L->getHeader()->getParent()));<br>
     return true;<br>
@@ -2495,7 +2551,7 @@ void InnerLoopVectorizer::createEmptyLoo<br>
   LoopScalarBody = OldBasicBlock;<br>
<br>
   LoopVectorizeHints Hints(Lp, true);<br>
-  Hints.setAlreadyVectorized(Lp);<br>
+  Hints.setAlreadyVectorized();<br>
 }<br>
<br>
 /// This function returns the identity element (or neutral element) for<br>
<br>
Added: llvm/trunk/test/Transforms/LoopVectorize/duplicated-metadata.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopVectorize/duplicated-metadata.ll?rev=215994&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopVectorize/duplicated-metadata.ll?rev=215994&view=auto</a><br>


==============================================================================<br>
--- llvm/trunk/test/Transforms/LoopVectorize/duplicated-metadata.ll (added)<br>
+++ llvm/trunk/test/Transforms/LoopVectorize/duplicated-metadata.ll Tue Aug 19 12:30:43 2014<br>
@@ -0,0 +1,30 @@<br>
+; RUN: opt < %s -loop-vectorize -S 2>&1 | FileCheck %s<br>
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"<br>
+target triple = "x86_64-unknown-linux-gnu"<br>
+<br>
+; This test makes sure we don't duplicate the loop vectorizer's metadata<br>
+; while marking them as already vectorized (by setting width = 1), even<br>
+; at lower optimization levels, where no extra cleanup is done<br>
+<br>
+define void @_Z3fooPf(float* %a) {<br>
+entry:<br>
+  br label %for.body<br>
+<br>
+for.body:                                         ; preds = %for.body, %entry<br>
+  %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]<br>
+  %arrayidx = getelementptr inbounds float* %a, i64 %indvars.iv<br>
+  %p = load float* %arrayidx, align 4<br>
+  %mul = fmul float %p, 2.000000e+00<br>
+  store float %mul, float* %arrayidx, align 4<br>
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1<br>
+  %exitcond = icmp eq i64 %indvars.iv.next, 1024<br>
+  br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !0<br>
+<br>
+for.end:                                          ; preds = %for.body<br>
+  ret void<br>
+}<br>
+<br>
+!0 = metadata !{metadata !0, metadata !1}<br>
+!1 = metadata !{metadata !"llvm.loop.vectorize.width", i32 4}<br>
+; CHECK-NOT: !{metadata !"llvm.loop.vectorize.width", i32 4}<br>
+; CHECK: !{metadata !"llvm.loop.interleave.count", i32 1}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>