[VPlan] Remove VPWidenRecipe constructor with no underlying instruction. NFCI

[lukel97]

My understanding is that a VPWidenRecipe should be used for recipes with an exact underlying scalar instruction, and VPInstruction should be used elsewhere e.g. for instructions generated as a part of the vectorization process.

The only user of the VPWidenRecipe constructor that doesn't take an underlying instruction is in adjustRecipesForReductions, but we can just use VPInstruction there.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-vectorizers

Author: Luke Lau (lukel97)

Changes

My understanding is that a VPWidenRecipe should be used for recipes with an exact underlying scalar instruction, and VPInstruction should be used elsewhere e.g. for instructions generated as a part of the vectorization process.

The only user of the VPWidenRecipe constructor that doesn't take an underlying instruction is in adjustRecipesForReductions, but we can just use VPInstruction there.

---

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

2 Files Affected:

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

diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index e5c3f17860103..c19d348ff1205 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -8641,9 +8641,9 @@ void LoopVectorizationPlanner::adjustRecipesForReductions(
                  CurrentLinkI->getOpcode() == Instruction::Sub) {
         Type *PhiTy = PhiR->getUnderlyingValue()->getType();
         auto *Zero = Plan->getConstantInt(PhiTy, 0);
-        VPWidenRecipe *Sub = new VPWidenRecipe(
-            Instruction::Sub, {Zero, CurrentLink->getOperand(1)}, {},
-            VPIRMetadata(), CurrentLinkI->getDebugLoc());
+        auto *Sub = new VPInstruction(Instruction::Sub,
+                                      {Zero, CurrentLink->getOperand(1)},
+                                      CurrentLinkI->getDebugLoc());
         Sub->setUnderlyingValue(CurrentLinkI);
         LinkVPBB->insert(Sub, CurrentLink->getIterator());
         VecOp = Sub;
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index cfe1f1e9d7528..0d51ea412acd1 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1447,12 +1447,6 @@ class LLVM_ABI_FOR_TEST VPWidenRecipe : public VPRecipeWithIRFlags,
   unsigned Opcode;
 
 public:
-  VPWidenRecipe(unsigned Opcode, ArrayRef<VPValue *> Operands,
-                const VPIRFlags &Flags, const VPIRMetadata &Metadata,
-                DebugLoc DL)
-      : VPRecipeWithIRFlags(VPDef::VPWidenSC, Operands, Flags, DL),
-        VPIRMetadata(Metadata), Opcode(Opcode) {}
-
   VPWidenRecipe(Instruction &I, ArrayRef<VPValue *> Operands)
       : VPRecipeWithIRFlags(VPDef::VPWidenSC, Operands, I), VPIRMetadata(I),
         Opcode(I.getOpcode()) {}
@@ -1460,9 +1454,8 @@ class LLVM_ABI_FOR_TEST VPWidenRecipe : public VPRecipeWithIRFlags,
   ~VPWidenRecipe() override = default;
 
   VPWidenRecipe *clone() override {
-    auto *R =
-        new VPWidenRecipe(getOpcode(), operands(), *this, *this, getDebugLoc());
-    R->setUnderlyingValue(getUnderlyingValue());
+    auto *R = new VPWidenRecipe(*getUnderlyingInstr(), operands());
+    R->transferFlags(*this);
     return R;
   }
 

[lukel97]

I did a quick check and the metadata doesn't ever seem to change from the underlying instruction. So I think it's fine to just let the Instruction constructor set the VPIRMetadata. The flags change however so we need to transfer that.

[fhahn]

Not now perhaps, but relying on that seems very dangerous and can easily break in the future and lead to subtle mis-compiles (e.g. we may have to remove metadata when hoisting)

[lukel97]

I've transferred the metadata over in 1a6bec9. I think VPWidenIntrinsicRecipe/VPWidenCallRecipe/VPWidenSelectRecipe seem to have the same issue, which would be good to fix in another PR

[fhahn]

I guess passing via the constructor would require more changes, right? if so the current approach is fine. I am currently working on propagating the metadata from initial VPInstructions, which should hopefully unify this soon

[artagnon]

Hm, can we also attempt to de-duplicate the opcodes between VPWidenRecipe and VPInstruction, as a follow-up? LGTM, thanks!