[PATCH] D147783: [VPlan] Add stride->constant VPlan mapping at construction.

Sun Jun 11 15:28:09 PDT 2023

fhahn marked 4 inline comments as done.
fhahn added inline comments.

================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8897
+    else
+      Plan->addVPValue(StrideV, new VPValue(CI));
+  }
----------------
Ayal wrote:
> This is essentially short-circuiting an RAUW. When VPlan will take care of the check for unit stride, only users inside the vector loop (, its preheader, middle block, and epilog vector loop?) will need to be replaced, the compare (and scalar loop) should continue to use the original stride of-course.
> Worth setting it up as an RAUW of original stride VPValue with "1" VPValue, after all users of the former have been set up?
Updated to do RAUW which means the `hasVPValueFor` helper isn't needed any longer. This required an update to the VPlan transform optimizing inductions to use the existing step, instead of creating a new one: df357a71dd7d10ee4bdc0bd22e6be048c5ad7088

================
Comment at: llvm/test/Transforms/LoopVectorize/RISCV/strided-accesses.ll:236
 ; NOSTRIDED-NEXT:    [[TMP5:%.*]] = add i64 [[INDEX]], 0
-; NOSTRIDED-NEXT:    [[TMP6:%.*]] = mul nuw nsw i64 [[TMP5]], [[STRIDE]]
+; NOSTRIDED-NEXT:    [[TMP6:%.*]] = mul nuw nsw i64 [[TMP5]], 1
 ; NOSTRIDED-NEXT:    [[TMP7:%.*]] = getelementptr i32, ptr [[P:%.*]], i64 [[TMP6]]
----------------
Ayal wrote:
> reames wrote:
> > Completely unrelated to this, but it looks like we've got opportunity to fold these multiplies now.  Is this coming from an IRBuilder or a SCEVExpander?  If the former, we may just need to adjust the folder at construction.  (Assuming that eager folding doesn't interact badly with something inside LV/FPlan - which you'll have a much better idea than I do.)
> Yes, there's room for some constant folding here, and also the add 0. Would be good to ensure LV's cost model for such predicated unit stride accesses is accurate, and potentially apply these foldings in VPlan, independent of foldings at IR generation.
The redundant `add` comes from recipe expansion. I put up D152660 and D152659 to simplify during IR construction, as there are multiple places where this should benefit. It would also make sense to do this based on recipes on the future.

================
Comment at: llvm/test/Transforms/LoopVectorize/RISCV/strided-accesses.ll:316
 ; NOSTRIDED-NEXT:    [[N_VEC:%.*]] = sub i64 1024, [[N_MOD_VF]]
 ; NOSTRIDED-NEXT:    [[IND_END:%.*]] = mul i64 [[N_VEC]], [[STRIDE]]
 ; NOSTRIDED-NEXT:    br label [[VECTOR_BODY:%.*]]
----------------
Ayal wrote:
> nit: such appearances of STRIDE in vector.ph should also be removed once Induction Resume Values are created via recipes, right?
yes exactly.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147783