[PATCH] D157187: [InstCombine] Propagate the nuw/nsw for instruction neg-sub

[Nikita Popov via Phabricator via llvm-commits]

nikic requested changes to this revision.
nikic added a comment.
This revision now requires changes to proceed.

You are making assumptions about the transforms Negator can do here -- in particular that a "sub" is only produced when negating another "sub". This is not correct. E.g. you miscompile this case: https://alive2.llvm.org/ce/z/Q_MiP4

You need to either integrate the flag handling into Negator to make sure it is only applied in the cases where it is actually valid, or you need to make this a separate transform.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:2069
+        // Make sure the sub and neg have the same type flag.
+        if (I.hasNoUnsignedWrap() && Op1Inst->hasNoUnsignedWrap())
+          InstOp1->setHasNoUnsignedWrap();
----------------
You don't seem to have test coverage for the nuw case, and it seems pretty useless to boot. `sub nuw 0, %x` can only be non-poison if `%x` is zero. This is not a useful case to handle.


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

https://reviews.llvm.org/D157187