[PATCH] D135970: [InstCombine] try to determine "exact" for sdiv

Sun Oct 16 06:59:40 PDT 2022

spatel added inline comments.

================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1351

     if (match(Op1, m_NegatedPower2())) {
       // X sdiv (-(1 << C)) -> -(X sdiv (1 << C)) ->
----------------
nikic wrote:
> Side note, as you are working on division transforms. This has an obvious generalization to any `nonneg / neg` (https://alive2.llvm.org/ce/z/bYVnFG), and similar for `neg / nonneg` and `neg / neg`. It does require adding two negations in the general case, but I believe we consider that worthwhile to relax sdiv to udiv -- or at least we do the same transform based on range information in CVP.
Thanks for pointing that out. I didn't know CVP did that transform. Looks like it was:
8d487668d09fb0e

And the instcombine change/enhancement was implemented/mentioned here:
0fdcca07ad2c0bdc2

Scanning over x86 instruction timings at least, it's not clear if unsigned div (DIV) is any faster than signed div (IDIV). So that seems questionable as an IR transform since it's not reversible in general, but if the negates are deleted/noise in most cases, then it's ok.

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

https://reviews.llvm.org/D135970