[PATCH] D60656: [LVI][CVP] Calculate with.overflow result range

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

This looks like a straightforward extension of the existing logic, so LGTM. Up to you if you want to wait for another review.

But how about making something like below as a preliminary NFC change? Then we would reduce the duplication, and this patch just becomes a single small addition.

  diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp
  index 02a829f500b..10c1dd8f415 100644
  --- a/llvm/lib/Analysis/LazyValueInfo.cpp
  +++ b/llvm/lib/Analysis/LazyValueInfo.cpp
  @@ -422,6 +422,8 @@ namespace {
                                BasicBlock *BB);
     Optional<ConstantRange> getRangeForOperand(unsigned Op, Instruction *I,
                                                BasicBlock *BB);
  +  bool solveBlockValueForMathOp(ValueLatticeElement &BBLV,
  +                                Instruction *MathInst, BasicBlock *BB);
     bool solveBlockValueBinaryOp(ValueLatticeElement &BBLV, BinaryOperator *BBI,
                                  BasicBlock *BB);
     bool solveBlockValueCast(ValueLatticeElement &BBLV, CastInst *CI,
  @@ -1019,6 +1021,31 @@ bool LazyValueInfoImpl::solveBlockValueCast(ValueLatticeElement &BBLV,
     return true;
   }
   
  +bool LazyValueInfoImpl::solveBlockValueForMathOp(ValueLatticeElement &BBLV,
  +                                                 Instruction *MathInst,
  +                                                 BasicBlock *BB) {
  +  assert((isa<BinaryOperator>(MathInst) || isa<WithOverflowInst>(MathInst)) &&
  +         "Expected binop or overflow op");
  +
  +  Optional<ConstantRange> Range0 = getRangeForOperand(0, MathInst, BB);
  +  Optional<ConstantRange> Range1 = getRangeForOperand(1, MathInst, BB);
  +  if (!Range0.hasValue() || !Range1.hasValue())
  +    return false;
  +
  +  // NOTE: We're currently limited by the set of operations that ConstantRange
  +  // can evaluate symbolically.  Enhancing that set will allows us to analyze
  +  // more definitions.
  +  Instruction::BinaryOps Opcode;
  +  if (auto *WithOV = dyn_cast<WithOverflowInst>(MathInst))
  +    Opcode = WithOV->getBinaryOp();
  +  else
  +    Opcode = cast<BinaryOperator>(MathInst)->getOpcode();
  +
  +  BBLV = ValueLatticeElement::getRange(
  +      Range0.getValue().binaryOp(Opcode, Range1.getValue()));
  +  return true;
  +}
  +
   bool LazyValueInfoImpl::solveBlockValueBinaryOp(ValueLatticeElement &BBLV,
                                                   BinaryOperator *BO,
                                                   BasicBlock *BB) {
  @@ -1049,26 +1076,7 @@ bool LazyValueInfoImpl::solveBlockValueBinaryOp(ValueLatticeElement &BBLV,
       return true;
     };
   
  -  // Figure out the ranges of the operands.  If that fails, use a
  -  // conservative range, but apply the transfer rule anyways.  This
  -  // lets us pick up facts from expressions like "and i32 (call i32
  -  // @foo()), 32"
  -  Optional<ConstantRange> LHSRes = getRangeForOperand(0, BO, BB);
  -  Optional<ConstantRange> RHSRes = getRangeForOperand(1, BO, BB);
  -
  -  if (!LHSRes.hasValue() || !RHSRes.hasValue())
  -    // More work to do before applying this transfer rule.
  -    return false;
  -
  -  ConstantRange LHSRange = LHSRes.getValue();
  -  ConstantRange RHSRange = RHSRes.getValue();
  -
  -  // NOTE: We're currently limited by the set of operations that ConstantRange
  -  // can evaluate symbolically.  Enhancing that set will allows us to analyze
  -  // more definitions.
  -  Instruction::BinaryOps BinOp = BO->getOpcode();
  -  BBLV = ValueLatticeElement::getRange(LHSRange.binaryOp(BinOp, RHSRange));
  -  return true;
  +  return solveBlockValueForMathOp(BBLV, BO, BB);
   }
   
   static ValueLatticeElement getValueFromICmpCondition(Value *Val, ICmpInst *ICI,


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

https://reviews.llvm.org/D60656