[PATCH] D68408: [InstCombine] Negator - sink sinkable negations

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp:180
+  }
+  case Instruction::SDiv:
+    // `sdiv` is negatible if divisor is not undef/INT_MIN/1.
----------------
spatel wrote:
> lebedev.ri wrote:
> > nikic wrote:
> > > Noticed while looking through the tramp3d-v4 diff: This should be behind a one-use check, to avoid duplicating expensive division instructions.
> > I was hesitant about this one indeed, it isn't a typo that one-use check has gone away here,
> > because we //generally// consider only instruction count.
> > 
> > @spatel thoughts?
> > 
> > But the main, bigger question this touches is:
> > "but what if all the uses would get negated by us?
> > In future, can we somehow sanely model the whole negatible tree,
> > not giving up at non-single-use instructions,
> > but defer that to after we've finished building new tree?"
> > 
> > 
> I missed that logic difference, and I'm not getting a reviewable diff of the attached files with llvm-diff or other apps. Can you create an IR example/regression test for that?
@spatel
To be clear, the "bigger question" is pretty rhetorical, or at least not for this review.

The actual question here is whether we should consider `sdiv` special,
and even if we can negate it without increasing instruction count,
we should only do so if there are no other uses of old `sdiv`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68408