Commit 61de6a4e authored by Sam Parker's avatar Sam Parker
Browse files

[NFC][SCEV] Add NoWrapFlag argument to InsertBinOp 

'Use wrap flags in InsertBinop' (rL362687) was reverted due to
miscompiles. This patch introduces the previous change to pass
no-wrap flags but now only FlagAnyWrap is passed.

Differential Revision: https://reviews.llvm.org/D61934

llvm-svn: 363147
parent ad805ef9

llvm/include/llvm/Analysis/ScalarEvolutionExpander.h

+1 −1
Original line number Diff line number Diff line
@@ -318,7 +318,7 @@ namespace llvm { 
    /// avoid inserting an obviously redundant operation, and hoisting to an

    /// outer loop when the opportunity is there and it is safe.

    Value *InsertBinop(Instruction::BinaryOps Opcode, Value *LHS, Value *RHS,

                       bool IsSafeToHoist);

                       SCEV::NoWrapFlags Flags, bool IsSafeToHoist);



    /// Arrange for there to be a cast of V to Ty at IP, reusing an existing

    /// cast if a suitable one exists, moving an existing cast if a suitable one

llvm/lib/Analysis/ScalarEvolutionExpander.cpp

+31 −19
Original line number Diff line number Diff line
@@ -169,7 +169,8 @@ Value *SCEVExpander::InsertNoopCastOfTo(Value *V, Type *Ty) { 
/// of work to avoid inserting an obviously redundant operation, and hoisting

/// to an outer loop when the opportunity is there and it is safe.

Value *SCEVExpander::InsertBinop(Instruction::BinaryOps Opcode,

                                 Value *LHS, Value *RHS, bool IsSafeToHoist) {

                                 Value *LHS, Value *RHS,

                                 SCEV::NoWrapFlags Flags, bool IsSafeToHoist) {

  // Fold a binop with constant operands.

  if (Constant *CLHS = dyn_cast<Constant>(LHS))

    if (Constant *CRHS = dyn_cast<Constant>(RHS))
@@ -188,20 +189,22 @@ Value *SCEVExpander::InsertBinop(Instruction::BinaryOps Opcode, 
      if (isa<DbgInfoIntrinsic>(IP))

        ScanLimit++;



      // Conservatively, do not use any instruction which has any of wrap/exact

      // flags installed.

      // TODO: Instead of simply disable poison instructions we can be clever

      //       here and match SCEV to this instruction.

      auto canGeneratePoison = [](Instruction *I) {

        if (isa<OverflowingBinaryOperator>(I) &&

            (I->hasNoSignedWrap() || I->hasNoUnsignedWrap()))

      auto canGenerateIncompatiblePoison = [&Flags](Instruction *I) {

        // Ensure that no-wrap flags match.

        if (isa<OverflowingBinaryOperator>(I)) {

          if (I->hasNoSignedWrap() != (Flags & SCEV::FlagNSW))

            return true;

          if (I->hasNoUnsignedWrap() != (Flags & SCEV::FlagNUW))

            return true;

        }

        // Conservatively, do not use any instruction which has any of exact

        // flags installed.

        if (isa<PossiblyExactOperator>(I) && I->isExact())

          return true;

        return false;

      };

      if (IP->getOpcode() == (unsigned)Opcode && IP->getOperand(0) == LHS &&

          IP->getOperand(1) == RHS && !canGeneratePoison(&*IP))

          IP->getOperand(1) == RHS && !canGenerateIncompatiblePoison(&*IP))

        return &*IP;

      if (IP == BlockBegin) break;

    }
@@ -226,6 +229,10 @@ Value *SCEVExpander::InsertBinop(Instruction::BinaryOps Opcode, 
  // If we haven't found this binop, insert it.

  Instruction *BO = cast<Instruction>(Builder.CreateBinOp(Opcode, LHS, RHS));

  BO->setDebugLoc(Loc);

  if (Flags & SCEV::FlagNUW)

    BO->setHasNoUnsignedWrap();

  if (Flags & SCEV::FlagNSW)

    BO->setHasNoSignedWrap();

  rememberInstruction(BO);



  return BO;
@@ -737,7 +744,8 @@ Value *SCEVExpander::visitAddExpr(const SCEVAddExpr *S) { 
      // Instead of doing a negate and add, just do a subtract.

      Value *W = expandCodeFor(SE.getNegativeSCEV(Op), Ty);

      Sum = InsertNoopCastOfTo(Sum, Ty);

      Sum = InsertBinop(Instruction::Sub, Sum, W, /*IsSafeToHoist*/ true);

      Sum = InsertBinop(Instruction::Sub, Sum, W, SCEV::FlagAnyWrap,

                        /*IsSafeToHoist*/ true);

      ++I;

    } else {

      // A simple add.
@@ -745,7 +753,8 @@ Value *SCEVExpander::visitAddExpr(const SCEVAddExpr *S) { 
      Sum = InsertNoopCastOfTo(Sum, Ty);

      // Canonicalize a constant to the RHS.

      if (isa<Constant>(Sum)) std::swap(Sum, W);

      Sum = InsertBinop(Instruction::Add, Sum, W, /*IsSafeToHoist*/ true);

      Sum = InsertBinop(Instruction::Add, Sum, W, SCEV::FlagAnyWrap,

                        /*IsSafeToHoist*/ true);

      ++I;

    }

  }
@@ -774,7 +783,7 @@ Value *SCEVExpander::visitMulExpr(const SCEVMulExpr *S) { 
  // Expand the calculation of X pow N in the following manner:

  // Let N = P1 + P2 + ... + PK, where all P are powers of 2. Then:

  // X pow N = (X pow P1) * (X pow P2) * ... * (X pow PK).

  const auto ExpandOpBinPowN = [this, &I, &OpsAndLoops, &Ty]() {

  const auto ExpandOpBinPowN = [this, &I, &OpsAndLoops, &Ty, &S]() {

    auto E = I;

    // Calculate how many times the same operand from the same loop is included

    // into this power.
@@ -797,9 +806,11 @@ Value *SCEVExpander::visitMulExpr(const SCEVMulExpr *S) { 
    if (Exponent & 1)

      Result = P;

    for (uint64_t BinExp = 2; BinExp <= Exponent; BinExp <<= 1) {

      P = InsertBinop(Instruction::Mul, P, P, /*IsSafeToHoist*/ true);

      P = InsertBinop(Instruction::Mul, P, P, SCEV::FlagAnyWrap,

                      /*IsSafeToHoist*/ true);

      if (Exponent & BinExp)

        Result = Result ? InsertBinop(Instruction::Mul, Result, P,

                                      SCEV::FlagAnyWrap,

                                      /*IsSafeToHoist*/ true)

                        : P;

    }
@@ -817,7 +828,7 @@ Value *SCEVExpander::visitMulExpr(const SCEVMulExpr *S) { 
      // Instead of doing a multiply by negative one, just do a negate.

      Prod = InsertNoopCastOfTo(Prod, Ty);

      Prod = InsertBinop(Instruction::Sub, Constant::getNullValue(Ty), Prod,

                         /*IsSafeToHoist*/ true);

                         SCEV::FlagAnyWrap, /*IsSafeToHoist*/ true);

      ++I;

    } else {

      // A simple mul.
@@ -831,9 +842,10 @@ Value *SCEVExpander::visitMulExpr(const SCEVMulExpr *S) { 
        assert(!Ty->isVectorTy() && "vector types are not SCEVable");

        Prod = InsertBinop(Instruction::Shl, Prod,

                           ConstantInt::get(Ty, RHS->logBase2()),

                           /*IsSafeToHoist*/ true);

                           SCEV::FlagAnyWrap, /*IsSafeToHoist*/ true);

      } else {

        Prod = InsertBinop(Instruction::Mul, Prod, W, /*IsSafeToHoist*/ true);

        Prod = InsertBinop(Instruction::Mul, Prod, W, SCEV::FlagAnyWrap,

                           /*IsSafeToHoist*/ true);

      }

    }

  }
@@ -850,11 +862,11 @@ Value *SCEVExpander::visitUDivExpr(const SCEVUDivExpr *S) { 
    if (RHS.isPowerOf2())

      return InsertBinop(Instruction::LShr, LHS,

                         ConstantInt::get(Ty, RHS.logBase2()),

                         /*IsSafeToHoist*/ true);

                         SCEV::FlagAnyWrap, /*IsSafeToHoist*/ true);

  }



  Value *RHS = expandCodeFor(S->getRHS(), Ty);

  return InsertBinop(Instruction::UDiv, LHS, RHS,

  return InsertBinop(Instruction::UDiv, LHS, RHS, SCEV::FlagAnyWrap,

                     /*IsSafeToHoist*/ SE.isKnownNonZero(S->getRHS()));

}