[PATCH] D150353: [CVP] Don't introduce zero after truncating when narrowUDivOrSDiv

[luxufan via Phabricator via llvm-commits]

StephenFan created this revision.
StephenFan added reviewers: nikic, goldstein.w.n.
Herald added a subscriber: hiraditya.
Herald added a project: All.
StephenFan requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

The constant range computed at use point does not mean the input
constant range or in other words, the constant range computed at define point,
is the same range as computed at use point. For intructions that is
always speculative, it is safe. However, for instruction UDiv or SDiv,
and we want to narrow the data type of these, zero may be introduced
after truncating the divisor and causing an UB.

So this patch checks that if the constant range at define point does not
contain zero, then it also must not contain zero after truncating.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150353

Files:
  llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
  llvm/test/Transforms/CorrelatedValuePropagation/udiv.ll
  llvm/test/Transforms/CorrelatedValuePropagation/urem.ll


Index: llvm/test/Transforms/CorrelatedValuePropagation/urem.ll
===================================================================
--- llvm/test/Transforms/CorrelatedValuePropagation/urem.ll
+++ llvm/test/Transforms/CorrelatedValuePropagation/urem.ll
@@ -394,4 +394,21 @@
   ret void
 }
 
+define i32 @udiv_do_not_truncate(i32 noundef %call, i32 %v) {
+; CHECK-LABEL: @udiv_do_not_truncate(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[CALL_NON_NULL:%.*]] = or i32 [[CALL:%.*]], 1
+; CHECK-NEXT:    [[DIV:%.*]] = urem i32 8192, [[CALL_NON_NULL]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i32 [[CALL_NON_NULL]], 8192
+; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[CMP]], i32 1, i32 [[DIV]]
+; CHECK-NEXT:    ret i32 [[SELECT]]
+;
+entry:
+  %call_non_null = or i32 %call, 1
+  %div = urem i32 8192, %call_non_null
+  %cmp = icmp ugt i32 %call_non_null, 8192
+  %select = select i1 %cmp, i32 1, i32 %div
+  ret i32 %select
+}
+
 declare void @use(i1)
Index: llvm/test/Transforms/CorrelatedValuePropagation/udiv.ll
===================================================================
--- llvm/test/Transforms/CorrelatedValuePropagation/udiv.ll
+++ llvm/test/Transforms/CorrelatedValuePropagation/udiv.ll
@@ -99,3 +99,4 @@
 exit:
   ret void
 }
+
Index: llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
+++ llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
@@ -785,8 +785,10 @@
 
 /// Try to shrink a udiv/urem's width down to the smallest power of two that's
 /// sufficient to contain its operands.
-static bool narrowUDivOrURem(BinaryOperator *Instr, const ConstantRange &XCR,
-                             const ConstantRange &YCR) {
+static bool narrowUDivOrURem(BinaryOperator *Instr,
+                             const ConstantRange &XCRAtUse,
+                             const ConstantRange &YCRAtUse,
+                             const ConstantRange &YCRAtDef) {
   assert(Instr->getOpcode() == Instruction::UDiv ||
          Instr->getOpcode() == Instruction::URem);
   assert(!Instr->getType()->isVectorTy());
@@ -796,7 +798,8 @@
 
   // What is the smallest bit width that can accommodate the entire value ranges
   // of both of the operands?
-  unsigned MaxActiveBits = std::max(XCR.getActiveBits(), YCR.getActiveBits());
+  unsigned MaxActiveBits =
+      std::max(XCRAtUse.getActiveBits(), YCRAtUse.getActiveBits());
   // Don't shrink below 8 bits wide.
   unsigned NewWidth = std::max<unsigned>(PowerOf2Ceil(MaxActiveBits), 8);
 
@@ -805,6 +808,13 @@
   if (NewWidth >= Instr->getType()->getIntegerBitWidth())
     return false;
 
+  // For divisor, if the constant range computed at define point does not
+  // contain zero, then we should ensure the constant range after truncating
+  // also does not contain zero. Otherwise we may introduce an UB.
+  if (!YCRAtDef.contains(APInt::getZero(YCRAtDef.getBitWidth())) &&
+      YCRAtDef.truncate(NewWidth).contains(APInt::getZero(NewWidth)))
+    return false;
+
   ++NumUDivURemsNarrowed;
   IRBuilder<> B{Instr};
   auto *TruncTy = Type::getIntNTy(Instr->getContext(), NewWidth);
@@ -829,12 +839,13 @@
   if (Instr->getType()->isVectorTy())
     return false;
 
-  ConstantRange XCR = LVI->getConstantRangeAtUse(Instr->getOperandUse(0));
-  ConstantRange YCR = LVI->getConstantRangeAtUse(Instr->getOperandUse(1));
-  if (expandUDivOrURem(Instr, XCR, YCR))
+  ConstantRange YCRAtDef = LVI->getConstantRange(Instr->getOperand(1), Instr);
+  ConstantRange XCRAtUse = LVI->getConstantRangeAtUse(Instr->getOperandUse(0));
+  ConstantRange YCRAtUse = LVI->getConstantRangeAtUse(Instr->getOperandUse(1));
+  if (expandUDivOrURem(Instr, XCRAtUse, YCRAtUse))
     return true;
 
-  return narrowUDivOrURem(Instr, XCR, YCR);
+  return narrowUDivOrURem(Instr, XCRAtUse, YCRAtUse, YCRAtDef);
 }
 
 static bool processSRem(BinaryOperator *SDI, const ConstantRange &LCR,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150353.521246.patch
Type: text/x-patch
Size: 3970 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230511/e1b00c9f/attachment.bin>