[PATCH] D144125: [VPlan] VPWidenIntOrFpInductionRecipe inherits from VPHeaderPHIRecipe
Michael Maitland via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 13 08:08:10 PDT 2023
michaelmaitland marked 2 inline comments as done.
michaelmaitland added inline comments.
================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1142-1143
+ /// Returns the step value of the induction.
+ VPValue *getStepValue() { return getOperand(1); }
+ const VPValue *getStepValue() const { return getOperand(1); }
+
----------------
Ayal wrote:
> Now that `VPWidenIntOrFpInductionRecipe` inherits from `VPHeaderPHIRecipe`, it also inherits `getBackedgeValue()`, which should return a VPValue defined by some recipe - `getBackedgeRecipe()` - **inside** the loop. The default returns `getOperand(1)`. However, `VPWidenIntOrFpInductionRecipe` has no such VPValue nor recipe to return because it takes care of generating the corresponding IR code only during execution. Better override `getBackedgeValue()` and `getBackedgeRecipe()`, asserting they are unreachable/never called?
> (`Step` which here serves as operand 1 instead is a value defined by a recipe in the loop preheader or constant - not a recipe.)
I'm happy to add the overridden methods with unreachables in them for now. However, I think in the future it would be nice if a recipe extends another recipe, all operands of the base recipe must exist and be at the same index in the parent recipe. If not all derived versions of HeaderPHIRecipes have backedge values and recipes, then it should not belong in the base class.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144125/new/
https://reviews.llvm.org/D144125
More information about the llvm-commits
mailing list