[LV]: Add costs to VPInstruction

[hassnaaHamdi]

• In some cases VPlanTransforms pass replaces some instructions by VPInstructions.
• Right now VPInstruction returns 0 cost, and that causes inaccurate VPlan-based cost
  calculations which may cause choosing VF different than chosen one by legacy cost model.

[llvmbot]

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: Hassnaa Hamdi (hassnaaHamdi)

Changes

• As VPlanTransforms replaces some instructions by VPInstructions, then VPInstruction should have costs.
• Use legacy model for now.
• Computed costs are for the original/replaced instructions to match with what legacy model computed.

---

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

1 Files Affected:

• (modified) llvm/lib/Transforms/Vectorize/VPlan.h (+3)

diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index a1ff684b2b8017..8813ea3c208b61 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1315,6 +1315,9 @@ class VPInstruction : public VPRecipeWithIRFlags,
   InstructionCost computeCost(ElementCount VF,
                               VPCostContext &Ctx) const override {
     // TODO: Compute accurate cost after retiring the legacy cost model.
+    // Use legacy cost model for now.
+    if (auto *I = dyn_cast_or_null<Instruction>(getUnderlyingValue()))
+      return Ctx.getLegacyCost(I, VF);
     return 0;
   }
 

[david-arm]

Hi @hassnaaHamdi, I think you should be able to do this without falling back on the legacy cost model. I also plan to put a patch up fairly soon that starts filling this function in properly because it's needed for the early exit work. What instructions were you hitting when getting into this instruction?

[hassnaaHamdi]

The original instruction was 'or disjoint' and it got replaced by the VPlanTransforms to 'Add' instruction wrapped in VPInstruction.

[fhahn]

Could you please add a test case, some more details of what VPInstructions need costs now and how this is related to the change for interleaving?

[hassnaaHamdi]

Closing this pull request as the work is done by another patch.