[PATCH] D39417: InstCombine: Preserve nuw when reassociating nuw ops

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

This consists of 3 separate changes, it would be easier to review them separately.



================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:198-229
 // Return true, if No Signed Wrap should be maintained for I.
 // The No Signed Wrap flag can be kept if the operation "B (I.getOpcode) C",
 // where both B and C should be ConstantInts, results in a constant that does
 // not overflow. This function only handles the Add and Sub opcodes. For
 // all other opcodes, the function conservatively returns false.
 static bool MaintainNoSignedWrap(BinaryOperator &I, Value *B, Value *C) {
   OverflowingBinaryOperator *OBO = dyn_cast<OverflowingBinaryOperator>(&I);
----------------
arsenm wrote:
> lebedev.ri wrote:
> > Why does `MaintainNoSignedWrap()` need all that logic to prove that no overflow happens,
> > but `MaintainNoUnsignedWrap()` does nothing of sorts?
> I'm not really sure why MaintainNoSignedWrap has all of those checks. It seems to me like it's looking an overflow that was already illegal?
https://rise4fun.com/Alive/8V9


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:226
 
+static bool MaintainNoUnsignedWrap(BinaryOperator &I) {
+  OverflowingBinaryOperator *OBO = dyn_cast<OverflowingBinaryOperator>(&I);
----------------
`Maintain` is the wrong term then, `has NUW flag` or something


================
Comment at: test/Transforms/InstCombine/reassociate-nuw.ll:1
+; RUN: opt -instcombine -S %s | FileCheck %s
+
----------------
Can you please precommit using `utils/update_test_checks.py` ?


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

https://reviews.llvm.org/D39417