[PATCH] D67612: [UnrolledInstAnalyzer] Use MSSA to find stored values outside of loop.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

Here are a couple of answers/discussion points:

First of, could you please elaborate a bit more on what you're trying to check for? Is it stores before the loop, after the loop or both?
Normally we'd use the walker on the instruction (`MSSA->getWalker()->getClobberingMemoryAccess(&I)`) to look before the loop. Ideally we'd avoid going into the innards of MSSA in loop passes, so if a usecase not covered by the walker is needed that should be added either to a walker or to the updater.
It looks like a good use of MemorySSA, but the search should happen behind an API.

Secondly, in the new pass manager it is necessary for MSSA to be preserved by all loop passes in the same loop pipeline, because analyses are not actually invalidated until the entire loop pipeline is done. Hence it's possible to end up using an invalidated analysis and only notice this on a crash, miscompile or non-deterministic behavior. This occurred in D65060 <https://reviews.llvm.org/D65060> and I tried to add a safety net in D66376 <https://reviews.llvm.org/D66376> to prevent this from happening again (LPM2 will not use MSSA even if LoopUnroll is taught to preserve it,  until all other passes in LPM2 all preserve it).

As far as preserving MemorySSA in LoopUnroll, this is a complex task. I had started work in this, but I did not complete it because unrolling does a lot of cloning/copying, and adding instructions "out of nowhere" to MSSA can lead to complex updates. It may be worth evaluating if updating MSSA is worth it for the unroll pass, or if it's faster to rebuild it from scratch. I did not get a chance to make this evaluation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67612