[PATCH] D107552: [InstCombine] Combine lshr of add that intends to get the carry as llvm.uadd.with.overflow

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp:1105
+    for (TruncInst *Trunc : Truncs) {
+      Trunc->replaceAllUsesWith(UAdd);
+    }
----------------
craig.topper wrote:
> lebedev.ri wrote:
> > craig.topper wrote:
> > > craig.topper wrote:
> > > > We probably need to do something here to schedule the dead truncates to revisited to remove them. But I'm not sure.
> > > I think this should use InstCombinerImpl::replaceInstUsesWith rather than Instruction::replaceAllUsesWith. Then I think it should call InstCombinerImpl::eraseInstFromFunction.
> > > 
> > > @spatel or @lebedev.ri does that sound right?
> > Right, the pass-specific call should be used.
> > I don't think we need to call `eraseInstFromFunction()` explcitly,
> > that should be handled automatically once you return non-null from this function.
> > Generally, i would think not having an intermediate vector would be better, but not sure.
> The truncates aren’t directly part of the chain of instructions we’re changing so I’m not sure if the get queued properly for DCE once we return.
`InstCombinerImpl::replaceInstUsesWith()` adds the instruction uses of which we replaced into worklist,
so the next time it is revisited, it will be dropped.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107552