From: Sam Parker Date: Wed, 12 Jun 2019 11:53:55 +0000 (+0000) Subject: [NFC][SCEV] Add NoWrapFlag argument to InsertBinOp X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=951993d66b9b0c6e2709a3a1a3eb1707ed2f0854;p=llvm [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 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@363147 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/Analysis/ScalarEvolutionExpander.h b/include/llvm/Analysis/ScalarEvolutionExpander.h index 72e949fd152..a519f93216b 100644 --- a/include/llvm/Analysis/ScalarEvolutionExpander.h +++ b/include/llvm/Analysis/ScalarEvolutionExpander.h @@ -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 diff --git a/lib/Analysis/ScalarEvolutionExpander.cpp b/lib/Analysis/ScalarEvolutionExpander.cpp index 0c77d814242..d8e0b47c337 100644 --- a/lib/Analysis/ScalarEvolutionExpander.cpp +++ b/lib/Analysis/ScalarEvolutionExpander.cpp @@ -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(LHS)) if (Constant *CRHS = dyn_cast(RHS)) @@ -188,20 +189,22 @@ Value *SCEVExpander::InsertBinop(Instruction::BinaryOps Opcode, if (isa(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(I) && - (I->hasNoSignedWrap() || I->hasNoUnsignedWrap())) - return true; + auto canGenerateIncompatiblePoison = [&Flags](Instruction *I) { + // Ensure that no-wrap flags match. + if (isa(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(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(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(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())); }