[PATCH] D60061: [InstCombine] ssubo X, C -> saddo X, -C

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:2161
+    // ssubo X, C -> saddo X, -C
+    if (match(Arg1, m_Constant(C)) && C->isNotMinSignedValue()) {
+      // Create a copy of the original constant and negate it.
----------------
lebedev.ri wrote:
> This will be yet another miscompile for vectors if there is `undef` element.
> You need to do
> ```
> if (match(Arg1, m_Constant(C)) && C->isNotMinSignedValue() && !C->containsUndefElement()) {
> ```
isNotMinSignedValue() does not accept undef elements. Correctly handling non-splat vectors and undefs is why this function exists (rather than just negating isMinSignedValue).


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:2221
     if (IID == Intrinsic::ssub_sat && match(Arg1, m_Constant(C)) &&
         C->isNotMinSignedValue()) {
       Value *NegVal = ConstantExpr::getNeg(C);
----------------
lebedev.ri wrote:
> This is also a miscompile for vectors for undef elements (please submit fix as a new patch)
See above.


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

https://reviews.llvm.org/D60061