[PATCH] D134606: [LAA] Change to function analysis for new PM.

Tue Sep 27 14:50:19 PDT 2022

fhahn added a comment.

Thanks for taking a look!

In D134606#3819182 <https://reviews.llvm.org/D134606#3819182>, @aeubanks wrote:

> two thoughts:
>
> IIUC the issue is that LoopVersioningLICM (a loop pass) is accessing other loops' analyses. a simpler fix would be to not go through the analysis manager just for LoopVersioningLICM and do the caching within the pass. that's basically what this patch does but isolated to LoopVersioningLICM

I don't think the issue is LoopVersioningLICM specifically.

The reported issues are with LoopVectorize and LoopDistribution, which both are function passes and use LAA (currently a loop pass). The issue is when LAA gets computed as part of a loop pipeline  (or before) and used after the loop pipeline by function passes. If it uses a SCEV that has been invalidated and/or removed earlier in the loop pipeline due to a change in a different loop, it will reference a stale SCEV.

> but perhaps there can be a more proper fix at the analysis manager level. I think (but am not 100% sure) there was the assumption that a loop pass cannot look at another loop's analysis (if we decide to go this route we should add checks). but if we allow loop passes to look at another loop's analysis, we should properly invalidate relevant loops in the pass manager. which loops' analyses does LoopVersioningLICM look at?

LAA is the only analysis that currently has this issue AFAIK (caching SCEV in a loop analysis) and I think it would be desirable to keep the fix as straight-forward as possible. There is very little benefit from LAA being a loop analysis in practice I think and I am also not sure if there's an efficient/lightweight way of tracking which loops in LAA that would need invalidating if a given SCEV gets invalidated. But I might be missing some utilities the pass infrastructure provides here.

In addition to that, we also need to be able to clear cached LAA info for all loops from function passes, if they make changes that may also invalidated SCEVs (D134611 <https://reviews.llvm.org/D134611>) and this seems to fit quite nicely with the function analysis.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134606