[PATCH] D109294: [Inline] Remove dead users before checking if function has one use (PR51667)

[Mircea Trofin via llvm-commits]

I like the second option. It addresses the original issue and removes any
fragility. I think option 3 invalidates the contract analyses have with the
system (i.e. IR mutation => analysis invalidated), and we shouldn't take
it, it leads to fragility and maintenance costs, and I offer the relatively
recent memory of the cgscc pass and immutable analyses and invalidation as
illustration of how not fun diagnosing these kinds of issues are.

On Fri, Sep 10, 2021 at 11:25 AM Arthur Eubanks via Phabricator <
reviews at reviews.llvm.org> wrote:

> aeubanks added a comment.
>
> (moving this to a main thread so we don't have to reply to a thread on
> code in an old revision)
>
> hmm, does removing dead constant users invalidate analyses? that's an
> interesting question. I guess technically yes it should, although not sure
> it'll matter in practice.
>
> I think we have three options
>
> 1. mtrofin's original idea of adding a cleanup pass before the inliner
> that calls removeDeadConstantUsers on each function, and we invalidate
> non-cfg analyses if we make any changes
> 2. add `Constant::hasOneLiveUse()` and use that in InlineCost.cpp rather
> than modifying IR
> 3. keep doing what we're doing here and assume that no analyses rely on
> dead constant users
>
> imo we can get away with the current patch
> any thoughts?
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D109294/new/
>
> https://reviews.llvm.org/D109294
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210910/159f3800/attachment.html>