[PATCH] D50377: [MustExecute] Rework LoopSafetyInfo to make it more optimistic about throws

Fedor Sergeev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Aug 12 13:21:20 PDT 2018


fedor.sergeev added a comment.

I'm really worried about LoopSafetyInfo getting more nontrivial state
(Philip already raised a question in https://reviews.llvm.org/D50426 about CurLoop potentially becoming invalid, and here we get even more with addition of ICF).

Before this change LoopSafetyInfo was one-time-per-Loop-worker + constant queries after that, and original interface conveyed that.
Now every query becomes a potentially mutating worker, with a cache (ICF) that causes correctness problems if not being invalidated properly.

I have a feeling that changes suggested to the interface do not fully reflect the new semantics.
I can easily see how people can forget to invalidateBlock.
(and, btw, are we sure that invalidateBlock is not needed in places that use LoopSafetyInfo other than LICM?)

I dont know what should be a better solution, though I do suspect this is a problem frequently seen in LLVM so there should already be solutions
to invalidation of stale information after IR mutation.

Feel free to ignore all the above if it does not bother you :)


https://reviews.llvm.org/D50377





More information about the llvm-commits mailing list