[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:1070
                                         InstCombinerImpl &IC) {
   if (!Cmp.hasOneUse() || !isa<Constant>(Cmp.getOperand(1)))
     return nullptr;
----------------
nikic wrote:
> nikic wrote:
> > I'm wondering what to do about this one use check. Should we canonicalize even if the compare has other uses?
> > 
> > We are folding cmp+sub+select into abs or abs+sub. However, both cmp and sub can potentially have extra uses.
> Here's how dropping the one use check would look like: https://gist.github.com/nikic/3dece504951cba82415fed2d6aa876bf
> 
> Nominally, we are only increasing instruction count for the combination of nabs with extra uses on both icmp and sub.
I think we shouldn't have one-use checks here, yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87188