[PATCH] D77325: [InstCombine] Don't limit operands in eraseInstFromFunction()

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D77325#1961451 <https://reviews.llvm.org/D77325#1961451>, @lebedev.ri wrote:

> This blames all the way back to rL80483 <https://reviews.llvm.org/rL80483>:
>
> > inline the trivial AddToWorkList/RemoveFromWorkList methods into their callers. simplify ReplaceInstUsesWith. Make EraseInstFromFunction only add operands to the worklist if there aren't too many of them (this was a scalability win for crazy programs that was only infrequently enforced). Switch more code to using EraseInstFromFunction instead of duplicating it inline. Change some fcmp/icmp optimizations to modify fcmp/icmp in place instead of creating a new one and deleting the old one just to change the predicate.
>
> So much like waymarking, some optimization that doesn't have clear consequences.
>  I think this shouldn't be here, at least for the consistency reasons, unless there's a testcase that shows otherwise.


Thanks for tracking down where this came from!

If this does turn out to cause performance issues, we should be able to mitigate this in a more principled fashion, by distinguishing things that are added to the worklist for (primarily) DCE purposes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77325