[PATCH] D60395: [InstCombine] canonicalize sdiv with NEG operand

[ChenZheng via Phabricator via llvm-commits]

shchenz marked an inline comment as done.
shchenz added a comment.

Will fix it later. Thanks for your comments @lebedev.ri



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1028-1029
+  Value *Y;
+  if (match(&I, m_SDiv(m_OneUse(m_NSWSub(m_Zero(), m_Value(X))), m_Value(Y))) ||
+    match(&I, m_SDiv(m_Value(X), m_OneUse(m_NSWSub(m_Zero(), m_Value(Y))))))
+    return BinaryOperator::CreateNeg(
----------------
lebedev.ri wrote:
> It's not commutative like that.
> ```
> ----------------------------------------
> Optimization: -X / Y  to  -(X / Y)
> Precondition: true
>   %t0 = sub nsw i8 0, %x
>   %r = sdiv i8 %t0, %y
> =>
>   %n0 = sdiv i8 %x, %y
>   %r = sub i8 0, %n0
> 
> Done: 1
> Optimization is correct!
> 
> ----------------------------------------
> Optimization: X / -Y --> -(X / Y)
> Precondition: true
>   %t0 = sub nsw i8 0, %y
>   %r = sdiv i8 %x, %t0
> =>
>   %n0 = sdiv i8 %x, %y
>   %r = sub i8 0, %n0
> 
> 
> ERROR: Domain of definedness of Target is smaller than Source's for i8 %r
> 
> Example:
> %y i8 = 0xFF (255, -1)
> %x i8 = 0x80 (128, -128)
> %t0 i8 = 0x01 (1)
> %n0 i8 = 0x80 (128, -128)
> Source value: 0x80 (128, -128)
> Target value: undef
> 
> ```
> (You could use https://rise4fun.com/Alive, but it appears down for the moment?)
hmm, seems can not canonicalize `X/-Y ` ---> `-(X/Y)`.
I will only focus on `-X / Y`  to  `-(X / Y)` for this patch.

There should  be same opportunity for `(X/-Y)` if `Y` is constant. Will let it to be improved in later patch.


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

https://reviews.llvm.org/D60395