[PATCH] D110479: [LV] Record memory widening decisions (NFCI)

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2920
   // Determine if the pointer operand of the access is either consecutive or
   // reverse consecutive.
+  bool CreateGatherScatter = !ConsecutiveStride;
----------------
Update above comment?  A gather/scatter will be created iff the stride is not consecutive, going either forward or reverse. This includes non-unit strided accesses, which may be part of interleave groups, and non strided accesses.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2927
          "The instruction should be scalarized");
   (void)ConsecutiveStride;
 
----------------
Above comment, assert, and artificial use of ConsecutiveStride are now redundant; cleanup?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1534
+      : VPRecipeBase(VPWidenMemoryInstructionSC, {Addr}), Ingredient(Load),
+        Consecutive(Consecutive), Reverse(Reverse) {
     new VPValue(VPValue::VPVMemoryInstructionSC, &Load, this);
----------------
assert that Reverse implies Consecutive?

Another alternative may be to designate one recipe for Gather/Scatter with arbitrary non-unit strides, and a separate possibly derived recipe for Consecutive/VectorMemoryInstructions, with a Reverse indicator?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1543
       : VPRecipeBase(VPWidenMemoryInstructionSC, {Addr, StoredValue}),
-        Ingredient(Store) {
+        Ingredient(Store), Consecutive(Consecutive), Reverse(Reverse) {
     setMask(Mask);
----------------
ditto


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1576
+
+  // Return whether the loaded/stored addresses are in reverse order.
+  bool isReverse() const { return Reverse; }
----------------
are consecutive and in reverse order


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110479/new/

https://reviews.llvm.org/D110479