[llvm] r215999 - Revert "Small refactor on VectorizerHint for deduplication"
Renato Golin
renato.golin at linaro.org
Mon Sep 1 03:10:26 PDT 2014
Thanks Seth,
I've incorporated your recommendation into r216870. Let me know if
that breaks anything.
cheers,
--renato
On 20 August 2014 03:08, Seth Cantrell <seth.cantrell at gmail.com> wrote:
> Here's the fix that worked for me on VS2012:
>
> diff --git a/lib/Transforms/Vectorize/LoopVectorize.cpp b/lib/Transforms/Vectorize/LoopVectorize.cpp
> index e0d8e93..67fbc44 100644
> --- a/lib/Transforms/Vectorize/LoopVectorize.cpp
> +++ b/lib/Transforms/Vectorize/LoopVectorize.cpp
> @@ -1013,7 +1013,7 @@ class LoopVectorizeHints {
> /// Vectorization forced
> Hint Force;
> /// Array to help iterating through all hints.
> - Hint *Hints[3] = { &Width, &Unroll, &Force };
> + Hint *Hints[3];
>
> /// Return the loop metadata prefix.
> static StringRef Prefix() { return "llvm.loop."; }
> @@ -1030,6 +1030,8 @@ public:
> Unroll("interleave.count", DisableUnrolling, HK_UNROLL),
> Force("vectorize.enable", FK_Undefined, HK_FORCE),
> TheLoop(L) {
> + Hints[0] = &Width; Hints[1] = &Unroll; Hints[2] = &Force;
> +
> // Populate values with existing loop metadata.
> getHintsFromMetadata();
>
> @@ -1044,7 +1046,11 @@ public:
> /// Mark the loop L as already vectorized by setting the width to 1.
> void setAlreadyVectorized() {
> Width.Value = Unroll.Value = 1;
> - writeHintsToMetadata({ Width, Unroll });
> + std::vector<Hint> hints;
> + hints.reserve(2);
> + hints.emplace_back(Width);
> + hints.emplace_back(Unroll);
> + writeHintsToMetadata(std::move(hints));
> }
>
> /// Dumps all the hint information.
>
>
>
>
>> Author: rengolin
>> Date: Tue Aug 19 13:08:50 2014
>> New Revision: 215999
>>
>> URL:
>> http://llvm.org/viewvc/llvm-project?rev=215999&view=rev
>>
>> Log:
>> Revert "Small refactor on VectorizerHint for deduplication"
>>
>> This reverts commit r215994 because MSVC 2012 can't cope with its C++11 goodness.
>>
>> Removed:
>> llvm/trunk/test/Transforms/LoopVectorize/duplicated-metadata.ll
>> Modified:
>> llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp?rev=215999&r1=215998&r2=215999&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp Tue Aug 19 13:08:50 2014
>> @@ -970,54 +970,7 @@ private:
>>
>> /// Utility class for getting and setting loop vectorizer hints in the form
>> /// of loop metadata.
>> -/// This class keeps a number of loop annotations locally (as member variables)
>> -/// and can, upon request, write them back as metadata on the loop. It will
>> -/// initially scan the loop for existing metadata, and will update the local
>> -/// values based on information in the loop.
>> -/// We cannot write all values to metadata, as the mere presence of some info,
>> -/// for example 'force', means a decision has been made. So, we need to be
>> -/// careful NOT to add them if the user hasn't specifically asked so.
>> class LoopVectorizeHints {
>> - enum HintKind {
>> - HK_WIDTH,
>> - HK_UNROLL,
>> - HK_FORCE
>> - };
>> -
>> - /// Hint - associates name and validation with the hint value.
>> - struct Hint {
>> - const char * Name;
>> - unsigned Value; // This may have to change for non-numeric values.
>> - HintKind Kind;
>> -
>> - Hint(const char * Name, unsigned Value, HintKind Kind)
>> - : Name(Name), Value(Value), Kind(Kind) { }
>> -
>> - bool validate(unsigned Val) {
>> - switch (Kind) {
>> - case HK_WIDTH:
>> - return isPowerOf2_32(Val) && Val <= MaxVectorWidth;
>> - case HK_UNROLL:
>> - return isPowerOf2_32(Val) && Val <= MaxUnrollFactor;
>> - case HK_FORCE:
>> - return (Val <= 1);
>> - }
>> - return false;
>> - }
>> - };
>> -
>> - /// Vectorization width.
>> - Hint Width;
>> - /// Vectorization unroll factor.
>> - Hint Unroll;
>> - /// Vectorization forced
>> - Hint Force;
>> - /// Array to help iterating through all hints.
>> - Hint *Hints[3] = { &Width, &Unroll, &Force };
>> -
>> - /// Return the loop metadata prefix.
>> - static StringRef Prefix() { return "llvm.loop."; }
>> -
>> public:
>> enum ForceKind {
>> FK_Undefined = -1, ///< Not selected.
>> @@ -1026,40 +979,70 @@ public:
>> };
>>
>> LoopVectorizeHints(const Loop *L, bool DisableUnrolling)
>> - : Width("vectorize.width", VectorizationFactor, HK_WIDTH),
>> - Unroll("interleave.count", DisableUnrolling, HK_UNROLL),
>> - Force("vectorize.enable", FK_Undefined, HK_FORCE),
>> - TheLoop(L) {
>> - // Populate values with existing loop metadata.
>> - getHintsFromMetadata();
>> -
>> + : Width(VectorizationFactor),
>> + Unroll(DisableUnrolling),
>> + Force(FK_Undefined),
>> + LoopID(L->getLoopID()) {
>> + getHints(L);
>> // force-vector-unroll overrides DisableUnrolling.
>> if (VectorizationUnroll.getNumOccurrences() > 0)
>> - Unroll.Value = VectorizationUnroll;
>> + Unroll = VectorizationUnroll;
>>
>> - DEBUG(if (DisableUnrolling && Unroll.Value == 1) dbgs()
>> + DEBUG(if (DisableUnrolling && Unroll == 1) dbgs()
>> << "LV: Unrolling disabled by the pass manager\n");
>> }
>>
>> + /// Return the loop metadata prefix.
>> + static StringRef Prefix() { return "llvm.loop."; }
>> +
>> + MDNode *createHint(LLVMContext &Context, StringRef Name, unsigned V) const {
>> + SmallVector<Value*, 2> Vals;
>> + Vals.push_back(MDString::get(Context, Name));
>> + Vals.push_back(ConstantInt::get(Type::getInt32Ty(Context), V));
>> + return MDNode::get(Context, Vals);
>> + }
>> +
>> /// Mark the loop L as already vectorized by setting the width to 1.
>> - void setAlreadyVectorized() {
>> - Width.Value = Unroll.Value = 1;
>> - writeHintsToMetadata({ Width, Unroll });
>> + void setAlreadyVectorized(Loop *L) {
>> + LLVMContext &Context = L->getHeader()->getContext();
>> +
>> + Width = 1;
>> +
>> + // Create a new loop id with one more operand for the already_vectorized
>> + // hint. If the loop already has a loop id then copy the existing operands.
>> + SmallVector<Value*, 4> Vals(1);
>> + if (LoopID)
>> + for (unsigned i = 1, ie = LoopID->getNumOperands(); i < ie; ++i)
>> + Vals.push_back(LoopID->getOperand(i));
>> +
>> + Vals.push_back(
>> + createHint(Context, Twine(Prefix(), "vectorize.width").str(), Width));
>> + Vals.push_back(
>> + createHint(Context, Twine(Prefix(), "interleave.count").str(), 1));
>> +
>> + MDNode *NewLoopID = MDNode::get(Context, Vals);
>> + // Set operand 0 to refer to the loop id itself.
>> + NewLoopID->replaceOperandWith(0, NewLoopID);
>> +
>> + L->setLoopID(NewLoopID);
>> + if (LoopID)
>> + LoopID->replaceAllUsesWith(NewLoopID);
>> +
>> + LoopID = NewLoopID;
>> }
>>
>> - /// Dumps all the hint information.
>> std::string emitRemark() const {
>> Report R;
>> - if (Force.Value == LoopVectorizeHints::FK_Disabled)
>> + if (Force == LoopVectorizeHints::FK_Disabled)
>> R << "vectorization is explicitly disabled";
>> else {
>> R << "use -Rpass-analysis=loop-vectorize for more info";
>> - if (Force.Value == LoopVectorizeHints::FK_Enabled) {
>> + if (Force == LoopVectorizeHints::FK_Enabled) {
>> R << " (Force=true";
>> - if (Width.Value != 0)
>> - R << ", Vector Width=" << Width.Value;
>> - if (Unroll.Value != 0)
>> - R << ", Interleave Count=" << Unroll.Value;
>> + if (Width != 0)
>> + R << ", Vector Width=" << Width;
>> + if (Unroll != 0)
>> + R << ", Interleave Count=" << Unroll;
>> R << ")";
>> }
>> }
>> @@ -1067,14 +1050,14 @@ public:
>> return R.str();
>> }
>>
>> - unsigned getWidth() const { return Width.Value; }
>> - unsigned getUnroll() const { return Unroll.Value; }
>> - enum ForceKind getForce() const { return (ForceKind)Force.Value; }
>> + unsigned getWidth() const { return Width; }
>> + unsigned getUnroll() const { return Unroll; }
>> + enum ForceKind getForce() const { return Force; }
>> + MDNode *getLoopID() const { return LoopID; }
>>
>> private:
>> - /// Find hints specified in the loop metadata and update local values.
>> - void getHintsFromMetadata() {
>> - MDNode *LoopID = TheLoop->getLoopID();
>> + /// Find hints specified in the loop metadata.
>> + void getHints(const Loop *L) {
>> if (!LoopID)
>> return;
>>
>> @@ -1103,91 +1086,52 @@ private:
>> continue;
>>
>> // Check if the hint starts with the loop metadata prefix.
>> - StringRef Name = S->getString();
>> + StringRef Hint = S->getString();
>> + if (!Hint.startswith(Prefix()))
>> + continue;
>> + // Remove the prefix.
>> + Hint = Hint.substr(Prefix().size(), StringRef::npos);
>> +
>> if (Args.size() == 1)
>> - setHint(Name, Args[0]);
>> + getHint(Hint, Args[0]);
>> }
>> }
>>
>> - /// Checks string hint with one operand and set value if valid.
>> - void setHint(StringRef Name, Value *Arg) {
>> - if (!Name.startswith(Prefix()))
>> - return;
>> - Name = Name.substr(Prefix().size(), StringRef::npos);
>> -
>> + // Check string hint with one operand.
>> + void getHint(StringRef Hint, Value *Arg) {
>> const ConstantInt *C = dyn_cast<ConstantInt>(Arg);
>> if (!C) return;
>> unsigned Val = C->getZExtValue();
>>
>> - for (auto H : Hints) {
>> - if (Name == H->Name) {
>> - if (H->validate(Val))
>> - H->Value = Val;
>> - else
>> - DEBUG(dbgs() << "LV: ignoring invalid hint '" << Name << "'\n");
>> - break;
>> - }
>> + if (Hint == "vectorize.width") {
>> + if (isPowerOf2_32(Val) && Val <= MaxVectorWidth)
>> + Width = Val;
>> + else
>> + DEBUG(dbgs() << "LV: ignoring invalid width hint metadata\n");
>> + } else if (Hint == "vectorize.enable") {
>> + if (C->getBitWidth() == 1)
>> + Force = Val == 1 ? LoopVectorizeHints::FK_Enabled
>> + : LoopVectorizeHints::FK_Disabled;
>> + else
>> + DEBUG(dbgs() << "LV: ignoring invalid enable hint metadata\n");
>> + } else if (Hint == "interleave.count") {
>> + if (isPowerOf2_32(Val) && Val <= MaxUnrollFactor)
>> + Unroll = Val;
>> + else
>> + DEBUG(dbgs() << "LV: ignoring invalid unroll hint metadata\n");
>> + } else {
>> + DEBUG(dbgs() << "LV: ignoring unknown hint " << Hint << '\n');
>> }
>> }
>>
>> - /// Create a new hint from name / value pair.
>> - MDNode *createHintMetadata(StringRef Name, unsigned V) const {
>> - LLVMContext &Context = TheLoop->getHeader()->getContext();
>> - SmallVector<Value*, 2> Vals;
>> - Vals.push_back(MDString::get(Context, Name));
>> - Vals.push_back(ConstantInt::get(Type::getInt32Ty(Context), V));
>> - return MDNode::get(Context, Vals);
>> - }
>> -
>> - /// Matches metadata with hint name.
>> - bool matchesHintMetadataName(MDNode *Node, std::vector<Hint> &HintTypes) {
>> - MDString* Name = dyn_cast<MDString>(Node->getOperand(0));
>> - if (!Name)
>> - return false;
>> -
>> - for (auto H : HintTypes)
>> - if (Name->getName().endswith(H.Name))
>> - return true;
>> - return false;
>> - }
>> -
>> - /// Sets current hints into loop metadata, keeping other values intact.
>> - void writeHintsToMetadata(std::vector<Hint> HintTypes) {
>> - if (HintTypes.size() == 0)
>> - return;
>> -
>> - // Reserve the first element to LoopID (see below).
>> - SmallVector<Value*, 4> Vals(1);
>> - // If the loop already has metadata, then ignore the existing operands.
>> - MDNode *LoopID = TheLoop->getLoopID();
>> - if (LoopID) {
>> - for (unsigned i = 1, ie = LoopID->getNumOperands(); i < ie; ++i) {
>> - MDNode *Node = cast<MDNode>(LoopID->getOperand(i));
>> - // If node in update list, ignore old value.
>> - if (!matchesHintMetadataName(Node, HintTypes))
>> - Vals.push_back(Node);
>> - }
>> - }
>> -
>> - // Now, add the missing hints.
>> - for (auto H : HintTypes)
>> - Vals.push_back(
>> - createHintMetadata(Twine(Prefix(), H.Name).str(), H.Value));
>> -
>> - // Replace current metadata node with new one.
>> - LLVMContext &Context = TheLoop->getHeader()->getContext();
>> - MDNode *NewLoopID = MDNode::get(Context, Vals);
>> - // Set operand 0 to refer to the loop id itself.
>> - NewLoopID->replaceOperandWith(0, NewLoopID);
>> -
>> - TheLoop->setLoopID(NewLoopID);
>> - if (LoopID)
>> - LoopID->replaceAllUsesWith(NewLoopID);
>> - LoopID = NewLoopID;
>> - }
>> + /// Vectorization width.
>> + unsigned Width;
>> + /// Vectorization unroll factor.
>> + unsigned Unroll;
>> + /// Vectorization forced
>> + enum ForceKind Force;
>>
>> - /// The loop these hints belong to.
>> - const Loop *TheLoop;
>> + MDNode *LoopID;
>> };
>>
>> static void emitMissedWarning(Function *F, Loop *L,
>> @@ -1449,7 +1393,7 @@ struct LoopVectorize : public FunctionPa
>> }
>>
>> // Mark the loop as already vectorized to avoid vectorizing again.
>> - Hints.setAlreadyVectorized();
>> + Hints.setAlreadyVectorized(L);
>>
>> DEBUG(verifyFunction(*L->getHeader()->getParent()));
>> return true;
>> @@ -2551,7 +2495,7 @@ void InnerLoopVectorizer::createEmptyLoo
>> LoopScalarBody = OldBasicBlock;
>>
>> LoopVectorizeHints Hints(Lp, true);
>> - Hints.setAlreadyVectorized();
>> + Hints.setAlreadyVectorized(Lp);
>> }
>>
>> /// This function returns the identity element (or neutral element) for
>>
>> Removed: llvm/trunk/test/Transforms/LoopVectorize/duplicated-metadata.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopVectorize/duplicated-metadata.ll?rev=215998&view=auto
>>
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/LoopVectorize/duplicated-metadata.ll (original)
>> +++ llvm/trunk/test/Transforms/LoopVectorize/duplicated-metadata.ll (removed)
>> @@ -1,30 +0,0 @@
>> -; RUN: opt < %s -loop-vectorize -S 2>&1 | FileCheck %s
>> -target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
>> -target triple = "x86_64-unknown-linux-gnu"
>> -
>> -; This test makes sure we don't duplicate the loop vectorizer's metadata
>> -; while marking them as already vectorized (by setting width = 1), even
>> -; at lower optimization levels, where no extra cleanup is done
>> -
>> -define void @_Z3fooPf(float* %a) {
>> -entry:
>> - br label %for.body
>> -
>> -for.body: ; preds = %for.body, %entry
>> - %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
>> - %arrayidx = getelementptr inbounds float* %a, i64 %indvars.iv
>> - %p = load float* %arrayidx, align 4
>> - %mul = fmul float %p, 2.000000e+00
>> - store float %mul, float* %arrayidx, align 4
>> - %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
>> - %exitcond = icmp eq i64 %indvars.iv.next, 1024
>> - br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !0
>> -
>> -for.end: ; preds = %for.body
>> - ret void
>> -}
>> -
>> -!0 = metadata !{metadata !0, metadata !1}
>> -!1 = metadata !{metadata !"llvm.loop.vectorize.width", i32 4}
>> -; CHECK-NOT: !{metadata !"llvm.loop.vectorize.width", i32 4}
>> -; CHECK: !{metadata !"llvm.loop.interleave.count", i32 1}
>>
>>
More information about the llvm-commits
mailing list