[VPlan] Refine the debug location passing for VPWidenIntrinsicRecipe

[Mel-Chen]

No description provided.

[llvmbot]

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: Mel Chen (Mel-Chen)

Changes

---

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

3 Files Affected:

• (modified) llvm/lib/Transforms/Vectorize/LoopVectorize.cpp (+1-2)
• (modified) llvm/lib/Transforms/Vectorize/VPlan.h (+3-4)
• (modified) llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp (+1-2)

diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 88086f24dfdce2..a370a8b728be27 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -8430,8 +8430,7 @@ VPSingleDefRecipe *VPRecipeBuilder::tryToWidenCall(CallInst *CI,
                 },
                 Range);
   if (ShouldUseVectorIntrinsic)
-    return new VPWidenIntrinsicRecipe(*CI, ID, Ops, CI->getType(),
-                                      CI->getDebugLoc());
+    return new VPWidenIntrinsicRecipe(*CI, ID, Ops, CI->getType());
 
   Function *Variant = nullptr;
   std::optional<unsigned> MaskPos;
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index a34e34a0d71f1e..1b8813c76d2682 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1678,8 +1678,7 @@ class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags {
 
 public:
   VPWidenIntrinsicRecipe(CallInst &CI, Intrinsic::ID VectorIntrinsicID,
-                         ArrayRef<VPValue *> CallArguments, Type *Ty,
-                         DebugLoc DL = {})
+                         ArrayRef<VPValue *> CallArguments, Type *Ty)
       : VPRecipeWithIRFlags(VPDef::VPWidenIntrinsicSC, CallArguments, CI),
         VectorIntrinsicID(VectorIntrinsicID), ResultTy(Ty),
         MayReadFromMemory(CI.mayReadFromMemory()),
@@ -1690,7 +1689,7 @@ class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags {
                          ArrayRef<VPValue *> CallArguments, Type *Ty,
                          bool MayReadFromMemory, bool MayWriteToMemory,
                          bool MayHaveSideEffects, DebugLoc DL = {})
-      : VPRecipeWithIRFlags(VPDef::VPWidenIntrinsicSC, CallArguments),
+      : VPRecipeWithIRFlags(VPDef::VPWidenIntrinsicSC, CallArguments, DL),
         VectorIntrinsicID(VectorIntrinsicID), ResultTy(Ty),
         MayReadFromMemory(MayReadFromMemory),
         MayWriteToMemory(MayWriteToMemory),
@@ -1701,7 +1700,7 @@ class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags {
   VPWidenIntrinsicRecipe *clone() override {
     return new VPWidenIntrinsicRecipe(*cast<CallInst>(getUnderlyingValue()),
                                       VectorIntrinsicID, {op_begin(), op_end()},
-                                      ResultTy, getDebugLoc());
+                                      ResultTy);
   }
 
   VP_CLASSOF_IMPL(VPDef::VPWidenIntrinsicSC)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index 03c4110761ac6a..0aa5fbe5cabe84 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -84,8 +84,7 @@ void VPlanTransforms::VPInstructionsToVPRecipes(
         } else if (CallInst *CI = dyn_cast<CallInst>(Inst)) {
           NewRecipe = new VPWidenIntrinsicRecipe(
               *CI, getVectorIntrinsicIDForCall(CI, &TLI),
-              {Ingredient.op_begin(), Ingredient.op_end() - 1}, CI->getType(),
-              CI->getDebugLoc());
+              {Ingredient.op_begin(), Ingredient.op_end() - 1}, CI->getType());
         } else if (SelectInst *SI = dyn_cast<SelectInst>(Inst)) {
           NewRecipe = new VPWidenSelectRecipe(*SI, Ingredient.operands());
         } else if (auto *CI = dyn_cast<CastInst>(Inst)) {

[alexey-bataev]

Why did you decide to drop debug locations?

[Mel-Chen]

Actually we didn't lose the debug locations.
Constructor VPWidenIntrinsicRecipe(CallInst &, Intrinsic::ID, ArrayRef<VPValue *>, Type *) will call constructor VPRecipeWithIRFlags(const unsigned char, ArrayRef<VPValue *>, Instruction &).

  template <typename IterT>
  VPRecipeWithIRFlags(const unsigned char SC, IterT Operands, Instruction &I)
      : VPSingleDefRecipe(SC, Operands, &I, I.getDebugLoc()) {
    if (auto *Op = dyn_cast<CmpInst>(&I)) {
    ...

The debug locations can be get from underlying instruction. That is why we can drop it.

[Mel-Chen]

ping

[david-arm]

LGTM! This seems like a sensible clean-up. The constructor variant taking the original instruction can use the debug location from that and the other variant was dropping the debug info, whereas now it's actually using it.

[fhahn]

not directly related to this PR, but across the function we should probably use the debug loc of the VPInstruction

[fhahn]

does this fixe cases where the debug location wasn't passed before? Does this mean a test is missing?