[llvm] [LV] Support strided load with a stride of -1 (PR #128718)
Mel Chen via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 7 03:05:04 PDT 2025
================
@@ -8324,16 +8398,27 @@ VPRecipeBuilder::tryToWidenMemory(Instruction *I, ArrayRef<VPValue *> Operands,
// reverse consecutive.
LoopVectorizationCostModel::InstWidening Decision =
CM.getWideningDecision(I, Range.Start);
+
+ auto SameWiden = [&](ElementCount VF) -> bool {
+ return Decision == CM.getWideningDecision(I, VF);
+ };
+ bool ContainsWidenVF =
+ LoopVectorizationPlanner::getDecisionAndClampRange(SameWiden, Range);
+ assert(ContainsWidenVF &&
+ "At least widen the memory accesses by the Start VF.");
+
----------------
Mel-Chen wrote:
> Just to check my understanding, with this patch do we now have different VFs with different widening decisions that now need pruned? E.g. CM_Strided for VF=1, CM_WidenReverse for VF=2? Was this an issue beforehand?
Yes, currently, when the VF is smaller, the cost model prefers CM_Strided, while for larger VFs, it favors CM_WidenReverse. This leads to a mismatch between the legacy cost model and the VPlan-based cost model, which is a situation we haven't encountered before.
> Also is this assert still needed? I think this is only checking `CM.getWideningDecision(I, Range.Start) == CM.getWideningDecision(I, Range.Start)`
I'm also torn about whether we should keep this assertion. ContainsWidenVF is theoretically supposed to be true; if it's not, we've definitely broken something. :(
https://github.com/llvm/llvm-project/pull/128718
More information about the llvm-commits
mailing list