[PATCH] D147750: [LAA/LV] Simplify stride speculation logic [nfc]

Thu Apr 13 08:53:37 PDT 2023

reames added a comment.

In D147750#4250830 <https://reviews.llvm.org/D147750#4250830>, @peixin wrote:

> Also, it seems that removing the `StrideSet` will cause the const stride of non-one (such as 4) in D147539 <https://reviews.llvm.org/D147539> hard to check in cost model.

This really isn't true JFYI.  The whole point of adding a predicate to PSE is that PSE will then *simplify using that predicate*.  I'm 100% sure it does the canonicalization today, but it definitely should be and doing so is straight forward.

In D147750#4251120 <https://reviews.llvm.org/D147750#4251120>, @fhahn wrote:

> In a way it is slightly more powerful to use PSE directly, as it has info about all predicates (not just about the strides). It should be possible to separate changing the type of the map and replacing stride set.

I separated the changes since it triggered more discussion than I'd really expected.

> Not sure if there's a big benefit from changing the type of the map; IIUC we will only add SCEVUnknown for the strides anyways.

We return only SCEVUnknown today.  That's an implementation detail of the *profitability* heuristic in this code.  We *could* return any loop invariant SCEV which is useful to speculate on.  I added a clarifying comment and restructured the patch description to emphasize this.

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

https://reviews.llvm.org/D147750