From: Michael Kruse Date: Mon, 11 Feb 2019 19:45:44 +0000 (+0000) Subject: Refactor setAlreadyUnrolled() and setAlreadyVectorized(). X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0bb9e7af427e7977ffa3f8cdd51ffb5049128c71;p=llvm Refactor setAlreadyUnrolled() and setAlreadyVectorized(). Loop::setAlreadyUnrolled() and LoopVectorizeHints::setLoopAlreadyUnrolled() both add loop metadata that stops the same loop from being transformed multiple times. This patch merges both implementations. In doing so we fix 3 potential issues: * setLoopAlreadyUnrolled() kept the llvm.loop.vectorize/interleave.* metadata even though it will not be used anymore. This already caused problems such as http://llvm.org/PR40546. Change the behavior to the one of setAlreadyUnrolled which deletes this loop metadata. * setAlreadyUnrolled() used to create a new LoopID by calling MDNode::get with nullptr as the first operand, then replacing it by the returned references using replaceOperandWith. It is possible that MDNode::get would instead return an existing node (due to de-duplication) that then gets modified. To avoid, use a fresh TempMDNode that does not get uniqued with anything else before replacing it with replaceOperandWith. * LoopVectorizeHints::matchesHintMetadataName() only compares the suffix of the attribute to set the new value for. That is, when called with "enable", would erase attributes such as "llvm.loop.unroll.enable", "llvm.loop.vectorize.enable" and "llvm.loop.distribute.enable" instead of the one to replace. Fortunately, function was only called with "isvectorized". Differential Revision: https://reviews.llvm.org/D57566 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@353738 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/Analysis/LoopInfo.h b/include/llvm/Analysis/LoopInfo.h index cb66e0dba74..0899630fe8e 100644 --- a/include/llvm/Analysis/LoopInfo.h +++ b/include/llvm/Analysis/LoopInfo.h @@ -1014,6 +1014,26 @@ MDNode *findOptionMDForLoop(const Loop *TheLoop, StringRef Name); /// is representing an access group. bool isValidAsAccessGroup(MDNode *AccGroup); +/// Create a new LoopID after the loop has been transformed. +/// +/// This can be used when no follow-up loop attributes are defined +/// (llvm::makeFollowupLoopID returning None) to stop transformations to be +/// applied again. +/// +/// @param Context The LLVMContext in which to create the new LoopID. +/// @param OrigLoopID The original LoopID; can be nullptr if the original +/// loop has no LoopID. +/// @param RemovePrefixes Remove all loop attributes that have these prefixes. +/// Use to remove metadata of the transformation that has +/// been applied. +/// @param AddAttrs Add these loop attributes to the new LoopID. +/// +/// @return A new LoopID that can be applied using Loop::setLoopID(). +llvm::MDNode * +makePostTransformationMetadata(llvm::LLVMContext &Context, MDNode *OrigLoopID, + llvm::ArrayRef RemovePrefixes, + llvm::ArrayRef AddAttrs); + } // End llvm namespace #endif diff --git a/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h b/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h index 90f0da9a551..ea0a1c2654b 100644 --- a/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h +++ b/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h @@ -98,11 +98,7 @@ public: OptimizationRemarkEmitter &ORE); /// Mark the loop L as already vectorized by setting the width to 1. - void setAlreadyVectorized() { - IsVectorized.Value = 1; - Hint Hints[] = {IsVectorized}; - writeHintsToMetadata(Hints); - } + void setAlreadyVectorized(); bool allowVectorization(Function *F, Loop *L, bool VectorizeOnlyWhenForced) const; @@ -151,15 +147,6 @@ private: /// Checks string hint with one operand and set value if valid. void setHint(StringRef Name, Metadata *Arg); - /// Create a new hint from name / value pair. - MDNode *createHintMetadata(StringRef Name, unsigned V) const; - - /// Matches metadata with hint name. - bool matchesHintMetadataName(MDNode *Node, ArrayRef HintTypes); - - /// Sets current hints into loop metadata, keeping other values intact. - void writeHintsToMetadata(ArrayRef HintTypes); - /// The loop these hints belong to. const Loop *TheLoop; diff --git a/lib/Analysis/LoopInfo.cpp b/lib/Analysis/LoopInfo.cpp index b7b06bdb054..9518c6e551c 100644 --- a/lib/Analysis/LoopInfo.cpp +++ b/lib/Analysis/LoopInfo.cpp @@ -254,35 +254,13 @@ void Loop::setLoopID(MDNode *LoopID) const { } void Loop::setLoopAlreadyUnrolled() { - MDNode *LoopID = getLoopID(); - // First remove any existing loop unrolling metadata. - SmallVector MDs; - // Reserve first location for self reference to the LoopID metadata node. - MDs.push_back(nullptr); - - if (LoopID) { - for (unsigned i = 1, ie = LoopID->getNumOperands(); i < ie; ++i) { - bool IsUnrollMetadata = false; - MDNode *MD = dyn_cast(LoopID->getOperand(i)); - if (MD) { - const MDString *S = dyn_cast(MD->getOperand(0)); - IsUnrollMetadata = S && S->getString().startswith("llvm.loop.unroll."); - } - if (!IsUnrollMetadata) - MDs.push_back(LoopID->getOperand(i)); - } - } - - // Add unroll(disable) metadata to disable future unrolling. LLVMContext &Context = getHeader()->getContext(); - SmallVector DisableOperands; - DisableOperands.push_back(MDString::get(Context, "llvm.loop.unroll.disable")); - MDNode *DisableNode = MDNode::get(Context, DisableOperands); - MDs.push_back(DisableNode); - MDNode *NewLoopID = MDNode::get(Context, MDs); - // Set operand 0 to refer to the loop id itself. - NewLoopID->replaceOperandWith(0, NewLoopID); + MDNode *DisableUnrollMD = + MDNode::get(Context, MDString::get(Context, "llvm.loop.unroll.disable")); + MDNode *LoopID = getLoopID(); + MDNode *NewLoopID = makePostTransformationMetadata( + Context, LoopID, {"llvm.loop.unroll."}, {DisableUnrollMD}); setLoopID(NewLoopID); } @@ -760,6 +738,46 @@ bool llvm::isValidAsAccessGroup(MDNode *Node) { return Node->getNumOperands() == 0 && Node->isDistinct(); } +MDNode *llvm::makePostTransformationMetadata(LLVMContext &Context, + MDNode *OrigLoopID, + ArrayRef RemovePrefixes, + ArrayRef AddAttrs) { + // First remove any existing loop metadata related to this transformation. + SmallVector MDs; + + // Reserve first location for self reference to the LoopID metadata node. + TempMDTuple TempNode = MDNode::getTemporary(Context, None); + MDs.push_back(TempNode.get()); + + // Remove metadata for the transformation that has been applied or that became + // outdated. + if (OrigLoopID) { + for (unsigned i = 1, ie = OrigLoopID->getNumOperands(); i < ie; ++i) { + bool IsVectorMetadata = false; + Metadata *Op = OrigLoopID->getOperand(i); + if (MDNode *MD = dyn_cast(Op)) { + const MDString *S = dyn_cast(MD->getOperand(0)); + if (S) + IsVectorMetadata = + llvm::any_of(RemovePrefixes, [S](StringRef Prefix) -> bool { + return S->getString().startswith(Prefix); + }); + } + if (!IsVectorMetadata) + MDs.push_back(Op); + } + } + + // Add metadata to avoid reapplying a transformation, such as + // llvm.loop.unroll.disable and llvm.loop.isvectorized. + MDs.append(AddAttrs.begin(), AddAttrs.end()); + + MDNode *NewLoopID = MDNode::getDistinct(Context, MDs); + // Replace the temporary node with a self-reference. + NewLoopID->replaceOperandWith(0, NewLoopID); + return NewLoopID; +} + //===----------------------------------------------------------------------===// // LoopInfo implementation // diff --git a/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp b/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp index 60aaa315a3c..1d030944815 100644 --- a/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp +++ b/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp @@ -104,6 +104,25 @@ LoopVectorizeHints::LoopVectorizeHints(const Loop *L, << "LV: Interleaving disabled by the pass manager\n"); } +void LoopVectorizeHints::setAlreadyVectorized() { + LLVMContext &Context = TheLoop->getHeader()->getContext(); + + MDNode *IsVectorizedMD = MDNode::get( + Context, + {MDString::get(Context, "llvm.loop.isvectorized"), + ConstantAsMetadata::get(ConstantInt::get(Context, APInt(32, 1)))}); + MDNode *LoopID = TheLoop->getLoopID(); + MDNode *NewLoopID = + makePostTransformationMetadata(Context, LoopID, + {Twine(Prefix(), "vectorize.").str(), + Twine(Prefix(), "interleave.").str()}, + {IsVectorizedMD}); + TheLoop->setLoopID(NewLoopID); + + // Update internal cache. + IsVectorized.Value = 1; +} + bool LoopVectorizeHints::allowVectorization( Function *F, Loop *L, bool VectorizeOnlyWhenForced) const { if (getForce() == LoopVectorizeHints::FK_Disabled) { @@ -231,57 +250,6 @@ void LoopVectorizeHints::setHint(StringRef Name, Metadata *Arg) { } } -MDNode *LoopVectorizeHints::createHintMetadata(StringRef Name, - unsigned V) const { - LLVMContext &Context = TheLoop->getHeader()->getContext(); - Metadata *MDs[] = { - MDString::get(Context, Name), - ConstantAsMetadata::get(ConstantInt::get(Type::getInt32Ty(Context), V))}; - return MDNode::get(Context, MDs); -} - -bool LoopVectorizeHints::matchesHintMetadataName(MDNode *Node, - ArrayRef HintTypes) { - MDString *Name = dyn_cast(Node->getOperand(0)); - if (!Name) - return false; - - for (auto H : HintTypes) - if (Name->getString().endswith(H.Name)) - return true; - return false; -} - -void LoopVectorizeHints::writeHintsToMetadata(ArrayRef HintTypes) { - if (HintTypes.empty()) - return; - - // Reserve the first element to LoopID (see below). - SmallVector MDs(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(LoopID->getOperand(i)); - // If node in update list, ignore old value. - if (!matchesHintMetadataName(Node, HintTypes)) - MDs.push_back(Node); - } - } - - // Now, add the missing hints. - for (auto H : HintTypes) - MDs.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, MDs); - // Set operand 0 to refer to the loop id itself. - NewLoopID->replaceOperandWith(0, NewLoopID); - - TheLoop->setLoopID(NewLoopID); -} - bool LoopVectorizationRequirements::doesNotMeet( Function *F, Loop *L, const LoopVectorizeHints &Hints) { const char *PassName = Hints.vectorizeAnalysisPassName(); diff --git a/test/Transforms/LoopVectorize/X86/vect.omp.force.small-tc.ll b/test/Transforms/LoopVectorize/X86/vect.omp.force.small-tc.ll index dc6b8f0ec35..7c249a1b422 100644 --- a/test/Transforms/LoopVectorize/X86/vect.omp.force.small-tc.ll +++ b/test/Transforms/LoopVectorize/X86/vect.omp.force.small-tc.ll @@ -58,7 +58,7 @@ define void @vectorized(float* noalias nocapture %A, float* noalias nocapture re ; CHECK-NEXT: store float [[ADD]], float* [[ARRAYIDX2]], align 4, !llvm.access.group !0 ; CHECK-NEXT: [[INDVARS_IV_NEXT]] = add nuw nsw i64 [[INDVARS_IV]], 1 ; CHECK-NEXT: [[EXITCOND:%.*]] = icmp eq i64 [[INDVARS_IV_NEXT]], 20 -; CHECK-NEXT: br i1 [[EXITCOND]], label [[FOR_END]], label [[FOR_BODY]], !llvm.loop !5 +; CHECK-NEXT: br i1 [[EXITCOND]], label [[FOR_END]], label [[FOR_BODY]], !llvm.loop !4 ; CHECK: for.end: ; CHECK-NEXT: ret void ; @@ -115,7 +115,7 @@ define void @vectorized1(float* noalias nocapture %A, float* noalias nocapture r ; CHECK-NEXT: call void @llvm.masked.store.v8f32.p0v8f32(<8 x float> [[TMP7]], <8 x float>* [[TMP9]], i32 4, <8 x i1> [[TMP8]]) ; CHECK-NEXT: [[INDEX_NEXT]] = add i64 [[INDEX]], 8 ; CHECK-NEXT: [[TMP10:%.*]] = icmp eq i64 [[INDEX_NEXT]], 24 -; CHECK-NEXT: br i1 [[TMP10]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop !8 +; CHECK-NEXT: br i1 [[TMP10]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop !7 ; CHECK: middle.block: ; CHECK-NEXT: br i1 true, label [[FOR_END:%.*]], label [[SCALAR_PH]] ; CHECK: scalar.ph: @@ -173,7 +173,7 @@ define void @vectorized2(float* noalias nocapture %A, float* noalias nocapture r ; CHECK-NEXT: store <8 x float> [[TMP7]], <8 x float>* [[TMP8]], align 4 ; CHECK-NEXT: [[INDEX_NEXT]] = add i64 [[INDEX]], 8 ; CHECK-NEXT: [[TMP9:%.*]] = icmp eq i64 [[INDEX_NEXT]], 16 -; CHECK-NEXT: br i1 [[TMP9]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop !11 +; CHECK-NEXT: br i1 [[TMP9]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop !10 ; CHECK: middle.block: ; CHECK-NEXT: [[CMP_N:%.*]] = icmp eq i64 16, 16 ; CHECK-NEXT: br i1 [[CMP_N]], label [[FOR_END:%.*]], label [[SCALAR_PH]] @@ -183,14 +183,14 @@ define void @vectorized2(float* noalias nocapture %A, float* noalias nocapture r ; CHECK: for.body: ; CHECK-NEXT: [[INDVARS_IV:%.*]] = phi i64 [ [[BC_RESUME_VAL]], [[SCALAR_PH]] ], [ [[INDVARS_IV_NEXT:%.*]], [[FOR_BODY]] ] ; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds float, float* [[B]], i64 [[INDVARS_IV]] -; CHECK-NEXT: [[TMP10:%.*]] = load float, float* [[ARRAYIDX]], align 4, !llvm.access.group !7 +; CHECK-NEXT: [[TMP10:%.*]] = load float, float* [[ARRAYIDX]], align 4, !llvm.access.group !6 ; CHECK-NEXT: [[ARRAYIDX2:%.*]] = getelementptr inbounds float, float* [[A]], i64 [[INDVARS_IV]] -; CHECK-NEXT: [[TMP11:%.*]] = load float, float* [[ARRAYIDX2]], align 4, !llvm.access.group !7 +; CHECK-NEXT: [[TMP11:%.*]] = load float, float* [[ARRAYIDX2]], align 4, !llvm.access.group !6 ; CHECK-NEXT: [[ADD:%.*]] = fadd fast float [[TMP10]], [[TMP11]] -; CHECK-NEXT: store float [[ADD]], float* [[ARRAYIDX2]], align 4, !llvm.access.group !7 +; CHECK-NEXT: store float [[ADD]], float* [[ARRAYIDX2]], align 4, !llvm.access.group !6 ; CHECK-NEXT: [[INDVARS_IV_NEXT]] = add nuw nsw i64 [[INDVARS_IV]], 1 ; CHECK-NEXT: [[EXITCOND:%.*]] = icmp eq i64 [[INDVARS_IV_NEXT]], 16 -; CHECK-NEXT: br i1 [[EXITCOND]], label [[FOR_END]], label [[FOR_BODY]], !llvm.loop !12 +; CHECK-NEXT: br i1 [[EXITCOND]], label [[FOR_END]], label [[FOR_BODY]], !llvm.loop !11 ; CHECK: for.end: ; CHECK-NEXT: ret void ; diff --git a/test/Transforms/LoopVectorize/remove_metadata.ll b/test/Transforms/LoopVectorize/remove_metadata.ll new file mode 100644 index 00000000000..793c134dd7d --- /dev/null +++ b/test/Transforms/LoopVectorize/remove_metadata.ll @@ -0,0 +1,32 @@ +; RUN: opt -loop-vectorize -force-vector-interleave=1 -force-vector-width=2 -S < %s | FileCheck %s +; +; Check that llvm.loop.vectorize.* metadata is removed after vectorization. +; +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128" + +; CHECK-LABEL: @disable_nonforced_enable( +; CHECK: store <2 x i32> +define void @disable_nonforced_enable(i32* nocapture %a, i32 %n) { +entry: + %cmp4 = icmp sgt i32 %n, 0 + br i1 %cmp4, label %for.body, label %for.end + +for.body: + %indvars.iv = phi i64 [ %indvars.iv.next, %for.body ], [ 0, %entry ] + %arrayidx = getelementptr inbounds i32, i32* %a, i64 %indvars.iv + %0 = trunc i64 %indvars.iv to i32 + store i32 %0, i32* %arrayidx, align 4 + %indvars.iv.next = add i64 %indvars.iv, 1 + %lftr.wideiv = trunc i64 %indvars.iv.next to i32 + %exitcond = icmp eq i32 %lftr.wideiv, %n + br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !0 + +for.end: + ret void +} + +!0 = !{!0, !{!"llvm.loop.vectorize.some_property"}, !{!"llvm.loop.vectorize.enable", i32 1}} + +; CHECK-NOT: llvm.loop.vectorize. +; CHECK: {!"llvm.loop.isvectorized", i32 1} +; CHECK-NOT: llvm.loop.vectorize.