[PATCH] D132199: [LoopPassManager] Ensure to construct loop nests with the outermost loop

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

I think I'm ok with this sort of patch being a temporary workaround
could you add a FIXME pointing to the discussion here?

Using PreservedAnalyses to tell the loop pass manager that the loop nest structure has changed is still IMO an abuse of it. The whole point of PreservedAnalyses is that we determine whether or not some cached analysis for a given IR unit should be invalidated. The loop nest analysis shouldn't be modeled as a loop analysis since it's not at the same IR level (that's why https://reviews.llvm.org/D132581 is wrong, caused issues, and was reverted). This is essentially adding an extra bit to the pass's return value to tell the loop pass manager to change how it behaves, which is not what PreservedAnalyses is intended to be used for. But that's exactly why LPMUpdater exists, for this sort of thing. So I'd still like to go back to the LPMUpdater version.

I also think there may be issues with loop analyses not being invalidated in regards to loop-interchange. If an inner loop has a cached analysis, right now I believe interchanging does not invalidate the (original) inner loop's analyses, but we may end up running a loop nest pass on it with its old analyses once it's the outer loop, even though the loop structure has changed. But that can be a separate patch.


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

https://reviews.llvm.org/D132199