[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