!fixup address latest comments, thanks

fhahn · fhahn · commit d3edd8957422 · 2025-04-19T20:16:31.000+01:00
diff --git a/llvm/lib/Transforms/Vectorize/VPRecipeBuilder.h b/llvm/lib/Transforms/Vectorize/VPRecipeBuilder.h
@@ -236,7 +236,7 @@ class VPRecipeBuilder {
 
   /// Returns the metatadata that can be preserved from the original instruction
   /// \p I, including noalias metadata guaranteed by runtime checks.
-  SmallVector<std::pair<unsigned, MDNode *>>
+  static SmallVector<std::pair<unsigned, MDNode *>>
   getMetadataToPropagate(Instruction *I);
 };
 } // end namespace llvm
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1194,39 +1194,33 @@ using MDArrayRef = ArrayRef<std::pair<unsigned, MDNode *>>;
 
 /// Helper to manage IR metadata for recipes. It filters out metadata that
 /// cannot be proagated.
-class VPWithIRMetadata {
+class VPIRMetadata {
   SmallVector<std::pair<unsigned, MDNode *>> Metadata;
 
 protected:
-  VPWithIRMetadata() {}
-  VPWithIRMetadata(MDArrayRef Metadata) : Metadata(Metadata) {}
-
-  void addMetadata(MDArrayRef Metadata) {
-    append_range(this->Metadata, Metadata);
-  }
+  VPIRMetadata(MDArrayRef Metadata) : Metadata(Metadata) {}
 
 public:
   /// Add all metadata to \p V if it is an instruction.
-  void setMetadata(Value *V) const;
-
-  void addMetadata(unsigned Kind, MDNode *N) { Metadata.emplace_back(Kind, N); }
+  void applyMetadata(Value *V) const;
 
+  /// Return the IR metadata.
   MDArrayRef getMetadata() const { return Metadata; }
 };
 
 /// VPWidenRecipe is a recipe for producing a widened instruction using the
 /// opcode and operands of the recipe. This recipe covers most of the
 /// traditional vectorization cases where each recipe transforms into a
 /// vectorized version of itself.
-class VPWidenRecipe : public VPRecipeWithIRFlags, public VPWithIRMetadata {
+class VPWidenRecipe : public VPRecipeWithIRFlags, public VPIRMetadata {
   unsigned Opcode;
 
 protected:
   template <typename IterT>
   VPWidenRecipe(unsigned VPDefOpcode, Instruction &I,
                 iterator_range<IterT> Operands, MDArrayRef Metadata)
-      : VPRecipeWithIRFlags(VPDefOpcode, Operands, I),
-        VPWithIRMetadata(Metadata), Opcode(I.getOpcode()) {}
+      : VPRecipeWithIRFlags(VPDefOpcode, Operands, I), VPIRMetadata(Metadata),
+        Opcode(I.getOpcode()) {}
 
 public:
   template <typename IterT>
@@ -1263,7 +1257,7 @@ class VPWidenRecipe : public VPRecipeWithIRFlags, public VPWithIRMetadata {
 };
 
 /// VPWidenCastRecipe is a recipe to create vector cast instructions.
-class VPWidenCastRecipe : public VPRecipeWithIRFlags, public VPWithIRMetadata {
+class VPWidenCastRecipe : public VPRecipeWithIRFlags, public VPIRMetadata {
   /// Cast instruction opcode.
   Instruction::CastOps Opcode;
 
@@ -1274,13 +1268,13 @@ class VPWidenCastRecipe : public VPRecipeWithIRFlags, public VPWithIRMetadata {
   VPWidenCastRecipe(Instruction::CastOps Opcode, VPValue *Op, Type *ResultTy,
                     CastInst &UI, MDArrayRef Metadata)
       : VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op, UI),
-        VPWithIRMetadata(Metadata), Opcode(Opcode), ResultTy(ResultTy) {
+        VPIRMetadata(Metadata), Opcode(Opcode), ResultTy(ResultTy) {
     assert(UI.getOpcode() == Opcode &&
            "opcode of underlying cast doesn't match");
   }
 
   VPWidenCastRecipe(Instruction::CastOps Opcode, VPValue *Op, Type *ResultTy)
-      : VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op), VPWithIRMetadata(),
+      : VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op), VPIRMetadata({}),
         Opcode(Opcode), ResultTy(ResultTy) {}
 
   ~VPWidenCastRecipe() override = default;
@@ -1315,8 +1309,7 @@ class VPWidenCastRecipe : public VPRecipeWithIRFlags, public VPWithIRMetadata {
 };
 
 /// A recipe for widening vector intrinsics.
-class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags,
-                               public VPWithIRMetadata {
+class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags, public VPIRMetadata {
   /// ID of the vector intrinsic to widen.
   Intrinsic::ID VectorIntrinsicID;
 
@@ -1337,7 +1330,7 @@ class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags,
                          ArrayRef<VPValue *> CallArguments, Type *Ty,
                          MDArrayRef Metadata, DebugLoc DL = {})
       : VPRecipeWithIRFlags(VPDef::VPWidenIntrinsicSC, CallArguments, CI),
-        VPWithIRMetadata(Metadata), VectorIntrinsicID(VectorIntrinsicID),
+        VPIRMetadata(Metadata), VectorIntrinsicID(VectorIntrinsicID),
         ResultTy(Ty), MayReadFromMemory(CI.mayReadFromMemory()),
         MayWriteToMemory(CI.mayWriteToMemory()),
         MayHaveSideEffects(CI.mayHaveSideEffects()) {}
@@ -1346,7 +1339,7 @@ class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags,
                          ArrayRef<VPValue *> CallArguments, Type *Ty,
                          MDArrayRef Metadata, DebugLoc DL = {})
       : VPRecipeWithIRFlags(VPDef::VPWidenIntrinsicSC, CallArguments, DL),
-        VPWithIRMetadata(Metadata), VectorIntrinsicID(VectorIntrinsicID),
+        VPIRMetadata(Metadata), VectorIntrinsicID(VectorIntrinsicID),
         ResultTy(Ty) {
     LLVMContext &Ctx = Ty->getContext();
     AttributeSet Attrs = Intrinsic::getFnAttributes(Ctx, VectorIntrinsicID);
@@ -1403,7 +1396,7 @@ class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags,
 };
 
 /// A recipe for widening Call instructions using library calls.
-class VPWidenCallRecipe : public VPRecipeWithIRFlags, public VPWithIRMetadata {
+class VPWidenCallRecipe : public VPRecipeWithIRFlags, public VPIRMetadata {
   /// Variant stores a pointer to the chosen function. There is a 1:1 mapping
   /// between a given VF and the chosen vectorized variant, so there will be a
   /// different VPlan for each VF with a valid variant.
@@ -1415,7 +1408,7 @@ class VPWidenCallRecipe : public VPRecipeWithIRFlags, public VPWithIRMetadata {
                     DebugLoc DL = {})
       : VPRecipeWithIRFlags(VPDef::VPWidenCallSC, CallArguments,
                             *cast<Instruction>(UV)),
-        VPWithIRMetadata(Metadata), Variant(Variant) {
+        VPIRMetadata(Metadata), Variant(Variant) {
     assert(
         isa<Function>(getOperand(getNumOperands() - 1)->getLiveInIRValue()) &&
         "last operand must be the called function");
@@ -1502,13 +1495,12 @@ class VPHistogramRecipe : public VPRecipeBase {
 };
 
 /// A recipe for widening select instructions.
-struct VPWidenSelectRecipe : public VPRecipeWithIRFlags,
-                             public VPWithIRMetadata {
+struct VPWidenSelectRecipe : public VPRecipeWithIRFlags, public VPIRMetadata {
   template <typename IterT>
   VPWidenSelectRecipe(SelectInst &I, iterator_range<IterT> Operands,
                       MDArrayRef Metadata)
       : VPRecipeWithIRFlags(VPDef::VPWidenSelectSC, Operands, I),
-        VPWithIRMetadata(Metadata) {}
+        VPIRMetadata(Metadata) {}
 
   ~VPWidenSelectRecipe() override = default;
 
@@ -2636,7 +2628,7 @@ class VPPredInstPHIRecipe : public VPSingleDefRecipe {
 
 /// A common base class for widening memory operations. An optional mask can be
 /// provided as the last operand.
-class VPWidenMemoryRecipe : public VPRecipeBase, public VPWithIRMetadata {
+class VPWidenMemoryRecipe : public VPRecipeBase, public VPIRMetadata {
 protected:
   Instruction &Ingredient;
 
@@ -2661,8 +2653,8 @@ class VPWidenMemoryRecipe : public VPRecipeBase, public VPWithIRMetadata {
                       std::initializer_list<VPValue *> Operands,
                       bool Consecutive, bool Reverse, MDArrayRef Metadata,
                       DebugLoc DL)
-      : VPRecipeBase(SC, Operands, DL), VPWithIRMetadata(Metadata),
-        Ingredient(I), Consecutive(Consecutive), Reverse(Reverse) {
+      : VPRecipeBase(SC, Operands, DL), VPIRMetadata(Metadata), Ingredient(I),
+        Consecutive(Consecutive), Reverse(Reverse) {
     assert((Consecutive || !Reverse) && "Reverse implies consecutive");
   }
 
@@ -2731,7 +2723,6 @@ struct VPWidenLoadRecipe final : public VPWidenMemoryRecipe, public VPValue {
     auto *Copy = new VPWidenLoadRecipe(cast<LoadInst>(Ingredient), getAddr(),
                                        getMask(), Consecutive, Reverse,
                                        getMetadata(), getDebugLoc());
-    Copy->addMetadata(getMetadata());
     return Copy;
   }
 
@@ -2766,7 +2757,6 @@ struct VPWidenLoadEVLRecipe final : public VPWidenMemoryRecipe, public VPValue {
                             L.isReverse(), L.getMetadata(), L.getDebugLoc()),
         VPValue(this, &getIngredient()) {
     setMask(Mask);
-    addMetadata(L.getMetadata());
   }
 
   VP_CLASSOF_IMPL(VPDef::VPWidenLoadEVLSC)
@@ -2812,7 +2802,6 @@ struct VPWidenStoreRecipe final : public VPWidenMemoryRecipe {
     auto *Copy = new VPWidenStoreRecipe(
         cast<StoreInst>(Ingredient), getAddr(), getStoredValue(), getMask(),
         Consecutive, Reverse, getMetadata(), getDebugLoc());
-    Copy->addMetadata(getMetadata());
     return Copy;
   }
 
@@ -2850,7 +2839,6 @@ struct VPWidenStoreEVLRecipe final : public VPWidenMemoryRecipe {
                             S.isConsecutive(), S.isReverse(), S.getMetadata(),
                             S.getDebugLoc()) {
     setMask(Mask);
-    addMetadata(S.getMetadata());
   }
 
   VP_CLASSOF_IMPL(VPDef::VPWidenStoreEVLSC)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -1204,7 +1204,7 @@ void VPIRPhi::print(raw_ostream &O, const Twine &Indent,
 }
 #endif
 
-void VPWithIRMetadata::setMetadata(Value *V) const {
+void VPIRMetadata::applyMetadata(Value *V) const {
   auto *I = dyn_cast<Instruction>(V);
   if (!I)
     return;
@@ -1239,10 +1239,10 @@ void VPWidenCallRecipe::execute(VPTransformState &State) {
 
   CallInst *V = State.Builder.CreateCall(Variant, Args, OpBundles);
   setFlags(V);
+  applyMetadata(V);
 
   if (!V->getType()->isVoidTy())
     State.set(this, V);
-  setMetadata(V);
 }
 
 InstructionCost VPWidenCallRecipe::computeCost(ElementCount VF,
@@ -1318,10 +1318,10 @@ void VPWidenIntrinsicRecipe::execute(VPTransformState &State) {
   CallInst *V = State.Builder.CreateCall(VectorF, Args, OpBundles);
 
   setFlags(V);
+  applyMetadata(V);
 
   if (!V->getType()->isVoidTy())
     State.set(this, V);
-  setMetadata(V);
 }
 
 InstructionCost VPWidenIntrinsicRecipe::computeCost(ElementCount VF,
@@ -1518,7 +1518,7 @@ void VPWidenSelectRecipe::execute(VPTransformState &State) {
   State.set(this, Sel);
   if (isa<FPMathOperator>(Sel))
     setFlags(cast<Instruction>(Sel));
-  setMetadata(Sel);
+  applyMetadata(Sel);
 }
 
 InstructionCost VPWidenSelectRecipe::computeCost(ElementCount VF,
@@ -1649,12 +1649,13 @@ void VPWidenRecipe::execute(VPTransformState &State) {
 
     Value *V = Builder.CreateNAryOp(Opcode, Ops);
 
-    if (auto *VecOp = dyn_cast<Instruction>(V))
+    if (auto *VecOp = dyn_cast<Instruction>(V)) {
       setFlags(VecOp);
+      applyMetadata(V);
+    }
 
     // Use this vector value for all users of the original instruction.
     State.set(this, V);
-    setMetadata(V);
     break;
   }
   case Instruction::ExtractValue: {
@@ -1686,8 +1687,8 @@ void VPWidenRecipe::execute(VPTransformState &State) {
     } else {
       C = Builder.CreateICmp(getPredicate(), A, B);
     }
+    applyMetadata(C);
     State.set(this, C);
-    setMetadata(C);
     break;
   }
   default:
@@ -1805,7 +1806,7 @@ void VPWidenCastRecipe::execute(VPTransformState &State) {
   State.set(this, Cast);
   if (auto *CastOp = dyn_cast<Instruction>(Cast)) {
     setFlags(CastOp);
-    setMetadata(CastOp);
+    applyMetadata(CastOp);
   }
 }
 
@@ -2759,7 +2760,7 @@ void VPWidenLoadRecipe::execute(VPTransformState &State) {
   }
   // Add metadata to the load, but setVectorValue to the reverse shuffle.
   State.addNewMetadata(NewLI, LI);
-  setMetadata(NewLI);
+  applyMetadata(NewLI);
   if (Reverse)
     NewLI = Builder.CreateVectorReverse(NewLI, "reverse");
   State.set(this, NewLI);
@@ -2820,7 +2821,7 @@ void VPWidenLoadEVLRecipe::execute(VPTransformState &State) {
   NewLI->addParamAttr(
       0, Attribute::getWithAlignment(NewLI->getContext(), Alignment));
   State.addNewMetadata(NewLI, LI);
-  setMetadata(NewLI);
+  applyMetadata(NewLI);
   Instruction *Res = NewLI;
   if (isReverse())
     Res = createReverseEVL(Builder, Res, EVL, "vp.reverse");
@@ -2897,7 +2898,7 @@ void VPWidenStoreRecipe::execute(VPTransformState &State) {
   else
     NewSI = Builder.CreateAlignedStore(StoredVal, Addr, Alignment);
   State.addNewMetadata(NewSI, SI);
-  setMetadata(NewSI);
+  applyMetadata(NewSI);
 }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
@@ -2945,7 +2946,7 @@ void VPWidenStoreEVLRecipe::execute(VPTransformState &State) {
   NewSI->addParamAttr(
       1, Attribute::getWithAlignment(NewSI->getContext(), Alignment));
   State.addNewMetadata(NewSI, SI);
-  setMetadata(NewSI);
+  applyMetadata(NewSI);
 }
 
 InstructionCost VPWidenStoreEVLRecipe::computeCost(ElementCount VF,
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -70,8 +70,7 @@ bool VPlanTransforms::tryToConvertVPInstructionsToVPRecipes(
         NewRecipe = new VPWidenIntOrFpInductionRecipe(
             Phi, Start, Step, &Plan->getVF(), *II, Ingredient.getDebugLoc());
       } else {
-        SmallVector<std::pair<unsigned, MDNode *>> Metadata;
-        getMetadataToPropagate(Inst, Metadata);
+        auto Metadata = VPRecipeBuilder::getMetadataToPropagate(Inst);
         assert(isa<VPInstruction>(&Ingredient) &&
                "only VPInstructions expected here");
         assert(!isa<PHINode>(Inst) && "phis should be handled above");
