[PATCH] D88166: [SCEV] Verify that all mapped SCEV AddRecs refer to valid loops.

[Philip Reames via Phabricator via llvm-commits]

reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

LGTM

In D88166#2300363 <https://reviews.llvm.org/D88166#2300363>, @fhahn wrote:

>> Actually, why not just implement the isInvalid check on Loop that way?
>
> LoopInfo manages the memory for each allocated loop and the memory is only really deleted when LI's memory is released. So the pointers to loops should stay valid as long as LI is alive, which allows for this quick check if the loop is valid. Not sure if there's a big benefit from changing that at the moment. What do you think?

I haven't dug into this, but the comments on isInvalid say that the flag is set by the destructor.  That's classic UB even if the underlying memory is retained.  If we can avoid relying on UB for our asserts, and yet still be efficient, that seems like a win.

Just to be clear here, this is the abstract "it would be better if", not the "please go and change X before landing".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88166