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

Arthur Eubanks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 29 14:54:10 PDT 2022


aeubanks added a comment.

In D134606#3824981 <https://reviews.llvm.org/D134606#3824981>, @fhahn wrote:

> In D134606#3824905 <https://reviews.llvm.org/D134606#3824905>, @aeubanks wrote:
>
>> I looked at the repro in #50940 to see why analyses weren't getting invalidated. I think something like https://github.com/llvm/llvm-project/commit/f4c0810a79776a63fd2ba57be0199a31e35d5ad7 is more appropriate. We should be invalidating all loop analyses after a loop adaptor is finished, if any loop pass made a change. That fixes https://github.com/llvm/llvm-project/issues/50940. Thoughts on that?
>
> Hmm, that seems quite a heavy hammer, IIUC this would invalidate all loop analyses? It seems like that would have a much bigger compile-time hit and seems overly aggressive, especially as we are trying hard to preserve loop analyses (and I think for almost all cases we are doing a good job).
>
> That's why avoiding the issue by making LAA a function analysis seems preferable IMO; I don't think the users of LAA really care if it is a function/loop analysis.

On principle we should never have invalid analyses that are in the cache and available for use. If a loop analysis can depend on a function analysis and that function analysis may be modified over the course of the loop pipeline, we should blow away the loop analysis. That patch blows away loop analyses as we return to a function pass manager from a loop pass manager if any loop passes have made changes.

Seemingly no compile time impact:
https://llvm-compile-time-tracker.com/compare.php?from=0cdd671df919974bed11293e37ac917cac55be64&to=f4c0810a79776a63fd2ba57be0199a31e35d5ad7&stat=instructions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134606



More information about the llvm-commits mailing list