[PATCH] D81189: [Reassociate] Teach ConvertShiftToMul to preserve nsw flag if the shift amount is not bitwidth - 1.

[Craig Topper via Phabricator via llvm-commits]

craig.topper created this revision.
craig.topper added reviewers: spatel, lebedev.ri.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

Multiply and shl have different signed overflow behavior in
some cases. But it looks like we should be ok as long as the
shift amount is less than bitwidth - 1.

Alive2: http://volta.cs.utah.edu:8080/z/MM4WZP


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81189

Files:
  llvm/lib/Transforms/Scalar/Reassociate.cpp
  llvm/test/Transforms/Reassociate/wrap-flags.ll


Index: llvm/test/Transforms/Reassociate/wrap-flags.ll
===================================================================
--- llvm/test/Transforms/Reassociate/wrap-flags.ll
+++ llvm/test/Transforms/Reassociate/wrap-flags.ll
@@ -18,6 +18,20 @@
   ret i32 %mul2
 }
 
+define i32 @shl_to_mul_nsw_2(i32 %i) {
+;
+; CHECK-LABEL: @shl_to_mul_nsw_2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[MUL:%.*]] = mul nsw i32 [[I:%.*]], 1073741824
+; CHECK-NEXT:    [[MUL2:%.*]] = add i32 [[MUL]], 1
+; CHECK-NEXT:    ret i32 [[MUL2]]
+;
+entry:
+  %mul = shl nsw i32 %i, 30
+  %mul2 = add i32 %mul, 1
+  ret i32 %mul2
+}
+
 define i32 @shl_to_mul_nuw(i32 %i) {
 ;
 ; CHECK-LABEL: @shl_to_mul_nuw(
@@ -46,6 +60,20 @@
   ret i32 %mul2
 }
 
+define i32 @shl_to_mul_nuw_nsw_bitwidth_m1(i32 %i) {
+;
+; CHECK-LABEL: @shl_to_mul_nuw_nsw_bitwidth_m1(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[MUL:%.*]] = mul nuw nsw i32 [[I:%.*]], -2147483648
+; CHECK-NEXT:    [[MUL2:%.*]] = add i32 [[MUL]], 1
+; CHECK-NEXT:    ret i32 [[MUL2]]
+;
+entry:
+  %mul = shl nuw nsw i32 %i, 31
+  %mul2 = add i32 %mul, 1
+  ret i32 %mul2
+}
+
 define i2 @pr23926(i2 %X1, i2 %X2) {
 ;
 ; CHECK-LABEL: @pr23926(
Index: llvm/lib/Transforms/Scalar/Reassociate.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/Reassociate.cpp
+++ llvm/lib/Transforms/Scalar/Reassociate.cpp
@@ -976,7 +976,8 @@
 /// this into a multiply by a constant to assist with further reassociation.
 static BinaryOperator *ConvertShiftToMul(Instruction *Shl) {
   Constant *MulCst = ConstantInt::get(Shl->getType(), 1);
-  MulCst = ConstantExpr::getShl(MulCst, cast<Constant>(Shl->getOperand(1)));
+  auto *SA = cast<ConstantInt>(Shl->getOperand(1));
+  MulCst = ConstantExpr::getShl(MulCst, SA);
 
   BinaryOperator *Mul =
     BinaryOperator::CreateMul(Shl->getOperand(0), MulCst, "", Shl);
@@ -989,10 +990,12 @@
 
   // We can safely preserve the nuw flag in all cases.  It's also safe to turn a
   // nuw nsw shl into a nuw nsw mul.  However, nsw in isolation requires special
-  // handling.
+  // handling.  It can be preserved as long as we're not left shifting by
+  // bitwidth - 1.
   bool NSW = cast<BinaryOperator>(Shl)->hasNoSignedWrap();
   bool NUW = cast<BinaryOperator>(Shl)->hasNoUnsignedWrap();
-  if (NSW && NUW)
+  unsigned BitWidth = Shl->getType()->getIntegerBitWidth();
+  if (NSW && (NUW || SA->getValue().ult(BitWidth - 1)))
     Mul->setHasNoSignedWrap(true);
   Mul->setHasNoUnsignedWrap(NUW);
   return Mul;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81189.268561.patch
Type: text/x-patch
Size: 2518 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200604/9616fbd6/attachment.bin>