[PATCH] D71209: InstCombine: Don't rewrite phi-of-bitcast when the phi has other users

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:2203
         if (OldPhiNodes.insert(PNode))
           PhiWorklist.push_back(PNode);
         continue;
----------------
lebedev.ri wrote:
> nikic wrote:
> > lebedev.ri wrote:
> > > I think we should be doing those checks here.
> > The `OldPhiNodes.count(PHI) == 0` part of the check wouldn't work correctly at this points -- it needs all PHIs to be collected first.
> Right. But the rest can be done here, no?
While it could be done, I don't think there'd be much benefit. We'd have to iterate all the users a second time just to check the phi nodes. The current implementation is also nicely symmetric between the checking loop (added in this patch) and the replacement loop (preexisting), and I think it's worth keeping that symmetry.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71209