[VPlan] Manage instruction metadata in VPlan. #135272

fhahn · 2025-04-10T22:23:10Z

Add a new helper to manage IR metadata that can be progated to generated instructions for recipes.

This helps to remove a number of remaining uses of getUnderlyingInstr during VPlan execution.

llvmbot · 2025-04-10T22:23:46Z

@llvm/pr-subscribers-llvm-transforms

Author: Florian Hahn (fhahn)

Changes

Add a new helper to manage IR metadata that can be progated to generated instructions for recipes.

This helps to remove a number of remaining uses of getUnderlyingInstr during VPlan execution.

Full diff: https://github.com/llvm/llvm-project/pull/135272.diff

4 Files Affected:

(modified) llvm/lib/Transforms/Vectorize/VPlan.cpp (+5-15)
(modified) llvm/lib/Transforms/Vectorize/VPlan.h (+35-17)
(modified) llvm/lib/Transforms/Vectorize/VPlanHelpers.h (+1-8)
(modified) llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp (+24-11)

diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index 9474e7a171dff..229949725128a 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -356,23 +356,13 @@ BasicBlock *VPTransformState::CFGState::getPreheaderBBFor(VPRecipeBase *R) {
   return VPBB2IRBB[LoopRegion->getPreheaderVPBB()];
 }
 
