[PATCH] D56355: [InstCombine] Simplify cttz/ctlz + icmp ugt/ult into mask check

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 5 11:14:43 PST 2019


lebedev.ri added a comment.

In D56355#1347389 <https://reviews.llvm.org/D56355#1347389>, @nikic wrote:

> In D56355#1347365 <https://reviews.llvm.org/D56355#1347365>, @lebedev.ri wrote:
>
> > Hm, why does this modify the existing instruction, and manually adds it to worklist,
> >  instead of simply creating a new instruction? This is rather unusual for instcombine i think.
>
>
> I assumed that this was to avoid creating a new instruction when we can reuse an existing one.
>  If this is just an anachronism, I can switch the existing code to create new nodes in a separate NFC commit and then adjust it here as well.


Unless the current approach was specifically requested during previous reviews (then a comment is needed),
IMHO yes please do change this.


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

https://reviews.llvm.org/D56355





More information about the llvm-commits mailing list