[PATCH] D154714: LAA: handle GEPs with >2 operands in findForkedSCEVs()

Fri Jul 7 08:14:54 PDT 2023

nikic added inline comments.

================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/forked-pointers.ll:987
+  %1 = or i64 %indvars.iv, 1
+  %arrayidx = getelementptr inbounds [2048 x i8], ptr %0, i64 0, i64 %indvars.iv
+  store i8 3, ptr %arrayidx
----------------
artagnon wrote:
> nikic wrote:
> > artagnon wrote:
> > > fhahn wrote:
> > > > I'm not sure if/how this fits into the forked pointer handling. The forked pointer handling is to support cases where a GEP can have multiple base pointers (e.g. due to a select or phi). But in this case there are 2 distinct GEPs each with their separate base pointer.
> > > I saw the comment on top of `findForkedPointers()`, and was initially confused too, but I think `findForkedPointers()` can be extended to support the use-case at hand: yes, there are two distinct GEPs, with separate base pointers, but really, both of them are loading the same ptr with different offsets: I look inside the load to find the SCEV associated to `@ld`, and the case at hand does seem to be vectorized successfully. Is the vectorization of the test-case I've provided incorrect?
> > > 
> > > If I weren't to modify `findForkedPointers()`, I'm left with SCEV AddExprs, which I have no idea how to handle independently of each other, and vectorization bails out saying that the expressions are not AddRecs.
> > For this code to be analyzable the two loads from `@ld` need to be CSE/GVNed first. Once that has happened, SCEV/LAA will be able to understand the relation between the GEPs.
> > 
> > You cannot simply assume that two loads from the same pointer will return the same value, as there may be clobbers in between.
> Good point. I'm curious about why CSE/GVN didn't eliminate one of the loads in this example.
That's because there is a store to `%arrayidx` in between, and it's possible for `%arrayidx` and `@ld` to be the same (if `@ld` is stored inside `@ld`).

In some cases TBAA metadata can help avoid this, but as `char` is involved here, it's not applicable to your case.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154714