[llvm] 1720ec6 - [InstCombine] restrict no-wrap propagation for i1/i2 to avoid miscompiles

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2023-01-18T10:32:12-05:00
New Revision: 1720ec6da040729f17dfc685f853b9c15658fbc6

URL: https://github.com/llvm/llvm-project/commit/1720ec6da040729f17dfc685f853b9c15658fbc6
DIFF: https://github.com/llvm/llvm-project/commit/1720ec6da040729f17dfc685f853b9c15658fbc6.diff

LOG: [InstCombine] restrict no-wrap propagation for i1/i2 to avoid miscompiles

This transform was added with 68c197f07eeae71b9b7,
and the post-commit review noted the potential
for miscompiles at narrow bitwidths.

I'm not sure how to expose the i1 nuw bug because we
already simplify that, but other cases show that
there are missing transforms to add in follow-up
patches.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
    llvm/test/Transforms/InstCombine/sub.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index f4393d6dd02d..b68efc993723 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -2379,9 +2379,9 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
     auto *OBO0 = cast<OverflowingBinaryOperator>(Op0);
     auto *OBO1 = cast<OverflowingBinaryOperator>(Op1);
     bool PropagateNSW = I.hasNoSignedWrap() && OBO0->hasNoSignedWrap() &&
-                        OBO1->hasNoSignedWrap();
+                        OBO1->hasNoSignedWrap() && BitWidth > 2;
     bool PropagateNUW = I.hasNoUnsignedWrap() && OBO0->hasNoUnsignedWrap() &&
-                        OBO1->hasNoUnsignedWrap();
+                        OBO1->hasNoUnsignedWrap() && BitWidth > 1;
     Value *Add = Builder.CreateAdd(X, Y, "add", PropagateNUW, PropagateNSW);
     Value *Sub = Builder.CreateSub(X, Y, "sub", PropagateNUW, PropagateNSW);
     Value *Mul = Builder.CreateMul(Add, Sub, "", PropagateNUW, PropagateNSW);

diff  --git a/llvm/test/Transforms/InstCombine/sub.ll b/llvm/test/Transforms/InstCombine/sub.ll
index 155c3afad2e3..96a527ec0749 100644
--- a/llvm/test/Transforms/InstCombine/sub.ll
+++ b/llvm/test/Transforms/InstCombine/sub.ll
@@ -2489,13 +2489,14 @@ define i1 @
diff _of_squares_nuw_i1(i1 %x, i1 %y) {
   ret i1 %r
 }
 
-; FIXME: It is not correct to propagate nsw.
+; It is not correct to propagate nsw.
+; TODO: This should reduce more.
 
 define i2 @
diff _of_squares_nsw_i2(i2 %x, i2 %y) {
 ; CHECK-LABEL: @
diff _of_squares_nsw_i2(
-; CHECK-NEXT:    [[ADD:%.*]] = add nsw i2 [[X:%.*]], [[Y:%.*]]
-; CHECK-NEXT:    [[SUB:%.*]] = sub nsw i2 [[X]], [[Y]]
-; CHECK-NEXT:    [[R:%.*]] = mul nsw i2 [[ADD]], [[SUB]]
+; CHECK-NEXT:    [[ADD:%.*]] = add i2 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    [[SUB:%.*]] = sub i2 [[X]], [[Y]]
+; CHECK-NEXT:    [[R:%.*]] = mul i2 [[ADD]], [[SUB]]
 ; CHECK-NEXT:    ret i2 [[R]]
 ;
   %x2 = mul nsw i2 %x, %x