-void VPTransformState::addNewMetadata(Instruction *To,
-                                      const Instruction *Orig) {
+void VPTransformState::addNewMetadata(Value *To, const Instruction *Orig) {
+
   // If the loop was versioned with memchecks, add the corresponding no-alias
   // metadata.
-  if (LVer && isa<LoadInst, StoreInst>(Orig))
-    LVer->annotateInstWithNoAlias(To, Orig);
-}
-
-void VPTransformState::addMetadata(Value *To, Instruction *From) {
-  // No source instruction to transfer metadata from?
-  if (!From)
-    return;
-
-  if (Instruction *ToI = dyn_cast<Instruction>(To)) {
-    propagateMetadata(ToI, From);
-    addNewMetadata(ToI, From);
-  }
+  Instruction *ToI = dyn_cast<Instruction>(To);
+  if (ToI && LVer && isa<LoadInst, StoreInst>(Orig))
+    LVer->annotateInstWithNoAlias(ToI, Orig);
 }
 
 void VPTransformState::setDebugLocFrom(DebugLoc DL) {
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 669e8f371d3d3..11f637f7b1518 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1114,18 +1114,33 @@ struct VPIRPhi : public VPIRInstruction {
 #endif
 };
 
+/// Helper to manage IR metadata for recipes. It filters out metadata that
+/// cannot be proagated.
+class VPWithIRMetadata {
+  SmallVector<std::pair<unsigned, MDNode *>> Metadata;
+
+public:
+  VPWithIRMetadata() {}
+
+  VPWithIRMetadata(Instruction &I) { getMetadataToPropagate(&I, Metadata); }
+
+  /// Add all metadata to \p V if it is an instruction.
+  void setMetadata(Value *V) const;
+};
+
 /// 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 {
+class VPWidenRecipe : public VPRecipeWithIRFlags, public VPWithIRMetadata {
   unsigned Opcode;
 
 protected:
   template <typename IterT>
   VPWidenRecipe(unsigned VPDefOpcode, Instruction &I,
                 iterator_range<IterT> Operands)
-      : VPRecipeWithIRFlags(VPDefOpcode, Operands, I), Opcode(I.getOpcode()) {}
+      : VPRecipeWithIRFlags(VPDefOpcode, Operands, I), VPWithIRMetadata(I),
+        Opcode(I.getOpcode()) {}
 
 public:
   template <typename IterT>
@@ -1160,7 +1175,7 @@ class VPWidenRecipe : public VPRecipeWithIRFlags {
 };
 
 /// VPWidenCastRecipe is a recipe to create vector cast instructions.
-class VPWidenCastRecipe : public VPRecipeWithIRFlags {
+class VPWidenCastRecipe : public VPRecipeWithIRFlags, public VPWithIRMetadata {
   /// Cast instruction opcode.
   Instruction::CastOps Opcode;
 
@@ -1170,15 +1185,15 @@ class VPWidenCastRecipe : public VPRecipeWithIRFlags {
 public:
   VPWidenCastRecipe(Instruction::CastOps Opcode, VPValue *Op, Type *ResultTy,
                     CastInst &UI)
-      : VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op, UI), Opcode(Opcode),
-        ResultTy(ResultTy) {
+      : VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op, UI), VPWithIRMetadata(UI),
+        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), Opcode(Opcode),
-        ResultTy(ResultTy) {}
+      : VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op), VPWithIRMetadata(),
+        Opcode(Opcode), ResultTy(ResultTy) {}
 
   ~VPWidenCastRecipe() override = default;
 
@@ -1260,7 +1275,8 @@ class VPScalarCastRecipe : public VPSingleDefRecipe {
 };
 
 /// A recipe for widening vector intrinsics.
-class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags {
+class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags,
+                               public VPWithIRMetadata {
   /// ID of the vector intrinsic to widen.
   Intrinsic::ID VectorIntrinsicID;
 
@@ -1281,8 +1297,8 @@ class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags {
                          ArrayRef<VPValue *> CallArguments, Type *Ty,
                          DebugLoc DL = {})
       : VPRecipeWithIRFlags(VPDef::VPWidenIntrinsicSC, CallArguments, CI),
-        VectorIntrinsicID(VectorIntrinsicID), ResultTy(Ty),
-        MayReadFromMemory(CI.mayReadFromMemory()),
+        VPWithIRMetadata(CI), VectorIntrinsicID(VectorIntrinsicID),
+        ResultTy(Ty), MayReadFromMemory(CI.mayReadFromMemory()),
         MayWriteToMemory(CI.mayWriteToMemory()),
         MayHaveSideEffects(CI.mayHaveSideEffects()) {}
 
@@ -1346,7 +1362,7 @@ class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags {
 };
 
 /// A recipe for widening Call instructions using library calls.
-class VPWidenCallRecipe : public VPRecipeWithIRFlags {
+class VPWidenCallRecipe : public VPRecipeWithIRFlags, public VPWithIRMetadata {
   /// 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.
@@ -1357,7 +1373,7 @@ class VPWidenCallRecipe : public VPRecipeWithIRFlags {
                     ArrayRef<VPValue *> CallArguments, DebugLoc DL = {})
       : VPRecipeWithIRFlags(VPDef::VPWidenCallSC, CallArguments,
                             *cast<Instruction>(UV)),
-        Variant(Variant) {
+        VPWithIRMetadata(*cast<Instruction>(UV)), Variant(Variant) {
     assert(
         isa<Function>(getOperand(getNumOperands() - 1)->getLiveInIRValue()) &&
         "last operand must be the called function");
@@ -1443,10 +1459,12 @@ class VPHistogramRecipe : public VPRecipeBase {
 };
 
 /// A recipe for widening select instructions.
-struct VPWidenSelectRecipe : public VPRecipeWithIRFlags {
+struct VPWidenSelectRecipe : public VPRecipeWithIRFlags,
+                             public VPWithIRMetadata {
   template <typename IterT>
   VPWidenSelectRecipe(SelectInst &I, iterator_range<IterT> Operands)
-      : VPRecipeWithIRFlags(VPDef::VPWidenSelectSC, Operands, I) {}
+      : VPRecipeWithIRFlags(VPDef::VPWidenSelectSC, Operands, I),
+        VPWithIRMetadata(I) {}
 
   ~VPWidenSelectRecipe() override = default;
 
@@ -2564,7 +2582,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 {
+class VPWidenMemoryRecipe : public VPRecipeBase, public VPWithIRMetadata {
 protected:
   Instruction &Ingredient;
 
@@ -2588,8 +2606,8 @@ class VPWidenMemoryRecipe : public VPRecipeBase {
   VPWidenMemoryRecipe(const char unsigned SC, Instruction &I,
                       std::initializer_list<VPValue *> Operands,
                       bool Consecutive, bool Reverse, DebugLoc DL)
-      : VPRecipeBase(SC, Operands, DL), Ingredient(I), Consecutive(Consecutive),
-        Reverse(Reverse) {
+      : VPRecipeBase(SC, Operands, DL), VPWithIRMetadata(I), Ingredient(I),
+        Consecutive(Consecutive), Reverse(Reverse) {
     assert((Consecutive || !Reverse) && "Reverse implies consecutive");
   }
 
diff --git a/llvm/lib/Transforms/Vectorize/VPlanHelpers.h b/llvm/lib/Transforms/Vectorize/VPlanHelpers.h
index bebea1915690f..4826dfd562c2d 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHelpers.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanHelpers.h
@@ -288,14 +288,7 @@ struct VPTransformState {
   /// Currently this is used to add the noalias annotations based on the
   /// inserted memchecks.  Use this for instructions that are *cloned* into the
   /// vector loop.
-  void addNewMetadata(Instruction *To, const Instruction *Orig);
-
-  /// Add metadata from one instruction to another.
-  ///
-  /// This includes both the original MDs from \p From and additional ones (\see
-  /// addNewMetadata).  Use this for *newly created* instructions in the vector
-  /// loop.
-  void addMetadata(Value *To, Instruction *From);
+  void addNewMetadata(Value *To, const Instruction *Orig);
 
   /// Set the debug location in the builder using the debug location \p DL.
   void setDebugLocFrom(DebugLoc DL);
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index 35e141ed01267..077867cbc7a92 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -1159,6 +1159,14 @@ void VPIRPhi::print(raw_ostream &O, const Twine &Indent,
 }
 #endif
 
+void VPWithIRMetadata::setMetadata(Value *V) const {
+  auto *I = dyn_cast<Instruction>(V);
+  if (!I)
+    return;
+  for (const auto &[Kind, Node] : Metadata)
+    I->setMetadata(Kind, Node);
+}
+
 void VPWidenCallRecipe::execute(VPTransformState &State) {
   assert(State.VF.isVector() && "not widening");
 
@@ -1189,7 +1197,7 @@ void VPWidenCallRecipe::execute(VPTransformState &State) {
 
   if (!V->getType()->isVoidTy())
     State.set(this, V);
-  State.addMetadata(V, CI);
+  setMetadata(V);
 }
 
 InstructionCost VPWidenCallRecipe::computeCost(ElementCount VF,
@@ -1268,7 +1276,7 @@ void VPWidenIntrinsicRecipe::execute(VPTransformState &State) {
 
   if (!V->getType()->isVoidTy())
     State.set(this, V);
-  State.addMetadata(V, CI);
+  setMetadata(V);
 }
 
 InstructionCost VPWidenIntrinsicRecipe::computeCost(ElementCount VF,
@@ -1465,7 +1473,7 @@ void VPWidenSelectRecipe::execute(VPTransformState &State) {
   State.set(this, Sel);
   if (isa<FPMathOperator>(Sel))
     setFlags(cast<Instruction>(Sel));
-  State.addMetadata(Sel, dyn_cast_or_null<Instruction>(getUnderlyingValue()));
+  setMetadata(Sel);
 }
 
 InstructionCost VPWidenSelectRecipe::computeCost(ElementCount VF,
@@ -1601,7 +1609,7 @@ void VPWidenRecipe::execute(VPTransformState &State) {
 
     // Use this vector value for all users of the original instruction.
     State.set(this, V);
-    State.addMetadata(V, dyn_cast_or_null<Instruction>(getUnderlyingValue()));
+    setMetadata(V);
     break;
   }
   case Instruction::ExtractValue: {
@@ -1634,7 +1642,7 @@ void VPWidenRecipe::execute(VPTransformState &State) {
       C = Builder.CreateICmp(getPredicate(), A, B);
     }
     State.set(this, C);
-    State.addMetadata(C, dyn_cast_or_null<Instruction>(getUnderlyingValue()));
+    setMetadata(C);
     break;
   }
   default:
@@ -1750,9 +1758,10 @@ void VPWidenCastRecipe::execute(VPTransformState &State) {
   Value *A = State.get(Op);
   Value *Cast = Builder.CreateCast(Instruction::CastOps(Opcode), A, DestTy);
   State.set(this, Cast);
-  State.addMetadata(Cast, cast_or_null<Instruction>(getUnderlyingValue()));
-  if (auto *CastOp = dyn_cast<Instruction>(Cast))
+  if (auto *CastOp = dyn_cast<Instruction>(Cast)) {
     setFlags(CastOp);
+    setMetadata(CastOp);
+  }
 }
 
 InstructionCost VPWidenCastRecipe::computeCost(ElementCount VF,
@@ -2735,7 +2744,8 @@ void VPWidenLoadRecipe::execute(VPTransformState &State) {
     NewLI = Builder.CreateAlignedLoad(DataTy, Addr, Alignment, "wide.load");
   }
   // Add metadata to the load, but setVectorValue to the reverse shuffle.
-  State.addMetadata(NewLI, LI);
+  State.addNewMetadata(NewLI, LI);
+  setMetadata(NewLI);
   if (Reverse)
     NewLI = Builder.CreateVectorReverse(NewLI, "reverse");
   State.set(this, NewLI);
@@ -2795,7 +2805,8 @@ void VPWidenLoadEVLRecipe::execute(VPTransformState &State) {
   }
   NewLI->addParamAttr(
       0, Attribute::getWithAlignment(NewLI->getContext(), Alignment));
-  State.addMetadata(NewLI, LI);
+  State.addNewMetadata(NewLI, LI);
+  setMetadata(NewLI);
   Instruction *Res = NewLI;
   if (isReverse())
     Res = createReverseEVL(Builder, Res, EVL, "vp.reverse");
@@ -2871,7 +2882,8 @@ void VPWidenStoreRecipe::execute(VPTransformState &State) {
     NewSI = Builder.CreateMaskedStore(StoredVal, Addr, Alignment, Mask);
   else
     NewSI = Builder.CreateAlignedStore(StoredVal, Addr, Alignment);
-  State.addMetadata(NewSI, SI);
+  State.addNewMetadata(NewSI, SI);
+  setMetadata(NewSI);
 }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
@@ -2918,7 +2930,8 @@ void VPWidenStoreEVLRecipe::execute(VPTransformState &State) {
   }
   NewSI->addParamAttr(
       1, Attribute::getWithAlignment(NewSI->getContext(), Alignment));
-  State.addMetadata(NewSI, SI);
+  State.addNewMetadata(NewSI, SI);
+  setMetadata(NewSI);
 }
 
 InstructionCost VPWidenStoreEVLRecipe::computeCost(ElementCount VF,

llvmbot · 2025-04-10T22:23:47Z

@llvm/pr-subscribers-vectorizers

Author: Florian Hahn (fhahn)

Changes

Add a new helper to manage IR metadata that can be progated to generated instructions for recipes.

This helps to remove a number of remaining uses of getUnderlyingInstr during VPlan execution.

Full diff: https://github.com/llvm/llvm-project/pull/135272.diff

4 Files Affected:

(modified) llvm/lib/Transforms/Vectorize/VPlan.cpp (+5-15)
(modified) llvm/lib/Transforms/Vectorize/VPlan.h (+35-17)
(modified) llvm/lib/Transforms/Vectorize/VPlanHelpers.h (+1-8)
(modified) llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp (+24-11)

diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index 9474e7a171dff..229949725128a 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -356,23 +356,13 @@ BasicBlock *VPTransformState::CFGState::getPreheaderBBFor(VPRecipeBase *R) {
   return VPBB2IRBB[LoopRegion->getPreheaderVPBB()];
 }
 
-void VPTransformState::addNewMetadata(Instruction *To,
-                                      const Instruction *Orig) {
+void VPTransformState::addNewMetadata(Value *To, const Instruction *Orig) {
+
   // If the loop was versioned with memchecks, add the corresponding no-alias
   // metadata.
-  if (LVer && isa<LoadInst, StoreInst>(Orig))
-    LVer->annotateInstWithNoAlias(To, Orig);
-}
-
-void VPTransformState::addMetadata(Value *To, Instruction *From) {
-  // No source instruction to transfer metadata from?
-  if (!From)
-    return;
-
-  if (Instruction *ToI = dyn_cast<Instruction>(To)) {
-    propagateMetadata(ToI, From);
-    addNewMetadata(ToI, From);
-  }
+  Instruction *ToI = dyn_cast<Instruction>(To);
+  if (ToI && LVer && isa<LoadInst, StoreInst>(Orig))
+    LVer->annotateInstWithNoAlias(ToI, Orig);
 }
 
 void VPTransformState::setDebugLocFrom(DebugLoc DL) {
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 669e8f371d3d3..11f637f7b1518 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1114,18 +1114,33 @@ struct VPIRPhi : public VPIRInstruction {
 #endif
 };
 
+/// Helper to manage IR metadata for recipes. It filters out metadata that
+/// cannot be proagated.
+class VPWithIRMetadata {
+  SmallVector<std::pair<unsigned, MDNode *>> Metadata;
+
+public:
+  VPWithIRMetadata() {}
+
+  VPWithIRMetadata(Instruction &I) { getMetadataToPropagate(&I, Metadata); }
+
+  /// Add all metadata to \p V if it is an instruction.
+  void setMetadata(Value *V) const;
+};
+
 /// 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 {
+class VPWidenRecipe : public VPRecipeWithIRFlags, public VPWithIRMetadata {
   unsigned Opcode;
 
 protected:
   template <typename IterT>
   VPWidenRecipe(unsigned VPDefOpcode, Instruction &I,
                 iterator_range<IterT> Operands)
-      : VPRecipeWithIRFlags(VPDefOpcode, Operands, I), Opcode(I.getOpcode()) {}
+      : VPRecipeWithIRFlags(VPDefOpcode, Operands, I), VPWithIRMetadata(I),
+        Opcode(I.getOpcode()) {}
 
 public:
   template <typename IterT>
@@ -1160,7 +1175,7 @@ class VPWidenRecipe : public VPRecipeWithIRFlags {
 };
 
 /// VPWidenCastRecipe is a recipe to create vector cast instructions.
-class VPWidenCastRecipe : public VPRecipeWithIRFlags {
+class VPWidenCastRecipe : public VPRecipeWithIRFlags, public VPWithIRMetadata {
   /// Cast instruction opcode.
   Instruction::CastOps Opcode;
 
@@ -1170,15 +1185,15 @@ class VPWidenCastRecipe : public VPRecipeWithIRFlags {
 public:
   VPWidenCastRecipe(Instruction::CastOps Opcode, VPValue *Op, Type *ResultTy,
                     CastInst &UI)
-      : VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op, UI), Opcode(Opcode),
-        ResultTy(ResultTy) {
+      : VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op, UI), VPWithIRMetadata(UI),
+        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), Opcode(Opcode),
-        ResultTy(ResultTy) {}
+      : VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op), VPWithIRMetadata(),
+        Opcode(Opcode), ResultTy(ResultTy) {}
 
   ~VPWidenCastRecipe() override = default;
 
@@ -1260,7 +1275,8 @@ class VPScalarCastRecipe : public VPSingleDefRecipe {
 };
 
 /// A recipe for widening vector intrinsics.
-class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags {
+class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags,
+                               public VPWithIRMetadata {
   /// ID of the vector intrinsic to widen.
   Intrinsic::ID VectorIntrinsicID;
 
@@ -1281,8 +1297,8 @@ class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags {
                          ArrayRef<VPValue *> CallArguments, Type *Ty,
                          DebugLoc DL = {})
       : VPRecipeWithIRFlags(VPDef::VPWidenIntrinsicSC, CallArguments, CI),
-        VectorIntrinsicID(VectorIntrinsicID), ResultTy(Ty),
-        MayReadFromMemory(CI.mayReadFromMemory()),
+        VPWithIRMetadata(CI), VectorIntrinsicID(VectorIntrinsicID),
+        ResultTy(Ty), MayReadFromMemory(CI.mayReadFromMemory()),
         MayWriteToMemory(CI.mayWriteToMemory()),
         MayHaveSideEffects(CI.mayHaveSideEffects()) {}
 
@@ -1346,7 +1362,7 @@ class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags {
 };
 
 /// A recipe for widening Call instructions using library calls.
-class VPWidenCallRecipe : public VPRecipeWithIRFlags {
+class VPWidenCallRecipe : public VPRecipeWithIRFlags, public VPWithIRMetadata {
   /// 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.
@@ -1357,7 +1373,7 @@ class VPWidenCallRecipe : public VPRecipeWithIRFlags {
                     ArrayRef<VPValue *> CallArguments, DebugLoc DL = {})
       : VPRecipeWithIRFlags(VPDef::VPWidenCallSC, CallArguments,
                             *cast<Instruction>(UV)),
-        Variant(Variant) {
+        VPWithIRMetadata(*cast<Instruction>(UV)), Variant(Variant) {
     assert(
         isa<Function>(getOperand(getNumOperands() - 1)->getLiveInIRValue()) &&
         "last operand must be the called function");
@@ -1443,10 +1459,12 @@ class VPHistogramRecipe : public VPRecipeBase {
 };
 
 /// A recipe for widening select instructions.
-struct VPWidenSelectRecipe : public VPRecipeWithIRFlags {
+struct VPWidenSelectRecipe : public VPRecipeWithIRFlags,
+                             public VPWithIRMetadata {
   template <typename IterT>
   VPWidenSelectRecipe(SelectInst &I, iterator_range<IterT> Operands)
-      : VPRecipeWithIRFlags(VPDef::VPWidenSelectSC, Operands, I) {}
+      : VPRecipeWithIRFlags(VPDef::VPWidenSelectSC, Operands, I),
+        VPWithIRMetadata(I) {}
 
   ~VPWidenSelectRecipe() override = default;
 
@@ -2564,7 +2582,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 {
+class VPWidenMemoryRecipe : public VPRecipeBase, public VPWithIRMetadata {
 protected:
   Instruction &Ingredient;
 
@@ -2588,8 +2606,8 @@ class VPWidenMemoryRecipe : public VPRecipeBase {
   VPWidenMemoryRecipe(const char unsigned SC, Instruction &I,
                       std::initializer_list<VPValue *> Operands,
                       bool Consecutive, bool Reverse, DebugLoc DL)
-      : VPRecipeBase(SC, Operands, DL), Ingredient(I), Consecutive(Consecutive),
-        Reverse(Reverse) {
+      : VPRecipeBase(SC, Operands, DL), VPWithIRMetadata(I), Ingredient(I),
+        Consecutive(Consecutive), Reverse(Reverse) {
     assert((Consecutive || !Reverse) && "Reverse implies consecutive");
   }
 
diff --git a/llvm/lib/Transforms/Vectorize/VPlanHelpers.h b/llvm/lib/Transforms/Vectorize/VPlanHelpers.h
index bebea1915690f..4826dfd562c2d 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHelpers.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanHelpers.h
@@ -288,14 +288,7 @@ struct VPTransformState {
   /// Currently this is used to add the noalias annotations based on the
   /// inserted memchecks.  Use this for instructions that are *cloned* into the
   /// vector loop.
-  void addNewMetadata(Instruction *To, const Instruction *Orig);
-
-  /// Add metadata from one instruction to another.
-  ///
-  /// This includes both the original MDs from \p From and additional ones (\see
-  /// addNewMetadata).  Use this for *newly created* instructions in the vector
-  /// loop.
-  void addMetadata(Value *To, Instruction *From);
+  void addNewMetadata(Value *To, const Instruction *Orig);
 
   /// Set the debug location in the builder using the debug location \p DL.
   void setDebugLocFrom(DebugLoc DL);
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index 35e141ed01267..077867cbc7a92 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -1159,6 +1159,14 @@ void VPIRPhi::print(raw_ostream &O, const Twine &Indent,
 }
 #endif
 
+void VPWithIRMetadata::setMetadata(Value *V) const {
+  auto *I = dyn_cast<Instruction>(V);
+  if (!I)
+    return;
+  for (const auto &[Kind, Node] : Metadata)
+    I->setMetadata(Kind, Node);
+}
+
 void VPWidenCallRecipe::execute(VPTransformState &State) {
   assert(State.VF.isVector() && "not widening");
 
@@ -1189,7 +1197,7 @@ void VPWidenCallRecipe::execute(VPTransformState &State) {
 
   if (!V->getType()->isVoidTy())
     State.set(this, V);
-  State.addMetadata(V, CI);
+  setMetadata(V);
 }
 
 InstructionCost VPWidenCallRecipe::computeCost(ElementCount VF,
@@ -1268,7 +1276,7 @@ void VPWidenIntrinsicRecipe::execute(VPTransformState &State) {
 
   if (!V->getType()->isVoidTy())
     State.set(this, V);
-  State.addMetadata(V, CI);
+  setMetadata(V);
 }
 
 InstructionCost VPWidenIntrinsicRecipe::computeCost(ElementCount VF,
@@ -1465,7 +1473,7 @@ void VPWidenSelectRecipe::execute(VPTransformState &State) {
   State.set(this, Sel);
   if (isa<FPMathOperator>(Sel))
     setFlags(cast<Instruction>(Sel));
-  State.addMetadata(Sel, dyn_cast_or_null<Instruction>(getUnderlyingValue()));
+  setMetadata(Sel);
 }
 
 InstructionCost VPWidenSelectRecipe::computeCost(ElementCount VF,
@@ -1601,7 +1609,7 @@ void VPWidenRecipe::execute(VPTransformState &State) {
 
     // Use this vector value for all users of the original instruction.
     State.set(this, V);
-    State.addMetadata(V, dyn_cast_or_null<Instruction>(getUnderlyingValue()));
+    setMetadata(V);
     break;
   }
   case Instruction::ExtractValue: {
@@ -1634,7 +1642,7 @@ void VPWidenRecipe::execute(VPTransformState &State) {
       C = Builder.CreateICmp(getPredicate(), A, B);
     }
     State.set(this, C);
-    State.addMetadata(C, dyn_cast_or_null<Instruction>(getUnderlyingValue()));
+    setMetadata(C);
     break;
   }
   default:
@@ -1750,9 +1758,10 @@ void VPWidenCastRecipe::execute(VPTransformState &State) {
   Value *A = State.get(Op);
   Value *Cast = Builder.CreateCast(Instruction::CastOps(Opcode), A, DestTy);
   State.set(this, Cast);
-  State.addMetadata(Cast, cast_or_null<Instruction>(getUnderlyingValue()));
-  if (auto *CastOp = dyn_cast<Instruction>(Cast))
+  if (auto *CastOp = dyn_cast<Instruction>(Cast)) {
     setFlags(CastOp);
+    setMetadata(CastOp);
+  }
 }
 
 InstructionCost VPWidenCastRecipe::computeCost(ElementCount VF,
@@ -2735,7 +2744,8 @@ void VPWidenLoadRecipe::execute(VPTransformState &State) {
     NewLI = Builder.CreateAlignedLoad(DataTy, Addr, Alignment, "wide.load");
   }
   // Add metadata to the load, but setVectorValue to the reverse shuffle.
-  State.addMetadata(NewLI, LI);
+  State.addNewMetadata(NewLI, LI);
+  setMetadata(NewLI);
   if (Reverse)
     NewLI = Builder.CreateVectorReverse(NewLI, "reverse");
   State.set(this, NewLI);
@@ -2795,7 +2805,8 @@ void VPWidenLoadEVLRecipe::execute(VPTransformState &State) {
   }
   NewLI->addParamAttr(
       0, Attribute::getWithAlignment(NewLI->getContext(), Alignment));
-  State.addMetadata(NewLI, LI);
+  State.addNewMetadata(NewLI, LI);
+  setMetadata(NewLI);
   Instruction *Res = NewLI;
   if (isReverse())
     Res = createReverseEVL(Builder, Res, EVL, "vp.reverse");
@@ -2871,7 +2882,8 @@ void VPWidenStoreRecipe::execute(VPTransformState &State) {
     NewSI = Builder.CreateMaskedStore(StoredVal, Addr, Alignment, Mask);
   else
     NewSI = Builder.CreateAlignedStore(StoredVal, Addr, Alignment);
-  State.addMetadata(NewSI, SI);
+  State.addNewMetadata(NewSI, SI);
+  setMetadata(NewSI);
 }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
@@ -2918,7 +2930,8 @@ void VPWidenStoreEVLRecipe::execute(VPTransformState &State) {
   }
   NewSI->addParamAttr(
       1, Attribute::getWithAlignment(NewSI->getContext(), Alignment));
-  State.addMetadata(NewSI, SI);
+  State.addNewMetadata(NewSI, SI);
+  setMetadata(NewSI);
 }
 
 InstructionCost VPWidenStoreEVLRecipe::computeCost(ElementCount VF,

ayalz · 2025-04-16T20:04:33Z

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

+SmallVector<std::pair<unsigned, MDNode *>>
+VPRecipeBuilder::getMetadataToPropagate(Instruction *I) {
+  SmallVector<std::pair<unsigned, MDNode *>> Metadata;
+  ::getMetadataToPropagate(I, Metadata);


What does ::getMetadataToPropagate() refer to?

It's a helper to get the metadata that can and should be propagated to the vector instructions: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/VectorUtils.cpp#L991

Ah, thanks.
Should these two getMetadataToPropagate() variants be located next to each other, presumably both in VectorUtils?

We will need a custom version that also adds no-alias metadata from LoopVersioning (in #136450), which is why i kept it here

Should that LVer version also be placed in VectorUtils?

Not sure, it is an implementation detail on LV and we would have to pass around the pointer in multiple places.

(It's gone for now in the latest version)

ayalz · 2025-04-17T14:46:28Z

llvm/lib/Transforms/Vectorize/VPlan.cpp

-  }
+  Instruction *ToI = dyn_cast<Instruction>(To);
+  if (ToI && LVer && isa<LoadInst, StoreInst>(Orig))
+    LVer->annotateInstWithNoAlias(ToI, Orig);


Better add the corresponding no-alias metadata to VPWithIRMetadata of relevant recipe, than to the ToI it generates during execution?

Yep, this is probably best done separately as follow-up: #136450

ayalz · 2025-04-17T15:08:13Z

llvm/lib/Transforms/Vectorize/VPlan.h

+
+/// Helper to manage IR metadata for recipes. It filters out metadata that
+/// cannot be proagated.
+class VPWithIRMetadata {


"With" seems redundant here. There are IRFlags and IRMetadata, there are recipes With the former or the latter or both or none. IRFlags are modeled by relevant VPlan recipes using a nameless union which VPRecipeWithIRFlags "has", rather than inherits from. VP[IR]FlagsTy might be an appropriate name for this union, and VP[IR]Flags a name for a base class. VP[IR]Metadata may be a consistent name for a base class wrapping IRMetadata.

VPRecipeWithMetadata may serve as a common base class for recipes having metadata but not flags, but according to this patch these currently include only VPWidenMemoryRecipe's, so perhaps currently redundant.

VPRecipeWithFlagsAndMetadata may serve as a common base class for recipes having both, which according to this patch currently include { VPWidenRecipe, VPWidenCallRecipe, VPWidenIntrinsicRecipe, VPWidenCastRecipe and VPWidenSelectRecipe }.

VPRecipeWith[IR]Flags will then continue to serve the following subclasses of recipes having flags but no metadata: { VPInstruction(?), VPReplicateRecipe(?), VPWidenGEPRecipe, VPScalarIVStepsRecipe, VPVectorPointerRecipe, VPReverseVectorPointerRecipe, }

I think VPRecipeWithIRFlags is not ideal at the moment, as it makes it more difficult to compose, ideally I think we would move towards replacing it with just VPIRFlags, which the relevant recipes opt in to separately, like VPIRMetadata.

Updated the name as suggested, thanks!

ayalz · 2025-04-17T21:41:50Z

llvm/lib/Transforms/Vectorize/VPlan.h

+    auto *Copy = new VPWidenLoadRecipe(cast<LoadInst>(Ingredient), getAddr(),
+                                       getMask(), Consecutive, Reverse,
+                                       getMetadata(), getDebugLoc());
+    Copy->addMetadata(getMetadata());


Copy already got the metadata upon construction?

Yep this was left over from an earlier version, removed, thanks!

ayalz · 2025-04-17T21:43:35Z

llvm/lib/Transforms/Vectorize/VPlan.h

        VPValue(this, &getIngredient()) {
    setMask(Mask);
+    addMetadata(L.getMetadata());


L's metadata was already gotten upon construction?

Yep this was left over from an earlier version, removed, thanks!

ayalz · 2025-04-18T09:40:08Z

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

@@ -1204,6 +1204,14 @@ void VPIRPhi::print(raw_ostream &O, const Twine &Indent,
 }
 #endif

+void VPWithIRMetadata::setMetadata(Value *V) const {


Better have the caller dyn_cast to Instruction (and realize metadata may be lost for non Instruction Values) than pretend this can set metadata to any Value?

Could do if preferred, but then we would need to update every caller?

ayalz · 2025-04-18T09:42:19Z

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp


  if (!V->getType()->isVoidTy())
    State.set(this, V);


nit: good to place setMetadata() next to setFlags() in general, here and below.

Suggested change

if (!V->getType()->isVoidTy())

State.set(this, V);

setMetadata(V);

if (!V->getType()->isVoidTy())

State.set(this, V);

Done thanks

ayalz · 2025-04-18T09:42:53Z

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

@@ -1234,7 +1242,7 @@ void VPWidenCallRecipe::execute(VPTransformState &State) {

  if (!V->getType()->isVoidTy())
    State.set(this, V);
-  State.addMetadata(V, CI);
+  setMetadata(V);


nit: good to place setMetadata() next to setFlags() in general, here and below.

Suggested change

setMetadata(V);

Done thanks

ayalz · 2025-04-18T09:45:29Z

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

@@ -1646,7 +1654,7 @@ void VPWidenRecipe::execute(VPTransformState &State) {



Here setFlags() is already applied under dyn_cast to Instruction. Place setMetadata() here as well, or rather applyMetadata(Instruction*)?

Done, thanks

ayalz · 2025-04-18T09:56:58Z

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

@@ -69,20 +69,22 @@ bool VPlanTransforms::tryToConvertVPInstructionsToVPRecipes(
        NewRecipe = new VPWidenIntOrFpInductionRecipe(
            Phi, Start, Step, &Plan->getVF(), *II, Ingredient.getDebugLoc());
      } else {
+        SmallVector<std::pair<unsigned, MDNode *>> Metadata;
+        getMetadataToPropagate(Inst, Metadata);


Recipe constructors above called getMetadataToPropagate(underlying Instruction) inline. Here we want to record Metadata earlier for several alternative users. Would doing Metadata = getMetadataToPropagate(Inst) work and be more consistent? Alternatively, could the metadata be added to NewRecipe after its construction?

updated to use VPRecipeBuilder::getMetadataToPropagate, thanks

Add a new helper to manage IR metadata that can be progated to generated instructions for recipes. This helps to remove a number of remaining uses of getUnderlyingInstr during VPlan execution. Add metadata on cloning.

Use VPIRMetadata added in llvm#135272 to also manage no-alias metadata added by versioning. Note that this means we have to build the no-alias metadata up-front once. If it is not used, it will be discarded automatically.

ayalz · 2025-04-20T13:05:59Z

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

@@ -8599,11 +8599,13 @@ VPRecipeBuilder::tryToWidenMemory(Instruction *I, ArrayRef<VPValue *> Operands,
  }
  if (LoadInst *Load = dyn_cast<LoadInst>(I))
    return new VPWidenLoadRecipe(*Load, Ptr, Mask, Consecutive, Reverse,
+                                 getMetadataToPropagate(Load),


If passing the metadata relieves the need to pass and store the underlying insn - great!
If the underlying insn is still needed, would it be better to have the constructor extract its metadata (to propagate, thereby reducing the diff), rather than pass both parameters independently, or are there cases where some other metadata is desired - e.g. none, as appears below?

Done, although it will need explicit passing in #136450

ayalz · 2025-04-20T13:08:27Z

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

+SmallVector<std::pair<unsigned, MDNode *>>
+VPRecipeBuilder::getMetadataToPropagate(Instruction *I) {
+  SmallVector<std::pair<unsigned, MDNode *>> Metadata;
+  ::getMetadataToPropagate(I, Metadata);


Ah, thanks.
Should these two getMetadataToPropagate() variants be located next to each other, presumably both in VectorUtils?

ayalz · 2025-04-20T18:07:11Z

llvm/lib/Transforms/Vectorize/VPlan.h

    assert(UI.getOpcode() == Opcode &&
           "opcode of underlying cast doesn't match");
  }

  VPWidenCastRecipe(Instruction::CastOps Opcode, VPValue *Op, Type *ResultTy)
-      : VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op), Opcode(Opcode),
-        ResultTy(ResultTy) {}
+      : VPRecipeWithIRFlags(VPDef::VPWidenCastSC, Op), VPIRMetadata({}),


This constructor has neither an underlying instruction nor any metadata.

Yep to keep NFC, also removed the metadata arg from VPWidenIntrinsicRecipe. Can be added in follow-ups to retain additional metadata.

ayalz · 2025-04-20T18:09:29Z

llvm/lib/Transforms/Vectorize/VPlan.h

        MayWriteToMemory(CI.mayWriteToMemory()),
        MayHaveSideEffects(CI.mayHaveSideEffects()) {}

  VPWidenIntrinsicRecipe(Intrinsic::ID VectorIntrinsicID,
                         ArrayRef<VPValue *> CallArguments, Type *Ty,
-                         DebugLoc DL = {})
+                         MDArrayRef Metadata, DebugLoc DL = {})


This constructor has no underlying instruction but does accept metadata.

Removed for now to keep this NFC. There are cases where we convert VPWidenIntrinsic recipes from wide recieps that can have metadata, which could be retained now. But best for a future change + test

ayalz · 2025-04-20T18:14:57Z

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

      setFlags(VecOp);
+      applyMetadata(V);


Suggested change

applyMetadata(V);

applyMetadata(VecOp);

(consistency)

done thanks

ayalz · 2025-04-20T18:20:48Z

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

+            Intrinsic::vp_select, Ops, TypeInfo.inferScalarType(Sel),
+            Sel->getMetadata(), Sel->getDebugLoc());


Here and below VPWidenIntrinsicRecipe is relieved from receiving the underlying instruction Sel, so its metadata needs to be passed.

ayalz · 2025-04-20T18:28:30Z

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

@@ -2755,7 +2758,7 @@ void VPlanTransforms::narrowInterleaveGroups(VPlan &Plan, ElementCount VF,
      auto *L = new VPWidenLoadRecipe(
          *cast<LoadInst>(LoadGroup->getInterleaveGroup()->getInsertPos()),
          LoadGroup->getAddr(), LoadGroup->getMask(), /*Consecutive=*/true,
-          /*Reverse=*/false, LoadGroup->getDebugLoc());
+          /*Reverse=*/false, {}, LoadGroup->getDebugLoc());


The single widenLoad/StoreRecipe in this case drops the metadata of its insertPos/underlying load/store? Seems an exception worthy of a note, perhaps an explicit subsequent dropMetadata() call?

This change is no longer needed?

ayalz · 2025-04-20T18:33:35Z

llvm/unittests/Transforms/Vectorize/VPlanTest.cpp

@@ -929,7 +929,7 @@ TEST_F(VPRecipeTest, CastVPWidenRecipeToVPUser) {
  SmallVector<VPValue *, 2> Args;
  Args.push_back(Op1);
  Args.push_back(Op2);
-  VPWidenRecipe WidenR(*AI, make_range(Args.begin(), Args.end()));
+  VPWidenRecipe WidenR(*AI, make_range(Args.begin(), Args.end()), {});


Here and below newly created Instructions are wrapped in widen recipes, should their metadata be preserved (by default) rather than dropped (explicitly)?

This is just for testing casting recipes, so I don't think the metadata matters here

ayalz · 2025-04-20T18:44:28Z

llvm/lib/Transforms/Vectorize/VPlan.h

+  /// Add all metadata to \p V if it is an instruction.
+  void applyMetadata(Value *V) const;


(original post outdated as it referred to setMetadata(), resurrecting here)

Better have the caller dyn_cast to Instruction (and realize metadata may be lost for non Instruction Values) than pretend this can set metadata to any Value?

Could do if preferred, but then we would need to update every caller?

Yes, admittedly, consistent with setFlags().
In some cases an assert/cast may be appropriate rather than a dyn_cast, when value is expected to be an instruction (otherwise cost may be inaccurate?)

turns out most places already check for instructions or have instructions. Updated to always pass as instruction, thanks!

setFlags already only accepts an instruction

Yeah, suggestion was inspired by setFlags(Instruction*).

Update name to apply flags to instructions, as suggested in #135272. Also changes the arg to a reference.

ayalz

Some comments relate to outdated changes.

ayalz · 2025-04-21T20:30:34Z

llvm/lib/Transforms/Vectorize/VPlan.h

+    auto *Copy =
+        new VPWidenLoadRecipe(cast<LoadInst>(Ingredient), getAddr(), getMask(),
+                              Consecutive, Reverse, getDebugLoc());
+    return Copy;


Retain existing inlined version?

done, thanks

ayalz · 2025-04-21T20:30:53Z

llvm/lib/Transforms/Vectorize/VPlan.h

+    auto *Copy = new VPWidenStoreRecipe(cast<StoreInst>(Ingredient), getAddr(),
+                                        getStoredValue(), getMask(),
+                                        Consecutive, Reverse, getDebugLoc());
+    return Copy;


done, thanks

ayalz · 2025-04-21T20:36:40Z

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -2626,8 +2652,8 @@ class VPWidenMemoryRecipe : public VPRecipeBase {
  VPWidenMemoryRecipe(const char unsigned SC, Instruction &I,
                      std::initializer_list<VPValue *> Operands,
                      bool Consecutive, bool Reverse, DebugLoc DL)
-      : VPRecipeBase(SC, Operands, DL), Ingredient(I), Consecutive(Consecutive),
-        Reverse(Reverse) {
+      : VPRecipeBase(SC, Operands, DL), VPIRMetadata(I), Ingredient(I),


Can this way of initializing VPIRMetadata(I) be applied to all recipe constructors that receive their underlying instruction I as parameter? This seems like a natural default behavior, could be followed by an update if some other exceptional metadata is desired.

Updated for now, although it might be good to encourage avoidance of relying on underlying instructions on construction ;)

Certainly, by all means!
Any recipe that can be constructed w/o an underlying instruction should be; at that point its metadata can be passed instead of the instruction itself.

ayalz · 2025-04-21T20:43:07Z

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

+SmallVector<std::pair<unsigned, MDNode *>>
+VPRecipeBuilder::getMetadataToPropagate(Instruction *I) {
+  SmallVector<std::pair<unsigned, MDNode *>> Metadata;
+  ::getMetadataToPropagate(I, Metadata);


Should that LVer version also be placed in VectorUtils?

ayalz · 2025-04-21T20:47:13Z

llvm/unittests/Transforms/Vectorize/VPlanTest.cpp

@@ -929,7 +929,7 @@ TEST_F(VPRecipeTest, CastVPWidenRecipeToVPUser) {
  SmallVector<VPValue *, 2> Args;
  Args.push_back(Op1);
  Args.push_back(Op2);
-  VPWidenRecipe WidenR(*AI, make_range(Args.begin(), Args.end()));
+  VPWidenRecipe WidenR(*AI, make_range(Args.begin(), Args.end()), {});


ayalz · 2025-04-21T20:50:21Z

llvm/lib/Transforms/Vectorize/VPlan.h

+
+public:
+  /// Add all metadata to \p V if it is an instruction.
+  void setMetadata(Value *V) const;


Thanks for pushing applyFlags() separately!

ayalz · 2025-04-21T20:52:51Z

llvm/lib/Transforms/Vectorize/VPlan.h

+  /// Add all metadata to \p V if it is an instruction.
+  void applyMetadata(Value *V) const;


Yeah, suggestion was inspired by setFlags(Instruction*).

ayalz · 2025-04-21T20:56:34Z

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

@@ -2755,7 +2758,7 @@ void VPlanTransforms::narrowInterleaveGroups(VPlan &Plan, ElementCount VF,
      auto *L = new VPWidenLoadRecipe(
          *cast<LoadInst>(LoadGroup->getInterleaveGroup()->getInsertPos()),
          LoadGroup->getAddr(), LoadGroup->getMask(), /*Consecutive=*/true,
-          /*Reverse=*/false, LoadGroup->getDebugLoc());
+          /*Reverse=*/false, {}, LoadGroup->getDebugLoc());


This change is no longer needed?

david-arm · 2025-04-22T11:13:49Z

Hi @fhahn, should the title of the ticket be "[VPlan] Manage instruction metadata in VPlan."?

ayalz · 2025-04-22T13:11:15Z

llvm/lib/Transforms/Vectorize/VPlan.cpp

+void VPTransformState::addNewMetadata(Value *To, const Instruction *Orig) {
+


Is this change still needed, now that metadata is applied to Instructions rather than Values?

Restored thanks

ayalz · 2025-04-22T13:20:17Z

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -1190,18 +1190,33 @@ struct VPIRPhi : public VPIRInstruction {
 #endif
 };

+/// Helper to manage IR metadata for recipes. It filters out metadata that
+/// cannot be proagated.


Suggested change

/// cannot be proagated.

/// cannot be propagated.

Fixed thanks

ayalz · 2025-04-22T13:27:02Z

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -2626,8 +2652,8 @@ class VPWidenMemoryRecipe : public VPRecipeBase {
  VPWidenMemoryRecipe(const char unsigned SC, Instruction &I,
                      std::initializer_list<VPValue *> Operands,
                      bool Consecutive, bool Reverse, DebugLoc DL)
-      : VPRecipeBase(SC, Operands, DL), Ingredient(I), Consecutive(Consecutive),
-        Reverse(Reverse) {
+      : VPRecipeBase(SC, Operands, DL), VPIRMetadata(I), Ingredient(I),


Certainly, by all means!
Any recipe that can be constructed w/o an underlying instruction should be; at that point its metadata can be passed instead of the instruction itself.

ayalz · 2025-04-22T13:33:50Z

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

+  if (auto *I = dyn_cast<Instruction>(Sel)) {
+    if (isa<FPMathOperator>(I))
+      applyFlags(*I);
+    applyMetadata(*I);
+  }


Independent, consistency nit: good to keep State.set() as the last operation of execute(), after fully finishing to build the new instruction/value, including applying its flags/metadata. Conceptually, flags and metadata could be folded into Builder.CreateSelect().

Can do separately, thanks.

ayalz · 2025-04-22T13:34:52Z

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

-  if (auto *CastOp = dyn_cast<Instruction>(Cast))
+  if (auto *CastOp = dyn_cast<Instruction>(Cast)) {
    applyFlags(*CastOp);
+    applyMetadata(*CastOp);
+  }


Same independent consistency nit.

ayalz · 2025-04-22T13:44:42Z

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

+  State.set(this,
+            Reverse ? Builder.CreateVectorReverse(NewLI, "reverse") : NewLI);


This change is to avoid casting the vector reverse of NewLI to Instruction:

if (Reverse) NewLI = cast<Instruction>(Builder.CreateVectorReverse(NewLI, "reverse")); State.set(this, NewLI);

?

Yes, otherwise we need to cast, could also change if desired

Yes, keeping State.set() separate as the last operation after completing the creation of instructions would be more consistent, e.g., also with VPWidenLoadEVLRecipe::execute() below. BTW, better ask here if (isReverse()) as asked there and above.

restored thanks

ayalz · 2025-04-22T13:49:19Z

llvm/unittests/Transforms/Vectorize/VPlanTest.cpp

@@ -950,7 +950,7 @@ TEST_F(VPRecipeTest, CastVPWidenCallRecipeToVPUserAndVPDef) {
  Args.push_back(Op1);
  Args.push_back(Op2);
  Args.push_back(CalledFn);
-  VPWidenCallRecipe Recipe(Call, Fn, Args);
+  VPWidenCallRecipe Recipe(Call, Fn, Args, {});


Is this change needed, here and below, given that the last DL argument defaults to {}?

Removed, thanks

fhahn

Hi @fhahn, should the title of the ticket be "[VPlan] Manage instruction metadata in VPlan."?

Yep, the typo should be fixed thanks!

fhahn · 2025-04-22T14:39:38Z

llvm/lib/Transforms/Vectorize/VPlan.cpp

+void VPTransformState::addNewMetadata(Value *To, const Instruction *Orig) {
+


Restored thanks

fhahn · 2025-04-22T15:07:54Z

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -1190,18 +1190,33 @@ struct VPIRPhi : public VPIRInstruction {
 #endif
 };

+/// Helper to manage IR metadata for recipes. It filters out metadata that
+/// cannot be proagated.


Fixed thanks

fhahn · 2025-04-22T15:08:46Z

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

+  if (auto *I = dyn_cast<Instruction>(Sel)) {
+    if (isa<FPMathOperator>(I))
+      applyFlags(*I);
+    applyMetadata(*I);
+  }


Can do separately, thanks.

fhahn · 2025-04-22T15:09:24Z

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

+  State.set(this,
+            Reverse ? Builder.CreateVectorReverse(NewLI, "reverse") : NewLI);


Yes, otherwise we need to cast, could also change if desired

fhahn · 2025-04-22T15:09:59Z

llvm/unittests/Transforms/Vectorize/VPlanTest.cpp

@@ -950,7 +950,7 @@ TEST_F(VPRecipeTest, CastVPWidenCallRecipeToVPUserAndVPDef) {
  Args.push_back(Op1);
  Args.push_back(Op2);
  Args.push_back(CalledFn);
-  VPWidenCallRecipe Recipe(Call, Fn, Args);
+  VPWidenCallRecipe Recipe(Call, Fn, Args, {});


Removed, thanks

ayalz

LGTM, thanks! Adding few minor comments.

ayalz · 2025-04-23T19:46:28Z

llvm/unittests/Transforms/Vectorize/VPlanTest.cpp

@@ -1257,7 +1257,7 @@ TEST_F(VPRecipeTest, MayHaveSideEffectsAndMayReadWriteMemory) {
    Args.push_back(Op1);
    Args.push_back(Op2);
    Args.push_back(CalledFn);
-    VPWidenCallRecipe Recipe(Call, TheFn, Args);
+    VPWidenCallRecipe Recipe(Call, TheFn, Args, {});


Is this change needed?

Ah yes removed, thanks

ayalz · 2025-04-23T19:50:42Z

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

+  State.set(this,
+            Reverse ? Builder.CreateVectorReverse(NewLI, "reverse") : NewLI);


Yes, keeping State.set() separate as the last operation after completing the creation of instructions would be more consistent, e.g., also with VPWidenLoadEVLRecipe::execute() below. BTW, better ask here if (isReverse()) as asked there and above.

ayalz · 2025-04-23T21:38:28Z

llvm/lib/Transforms/Vectorize/VPlanHelpers.h

@@ -290,13 +290,6 @@ struct VPTransformState {
  /// vector loop.


Is the above description still accurate? Would be good to clarify when State.addNewMetadata(To, Orig) needs to complement applyMetadata(To). Will possibly be handled by #136450

I think this is still accurate; applyMetadata in this patch applies metadata already on Orig, while addNewMetadata adds metadata from versioning, not present at the original instructions.

Use VPIRMetadata added in llvm#135272 to also manage no-alias metadata added by versioning. Note that this means we have to build the no-alias metadata up-front once. If it is not used, it will be discarded automatically.

Use VPIRMetadata added in llvm#135272 to also manage no-alias metadata added by versioning. Note that this means we have to build the no-alias metadata up-front once. If it is not used, it will be discarded automatically. !fixup move applyMetadata; !fixup address comments, thanks !fixup address remaining comments, thanks !fixup address latest comments, thanks

Update name to apply flags to instructions, as suggested in llvm/llvm-project#135272. Also changes the arg to a reference.

Add a new helper to manage IR metadata that can be progated to generated instructions for recipes. This helps to remove a number of remaining uses of getUnderlyingInstr during VPlan execution. PR: llvm/llvm-project#135272

Update name to apply flags to instructions, as suggested in llvm#135272. Also changes the arg to a reference.

Add a new helper to manage IR metadata that can be progated to generated instructions for recipes. This helps to remove a number of remaining uses of getUnderlyingInstr during VPlan execution. PR: llvm#135272

Update name to apply flags to instructions, as suggested in llvm#135272. Also changes the arg to a reference.

Add a new helper to manage IR metadata that can be progated to generated instructions for recipes. This helps to remove a number of remaining uses of getUnderlyingInstr during VPlan execution. PR: llvm#135272

Update name to apply flags to instructions, as suggested in llvm#135272. Also changes the arg to a reference.

Add a new helper to manage IR metadata that can be progated to generated instructions for recipes. This helps to remove a number of remaining uses of getUnderlyingInstr during VPlan execution. PR: llvm#135272

Use VPIRMetadata added in llvm#135272 to also manage no-alias metadata added by versioning. Note that this means we have to build the no-alias metadata up-front once. If it is not used, it will be discarded automatically. !fixup move applyMetadata; !fixup address comments, thanks !fixup address remaining comments, thanks !fixup address latest comments, thanks

Add a new helper to manage IR metadata that can be progated to generated instructions for recipes. This helps to remove a number of remaining uses of getUnderlyingInstr during VPlan execution. PR: llvm#135272

Use VPIRMetadata added in llvm#135272 to also manage no-alias metadata added by versioning. Note that this means we have to build the no-alias metadata up-front once. If it is not used, it will be discarded automatically. !fixup move applyMetadata; !fixup address comments, thanks !fixup address remaining comments, thanks !fixup address latest comments, thanks

Use VPIRMetadata added in #135272 to also manage no-alias metadata added by versioning. Note that this means we have to build the no-alias metadata up-front once. If it is not used, it will be discarded automatically. This also fixes a case where incorrect metadata was added to wide loads/stores that got converted from an interleave group. Compile-time impact is neutral: https://llvm-compile-time-tracker.com/compare.php?from=38bf1af41c5425a552a53feb13c71d82873f1c18&to=2fd7844cfdf5ec0f1c2ce0b9b3ae0763245b6922&stat=instructions:u

…6450) Use VPIRMetadata added in llvm/llvm-project#135272 to also manage no-alias metadata added by versioning. Note that this means we have to build the no-alias metadata up-front once. If it is not used, it will be discarded automatically. This also fixes a case where incorrect metadata was added to wide loads/stores that got converted from an interleave group. Compile-time impact is neutral: https://llvm-compile-time-tracker.com/compare.php?from=38bf1af41c5425a552a53feb13c71d82873f1c18&to=2fd7844cfdf5ec0f1c2ce0b9b3ae0763245b6922&stat=instructions:u

fhahn requested review from rengolin, LiqinWeng, ayalz and aniragil April 10, 2025 22:23

llvmbot added vectorizers llvm:transforms labels Apr 10, 2025

fhahn mentioned this pull request Apr 10, 2025

[LV] Fix missing MetaData for histogram instructions #134241

Open

fhahn force-pushed the vplan-irmetadata branch 3 times, most recently from c45aa04 to b57fc68 Compare April 17, 2025 13:27

ayalz reviewed Apr 18, 2025

View reviewed changes

fhahn added 2 commits April 19, 2025 20:13

[VPlan] Manage instruction medata in VPlan.

3760c4f

Add a new helper to manage IR metadata that can be progated to generated instructions for recipes. This helps to remove a number of remaining uses of getUnderlyingInstr during VPlan execution. Add metadata on cloning.

!fixup address latest comments, thanks

d3edd89

fhahn force-pushed the vplan-irmetadata branch from b57fc68 to d3edd89 Compare April 19, 2025 19:16

fhahn mentioned this pull request Apr 19, 2025

[VPlan] Manage noalias/alias_scope metadata in VPlan. #136450

Merged

ayalz reviewed Apr 20, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main' into vplan-irmetadata

980a939

fhahn added a commit that referenced this pull request Apr 21, 2025

[VPlan] Rename setFlags -> applyFlags (NFC).

3e5a9d9

Update name to apply flags to instructions, as suggested in #135272. Also changes the arg to a reference.

fhahn added 2 commits April 21, 2025 19:00

!fixup address latest comments, thanks

7ad19f7

Merge remote-tracking branch 'origin/main' into vplan-irmetadata

f9d1fb5

ayalz reviewed Apr 21, 2025

View reviewed changes

fhahn added 2 commits April 22, 2025 10:49

Merge remote-tracking branch 'origin/main' into vplan-irmetadata

6c8ef1f

!fixup address latest comments, thanks

ef9bcc9

ayalz reviewed Apr 22, 2025

View reviewed changes

fhahn added 2 commits April 22, 2025 14:59

Merge remote-tracking branch 'origin/main' into vplan-irmetadata

681bb63

!fixup address latest comments, thanks!

14ee2b1

fhahn changed the title ~~[VPlan] Manage instruction medata in VPlan.~~ [VPlan] Manage instruction metadata in VPlan. Apr 22, 2025

fhahn commented Apr 22, 2025

View reviewed changes

ayalz approved these changes Apr 23, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main' into vplan-irmetadata

923336d

fhahn merged commit 5d136f9 into llvm:main Apr 24, 2025
11 checks passed

fhahn deleted the vplan-irmetadata branch April 24, 2025 10:58

llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request May 6, 2025

Automerge: [VPlan] Rename setFlags -> applyFlags (NFC).

c44df8d

Update name to apply flags to instructions, as suggested in llvm/llvm-project#135272. Also changes the arg to a reference.

IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025

[VPlan] Rename setFlags -> applyFlags (NFC).

d446141

Update name to apply flags to instructions, as suggested in llvm#135272. Also changes the arg to a reference.

IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025

[VPlan] Rename setFlags -> applyFlags (NFC).

53aaeaf

Update name to apply flags to instructions, as suggested in llvm#135272. Also changes the arg to a reference.

IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025

[VPlan] Rename setFlags -> applyFlags (NFC).

f1d88d5

Update name to apply flags to instructions, as suggested in llvm#135272. Also changes the arg to a reference.

		@@ -1646,7 +1654,7 @@ void VPWidenRecipe::execute(VPTransformState &State) {

		Intrinsic::vp_select, Ops, TypeInfo.inferScalarType(Sel),
		Sel->getMetadata(), Sel->getDebugLoc());

		/// Add all metadata to \p V if it is an instruction.
		void applyMetadata(Value *V) const;

		void VPTransformState::addNewMetadata(Value To, const Instruction Orig) {

		State.set(this,
		Reverse ? Builder.CreateVectorReverse(NewLI, "reverse") : NewLI);

		@@ -290,13 +290,6 @@ struct VPTransformState {
		/// vector loop.

[VPlan] Manage instruction metadata in VPlan. #135272

[VPlan] Manage instruction metadata in VPlan. #135272

Uh oh!

Conversation

fhahn commented Apr 10, 2025

Uh oh!

llvmbot commented Apr 10, 2025

Uh oh!

llvmbot commented Apr 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!