[PATCH] D38201: Use a BumpPtrAllocator for Loop objects

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

In https://reviews.llvm.org/D38201#880392, @sanjoy wrote:

> There is a problem with this patch -- the legacy pass manager checks if a loop is valid or not by querying `isInvalid`, which is incorrect; since we call the destructor for loop objects to mark them as "invalid" calling `isInvalid` for possibly invalid loop objects has UB.  We already have this UB in `User::operator delete` though, and so given that and that the legacy pass manager is going away at some point, perhaps this is okay?  Alternatively, I could add a mechanism on the legacy loop pass manager that lets passes denote loop objects to have been deleted (by remembering the pointers in a set) and use that to decide if a Loop object is valid or not (and with that, I can guard `isInvalid` with `#ifndef NDEBUG`).


Sadly, I think this would be problematic. I think it'll trip ASan's use-after-scope or use-after-lifetime checks.... =/ But maybe there is a way around that. Or maybe just implement the solution you suggest. Dunno, but definitely worth checking.

> The asserts are using `isInvalid` correctly, as far as I can tell; they should never have UB in an execution that uses the loop objects correctly.

Cool.


https://reviews.llvm.org/D38201