<div dir="rtl"><div dir="ltr">Maybe move init into constructor?</div><div dir="ltr"><br></div><div dir="ltr"><div dir="ltr"> Hint *Hints[3];</div><div>...</div></div><div dir="ltr"><div dir="ltr"> LoopVectorizeHints(const Loop *L, bool DisableUnrolling)</div>
<div dir="ltr"> : Width("vectorize.width", VectorizationFactor, HK_WIDTH),</div><div dir="ltr"> Unroll("interleave.count", DisableUnrolling, HK_UNROLL),</div><div dir="ltr"> Force("vectorize.enable", FK_Undefined, HK_FORCE),</div>
<div dir="ltr"> TheLoop(L) {</div><div dir="ltr"> Hints[0] = &Width;</div><div dir="ltr"> Hints[1] = &Unroll;</div><div dir="ltr"> Hints[2] = &Force;</div><div dir="ltr"> // Populate values with existing loop metadata.</div>
<div dir="ltr"> getHintsFromMetadata();</div><div><br></div></div></div><div class="gmail_extra"><div dir="ltr"><br><br><div class="gmail_quote">2014-08-19 20:55 GMT+03:00 Yaron Keren <span dir="ltr"><<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0 .8ex;border-left:1px #ccc solid;border-right:1px #ccc solid;padding-left:1ex;padding-right:1ex"><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><div><div class="h5"><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" target="_blank">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></div></div><br></div>
</blockquote></div></div></div>