[VPlan] Passing non-null instruction when creating VPReductionRecipe in unit test. NFC

[ElvisWang123]

No description provided.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Elvis Wang (ElvisWang123)

Changes

---

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

1 Files Affected:

• (modified) llvm/unittests/Transforms/Vectorize/VPlanTest.cpp (+18-4)

diff --git a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
index 3179cfc676ab67..4108b954d13412 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
@@ -1165,22 +1165,27 @@ TEST(VPRecipeTest, MayHaveSideEffectsAndMayReadWriteMemory) {
   }
 
   {
+    auto *Add = BinaryOperator::CreateAdd(PoisonValue::get(Int32),
+                                          PoisonValue::get(Int32));
     VPValue ChainOp;
     VPValue VecOp;
     VPValue CondOp;
-    VPReductionRecipe Recipe(RecurrenceDescriptor(), nullptr, &ChainOp, &CondOp,
+    VPReductionRecipe Recipe(RecurrenceDescriptor(), Add, &ChainOp, &CondOp,
                              &VecOp, false);
     EXPECT_FALSE(Recipe.mayHaveSideEffects());
     EXPECT_FALSE(Recipe.mayReadFromMemory());
     EXPECT_FALSE(Recipe.mayWriteToMemory());
     EXPECT_FALSE(Recipe.mayReadOrWriteMemory());
+    delete Add;
   }
 
   {
+    auto *Add = BinaryOperator::CreateAdd(PoisonValue::get(Int32),
+                                          PoisonValue::get(Int32));
     VPValue ChainOp;
     VPValue VecOp;
     VPValue CondOp;
-    VPReductionRecipe Recipe(RecurrenceDescriptor(), nullptr, &ChainOp, &CondOp,
+    VPReductionRecipe Recipe(RecurrenceDescriptor(), Add, &ChainOp, &CondOp,
                              &VecOp, false);
     VPValue EVL;
     VPReductionEVLRecipe EVLRecipe(Recipe, EVL, &CondOp);
@@ -1188,6 +1193,7 @@ TEST(VPRecipeTest, MayHaveSideEffectsAndMayReadWriteMemory) {
     EXPECT_FALSE(EVLRecipe.mayReadFromMemory());
     EXPECT_FALSE(EVLRecipe.mayWriteToMemory());
     EXPECT_FALSE(EVLRecipe.mayReadOrWriteMemory());
+    delete Add;
   }
 
   {
@@ -1540,30 +1546,38 @@ TEST(VPRecipeTest, dumpRecipeUnnamedVPValuesNotInPlanOrBlock) {
 
 TEST(VPRecipeTest, CastVPReductionRecipeToVPUser) {
   LLVMContext C;
+  IntegerType *Int32 = IntegerType::get(C, 32);
+  auto *Add = BinaryOperator::CreateAdd(PoisonValue::get(Int32),
+                                        PoisonValue::get(Int32));
 
   VPValue ChainOp;
   VPValue VecOp;
   VPValue CondOp;
-  VPReductionRecipe Recipe(RecurrenceDescriptor(), nullptr, &ChainOp, &CondOp,
+  VPReductionRecipe Recipe(RecurrenceDescriptor(), Add, &ChainOp, &CondOp,
                            &VecOp, false);
   EXPECT_TRUE(isa<VPUser>(&Recipe));
   VPRecipeBase *BaseR = &Recipe;
   EXPECT_TRUE(isa<VPUser>(BaseR));
+  delete Add;
 }
 
 TEST(VPRecipeTest, CastVPReductionEVLRecipeToVPUser) {
   LLVMContext C;
+  IntegerType *Int32 = IntegerType::get(C, 32);
+  auto *Add = BinaryOperator::CreateAdd(PoisonValue::get(Int32),
+                                        PoisonValue::get(Int32));
 
   VPValue ChainOp;
   VPValue VecOp;
   VPValue CondOp;
-  VPReductionRecipe Recipe(RecurrenceDescriptor(), nullptr, &ChainOp, &CondOp,
+  VPReductionRecipe Recipe(RecurrenceDescriptor(), Add, &ChainOp, &CondOp,
                            &VecOp, false);
   VPValue EVL;
   VPReductionEVLRecipe EVLRecipe(Recipe, EVL, &CondOp);
   EXPECT_TRUE(isa<VPUser>(&EVLRecipe));
   VPRecipeBase *BaseR = &EVLRecipe;
   EXPECT_TRUE(isa<VPUser>(BaseR));
+  delete Add;
 }
 
 struct VPDoubleValueDef : public VPRecipeBase {

[ElvisWang123]

I think we need to pass non-null underlying instruction when creating VPReductionRecipe in current implementation.
Otherwise the getUnderlyingInstr() which is used in print() and clone() will crash.

Split from #120054.

We can add another constructor for VPReductionRecipe without underlying instruction in the future.

[alexey-bataev]

LG

[fhahn]

After recent changes to #120054 IIUC the patch won't be needed any longer?

[ElvisWang123]

After recent changes to #120054 IIUC the patch won't be needed any longer?

Yes, not needed but good to have.
Current structure of VPReductionRecipe is based on assumption of non-null underlying instruction. (print() and clone())
Testing with nullptr seems not that appropriate.

An alternative implementation is that we could remove all underlying instruction dependency in VPReductionRecipe and not modify the unit test.

[ElvisWang123]

Closed since ed19620 already contains this changes.