[PATCH] D158510: [InstCombine] Propagate nsw flag when negating

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D158510#4607222 <https://reviews.llvm.org/D158510#4607222>, @goldstein.w.n wrote:

> All of these except the `-1 << X` case work for NUW as well: https://alive2.llvm.org/ce/z/zzPZRQ
> Maybe instead of a bool pass a bitmask of flags? Would make it easier to add `NUW` support in a followup.

`sub nuw 0, %x` is only non-poison when `%x` is zero. For that reason, it's not really something that occurs in practice, so I don't think it's worth handling.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:264
       return BinaryOperator::CreateMul(
           NegOp0, ConstantExpr::getNeg(cast<Constant>(Op1)), I.getName());
 
----------------
goldstein.w.n wrote:
> You can have NSW on this mul given that `Op1` is a constant.
I don't think you can add nsw on the mul if I got this right: https://alive2.llvm.org/ce/z/lSk99J

Though it looks like passing nsw to the negator would be okay: https://alive2.llvm.org/ce/z/MrwT5K


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp:306
     // `freeze` is negatible if its operand is negatible.
-    Value *NegOp = negate(I->getOperand(0), Depth + 1);
+    Value *NegOp = negate(I->getOperand(0), /* IsNSW */ false, Depth + 1);
     if (!NegOp) // Early return.
----------------
goldstein.w.n wrote:
> Why does freeze kill `nsw`?
Hm, looks like it's okay to keep nsw: https://alive2.llvm.org/ce/z/vnYqoi That's quite unexpected to me.


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

https://reviews.llvm.org/D158510