diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h --- a/llvm/include/llvm/Analysis/ValueTracking.h +++ b/llvm/include/llvm/Analysis/ValueTracking.h @@ -709,6 +709,7 @@ /// This method can return true for instructions that read memory; /// for such instructions, moving them may change the resulting value. bool isSafeToSpeculativelyExecute(const Instruction *I, + bool PreservesOpCharacteristics, const Instruction *CtxI = nullptr, AssumptionCache *AC = nullptr, const DominatorTree *DT = nullptr, @@ -732,9 +733,9 @@ /// This behavior is a shortcoming in the current implementation and not /// intentional. bool isSafeToSpeculativelyExecuteWithOpcode( - unsigned Opcode, const Instruction *Inst, const Instruction *CtxI = nullptr, - AssumptionCache *AC = nullptr, const DominatorTree *DT = nullptr, - const TargetLibraryInfo *TLI = nullptr); + unsigned Opcode, const Instruction *Inst, bool PreservesOpCharacteristics, + const Instruction *CtxI = nullptr, AssumptionCache *AC = nullptr, + const DominatorTree *DT = nullptr, const TargetLibraryInfo *TLI = nullptr); /// Returns true if the result or effects of the given instructions \p I /// depend values not reachable through the def use graph. diff --git a/llvm/lib/Analysis/IVUsers.cpp b/llvm/lib/Analysis/IVUsers.cpp --- a/llvm/lib/Analysis/IVUsers.cpp +++ b/llvm/lib/Analysis/IVUsers.cpp @@ -147,7 +147,8 @@ // IVUsers is used by LSR which assumes that all SCEV expressions are safe to // pass to SCEVExpander. Expressions are not safe to expand if they represent // operations that are not safe to speculate, namely integer division. - if (!isa(I) && !isSafeToSpeculativelyExecute(I)) + if (!isa(I) && + !isSafeToSpeculativelyExecute(I, /* PreservesOpCharacteristics */ true)) return false; // LSR is not APInt clean, do not touch integers bigger than 64-bits. diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp --- a/llvm/lib/Analysis/LazyValueInfo.cpp +++ b/llvm/lib/Analysis/LazyValueInfo.cpp @@ -1692,7 +1692,9 @@ // This also disallows looking through phi nodes: If the phi node is part // of a cycle, we might end up reasoning about values from different cycle // iterations (PR60629). - if (!CurrI->hasOneUse() || !isSafeToSpeculativelyExecute(CurrI)) + if (!CurrI->hasOneUse() || + !isSafeToSpeculativelyExecute(CurrI, + /* PreservesOpCharacteristics */ false)) break; CurrU = &*CurrI->use_begin(); } diff --git a/llvm/lib/Analysis/LoopInfo.cpp b/llvm/lib/Analysis/LoopInfo.cpp --- a/llvm/lib/Analysis/LoopInfo.cpp +++ b/llvm/lib/Analysis/LoopInfo.cpp @@ -81,7 +81,7 @@ // Test if the value is already loop-invariant. if (isLoopInvariant(I)) return true; - if (!isSafeToSpeculativelyExecute(I)) + if (!isSafeToSpeculativelyExecute(I, /* PreservesOpCharacteristics */ true)) return false; if (I->mayReadFromMemory()) return false; diff --git a/llvm/lib/Analysis/LoopNestAnalysis.cpp b/llvm/lib/Analysis/LoopNestAnalysis.cpp --- a/llvm/lib/Analysis/LoopNestAnalysis.cpp +++ b/llvm/lib/Analysis/LoopNestAnalysis.cpp @@ -87,7 +87,8 @@ std::optional OuterLoopLB) { bool IsAllowed = - isSafeToSpeculativelyExecute(&I) || isa(I) || isa(I); + isSafeToSpeculativelyExecute(&I, /* PreservesOpCharacteristics */ true) || + isa(I) || isa(I); if (!IsAllowed) return false; // The only binary instruction allowed is the outer loop step instruction, diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp --- a/llvm/lib/Analysis/ValueTracking.cpp +++ b/llvm/lib/Analysis/ValueTracking.cpp @@ -6122,17 +6122,18 @@ } bool llvm::isSafeToSpeculativelyExecute(const Instruction *Inst, + bool PreservesOpCharacteristics, const Instruction *CtxI, AssumptionCache *AC, const DominatorTree *DT, const TargetLibraryInfo *TLI) { - return isSafeToSpeculativelyExecuteWithOpcode(Inst->getOpcode(), Inst, CtxI, - AC, DT, TLI); + return isSafeToSpeculativelyExecuteWithOpcode( + Inst->getOpcode(), Inst, PreservesOpCharacteristics, CtxI, AC, DT, TLI); } bool llvm::isSafeToSpeculativelyExecuteWithOpcode( - unsigned Opcode, const Instruction *Inst, const Instruction *CtxI, - AssumptionCache *AC, const DominatorTree *DT, + unsigned Opcode, const Instruction *Inst, bool PreservesOpCharacteristics, + const Instruction *CtxI, AssumptionCache *AC, const DominatorTree *DT, const TargetLibraryInfo *TLI) { #ifndef NDEBUG if (Inst->getOpcode() != Opcode) { @@ -6158,31 +6159,68 @@ default: return true; case Instruction::UDiv: - case Instruction::URem: { - // x / y is undefined if y == 0. - const APInt *V; - if (match(Inst->getOperand(1), m_APInt(V))) - return *V != 0; - return false; - } + case Instruction::URem: case Instruction::SDiv: case Instruction::SRem: { - // x / y is undefined if y == 0 or x == INT_MIN and y == -1 - const APInt *Numerator, *Denominator; - if (!match(Inst->getOperand(1), m_APInt(Denominator))) - return false; - // We cannot hoist this division if the denominator is 0. - if (*Denominator == 0) + const DataLayout &DL = Inst->getModule()->getDataLayout(); + + // We wrap isGuaranteedNotToBePoison, isKnownNonZero, and computeKnownBits. + // If PreservesOpCharacteristics is true we use proper ValueTracking + // analysis. Otherwise we just check based on if the value is constant. The + // reason for this is `isSafeToSpeculativelyExecute` is used in a variety of + // places where the user expects to be able to modify the operands and + // fundementally change their characteristics with regards to the + // ValueTracking queries we use here (for example truncate the Num/Denom). + // So, unless the use gurantees they will preserve the characteristics, + // conservatively only do analysis on constant operands. + auto OpIsNonPoison = [&](Value *Op) { + return PreservesOpCharacteristics + ? isGuaranteedNotToBePoison(Op, AC, CtxI, DT) + : match(Op, m_ImmConstant()); + }; + + auto OpIsNonZero = [&](Value *Op) { + const APInt *C; + return PreservesOpCharacteristics + ? isKnownNonZero(Op, DL, /*Depth*/ 0, AC, CtxI, DT) + : (match(Op, m_APInt(C)) && !C->isZero()); + }; + + auto OpKnownBits = [&](Value *Op) { + if (PreservesOpCharacteristics) + return computeKnownBits(Op, DL, /*Depth*/ 0, AC, CtxI, DT); + + const APInt *C; + if (match(Op, m_APInt(C))) + return KnownBits::makeConstant(*C); + + KnownBits Known(getBitWidth(Op->getType(), DL)); + Known.resetAll(); + return Known; + }; + + // x / y is undefined if y == 0 or y is poison. + if (!OpIsNonPoison(Inst->getOperand(1)) || + !OpIsNonZero(Inst->getOperand(1))) return false; + + // Unsigned case only needs to avoid denominator == 0 or poison. + if (Opcode == Instruction::UDiv || Opcode == Instruction::URem) + return true; + + // x s/ y is also undefined if x == INT_MIN and y == -1 + KnownBits KnownDenominator = OpKnownBits(Inst->getOperand(1)); + // It's safe to hoist if the denominator is not 0 or -1. - if (!Denominator->isAllOnes()) + if (!KnownDenominator.Zero.isZero()) return true; - // At this point we know that the denominator is -1. It is safe to hoist as - // long we know that the numerator is not INT_MIN. - if (match(Inst->getOperand(0), m_APInt(Numerator))) - return !Numerator->isMinSignedValue(); - // The numerator *might* be MinSignedValue. - return false; + + // At this point denominator may be -1. It is safe to hoist as + // long we know that the numerator is neither poison nor INT_MIN. + if (!OpIsNonPoison(Inst->getOperand(0))) + return false; + KnownBits KnownNumerator = OpKnownBits(Inst->getOperand(0)); + return !KnownNumerator.getSignedMinValue().isMinSignedValue(); } case Instruction::Load: { const LoadInst *LI = dyn_cast(Inst); @@ -6234,7 +6272,7 @@ if (I.mayReadOrWriteMemory()) // Memory dependency possible return true; - if (!isSafeToSpeculativelyExecute(&I)) + if (!isSafeToSpeculativelyExecute(&I, /* PreservesOpCharacteristics */ true)) // Can't move above a maythrow call or infinite loop. Or if an // inalloca alloca, above a stacksave call. return true; diff --git a/llvm/lib/CodeGen/Analysis.cpp b/llvm/lib/CodeGen/Analysis.cpp --- a/llvm/lib/CodeGen/Analysis.cpp +++ b/llvm/lib/CodeGen/Analysis.cpp @@ -608,7 +608,8 @@ II->getIntrinsicID() == Intrinsic::experimental_noalias_scope_decl) continue; if (BBI->mayHaveSideEffects() || BBI->mayReadFromMemory() || - !isSafeToSpeculativelyExecute(&*BBI)) + !isSafeToSpeculativelyExecute(&*BBI, + /* PreservesOpCharacteristics */ true)) return false; } diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp --- a/llvm/lib/CodeGen/CodeGenPrepare.cpp +++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp @@ -6720,7 +6720,9 @@ auto *I = dyn_cast(V); // If it's safe to speculatively execute, then it should not have side // effects; therefore, it's safe to sink and possibly *not* execute. - return I && I->hasOneUse() && isSafeToSpeculativelyExecute(I) && + return I && I->hasOneUse() && + isSafeToSpeculativelyExecute(I, + /* PreservesOpCharacteristics */ true) && TTI->isExpensiveToSpeculativelyExecute(I); } diff --git a/llvm/lib/CodeGen/ExpandVectorPredication.cpp b/llvm/lib/CodeGen/ExpandVectorPredication.cpp --- a/llvm/lib/CodeGen/ExpandVectorPredication.cpp +++ b/llvm/lib/CodeGen/ExpandVectorPredication.cpp @@ -125,7 +125,8 @@ // Fallback to whether the intrinsic is speculatable. std::optional OpcOpt = VPI.getFunctionalOpcode(); unsigned FunctionalOpc = OpcOpt.value_or((unsigned)Instruction::Call); - return isSafeToSpeculativelyExecuteWithOpcode(FunctionalOpc, &VPI); + return isSafeToSpeculativelyExecuteWithOpcode( + FunctionalOpc, &VPI, /* PreservesOpCharacteristics */ true); } //// } Helpers diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -2777,8 +2777,10 @@ // computed between guards. Instruction *NextInst = II->getNextNonDebugInstruction(); for (unsigned i = 0; i < GuardWideningWindow; i++) { - // Note: Using context-free form to avoid compile time blow up - if (!isSafeToSpeculativelyExecute(NextInst)) + // Note: Using context-free form to avoid compile time blow up. Likewise + // don't set 'PreservesOpCharacteristics' to keep less expensive analysis. + if (!isSafeToSpeculativelyExecute(NextInst, + /* PreservesOpCharacteristics */ false)) break; NextInst = NextInst->getNextNonDebugInstruction(); } diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp --- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp @@ -1245,7 +1245,8 @@ return false; auto *I = dyn_cast(V); - if (!I || !I->hasOneUse() || !isSafeToSpeculativelyExecute(I)) + if (!I || !I->hasOneUse() || + !isSafeToSpeculativelyExecute(I, /* PreservesOpCharacteristics */ true)) return false; bool Changed = false; diff --git a/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp b/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp --- a/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp @@ -2733,7 +2733,8 @@ return nullptr; auto *BinOp = cast(Op0); - if (!isSafeToSpeculativelyExecute(BinOp)) + if (!isSafeToSpeculativelyExecute(BinOp, + /* PreservesOpCharacteristics */ false)) return nullptr; Value *NewBO = Builder.CreateBinOp(BinOp->getOpcode(), X, Y); diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -1431,7 +1431,8 @@ // It may not be safe to reorder shuffles and things like div, urem, etc. // because we may trap when executing those ops on unknown vector elements. // See PR20059. - if (!isSafeToSpeculativelyExecute(&Inst)) + if (!isSafeToSpeculativelyExecute(&Inst, + /* PreservesOpCharacteristics */ false)) return nullptr; auto createBinOpShuffle = [&](Value *X, Value *Y, ArrayRef M) { diff --git a/llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp b/llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp --- a/llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp +++ b/llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp @@ -478,7 +478,8 @@ static bool isHoistable(Instruction *I, DominatorTree &DT) { if (!isHoistableInstructionType(I)) return false; - return isSafeToSpeculativelyExecute(I, nullptr, nullptr, &DT); + return isSafeToSpeculativelyExecute(I, /* PreservesOpCharacteristics */ true, + nullptr, nullptr, &DT); } // Recursively traverse the use-def chains of the given value and return a set diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp --- a/llvm/lib/Transforms/Scalar/GVN.cpp +++ b/llvm/lib/Transforms/Scalar/GVN.cpp @@ -1528,11 +1528,14 @@ // to speculatively execute the load at that points. if (MustEnsureSafetyOfSpeculativeExecution) { if (CriticalEdgePred.size()) - if (!isSafeToSpeculativelyExecute(Load, LoadBB->getFirstNonPHI(), AC, DT)) + if (!isSafeToSpeculativelyExecute(Load, + /* PreservesOpCharacteristics */ true, + LoadBB->getFirstNonPHI(), AC, DT)) return false; for (auto &PL : PredLoads) - if (!isSafeToSpeculativelyExecute(Load, PL.first->getTerminator(), AC, - DT)) + if (!isSafeToSpeculativelyExecute(Load, + /* PreservesOpCharacteristics */ true, + PL.first->getTerminator(), AC, DT)) return false; } @@ -2846,7 +2849,8 @@ Instruction *PREInstr = nullptr; if (NumWithout != 0) { - if (!isSafeToSpeculativelyExecute(CurInst)) { + if (!isSafeToSpeculativelyExecute(CurInst, + /* PreservesOpCharacteristics */ true)) { // It is only valid to insert a new instruction if the current instruction // is always executed. An instruction with implicit control flow could // prevent us from doing it. If we cannot speculate the execution, then diff --git a/llvm/lib/Transforms/Scalar/GuardWidening.cpp b/llvm/lib/Transforms/Scalar/GuardWidening.cpp --- a/llvm/lib/Transforms/Scalar/GuardWidening.cpp +++ b/llvm/lib/Transforms/Scalar/GuardWidening.cpp @@ -535,7 +535,10 @@ if (!Inst || DT.dominates(Inst, Loc) || Visited.count(Inst)) return true; - if (!isSafeToSpeculativelyExecute(Inst, Loc, &AC, &DT) || + // TODO: We may be able to set PreservesOpCharacteristics to true. AFAICT the + // only reason its not possible is because of the assert in makeAvailableAt. + if (!isSafeToSpeculativelyExecute( + Inst, /* PreservesOpCharacteristics */ false, Loc, &AC, &DT) || Inst->mayReadFromMemory()) return false; @@ -555,7 +558,8 @@ if (!Inst || DT.dominates(Inst, Loc)) return; - assert(isSafeToSpeculativelyExecute(Inst, Loc, &AC, &DT) && + assert(isSafeToSpeculativelyExecute( + Inst, /* PreservesOpCharacteristics */ false, Loc, &AC, &DT) && !Inst->mayReadFromMemory() && "Should've checked with canBeHoistedTo!"); diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp --- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp +++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp @@ -1374,7 +1374,8 @@ // It requires domination tree analysis, so for this simple case it is an // overkill. if (PredsScanned.size() != AvailablePreds.size() && - !isSafeToSpeculativelyExecute(LoadI)) + !isSafeToSpeculativelyExecute(LoadI, + /* PreservesOpCharacteristics */ true)) for (auto I = LoadBB->begin(); &*I != LoadI; ++I) if (!isGuaranteedToTransferExecutionToSuccessor(&*I)) return false; diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp --- a/llvm/lib/Transforms/Scalar/LICM.cpp +++ b/llvm/lib/Transforms/Scalar/LICM.cpp @@ -1768,7 +1768,8 @@ OptimizationRemarkEmitter *ORE, const Instruction *CtxI, AssumptionCache *AC, bool AllowSpeculation) { if (AllowSpeculation && - isSafeToSpeculativelyExecute(&Inst, CtxI, AC, DT, TLI)) + isSafeToSpeculativelyExecute(&Inst, /* PreservesOpCharacteristics */ true, + CtxI, AC, DT, TLI)) return true; bool GuaranteedToExecute = diff --git a/llvm/lib/Transforms/Scalar/LoopFlatten.cpp b/llvm/lib/Transforms/Scalar/LoopFlatten.cpp --- a/llvm/lib/Transforms/Scalar/LoopFlatten.cpp +++ b/llvm/lib/Transforms/Scalar/LoopFlatten.cpp @@ -553,7 +553,8 @@ for (auto &I : *B) { if (!isa(&I) && !I.isTerminator() && - !isSafeToSpeculativelyExecute(&I)) { + !isSafeToSpeculativelyExecute( + &I, /* PreservesOpCharacteristics */ true)) { LLVM_DEBUG(dbgs() << "Cannot flatten because instruction may have " "side effects: "; I.dump()); diff --git a/llvm/lib/Transforms/Scalar/LoopRerollPass.cpp b/llvm/lib/Transforms/Scalar/LoopRerollPass.cpp --- a/llvm/lib/Transforms/Scalar/LoopRerollPass.cpp +++ b/llvm/lib/Transforms/Scalar/LoopRerollPass.cpp @@ -1276,7 +1276,8 @@ // needed because otherwise isSafeToSpeculativelyExecute returns // false on PHI nodes. if (!isa(I) && !isUnorderedLoadStore(I) && - !isSafeToSpeculativelyExecute(I)) + !isSafeToSpeculativelyExecute( + I, /* PreservesOpCharacteristics */ true)) // Intervening instructions cause side effects. FutureSideEffects = true; } @@ -1308,10 +1309,13 @@ // side effects, and this instruction might also, then we can't reorder // them, and this matching fails. As an exception, we allow the alias // set tracker to handle regular (unordered) load/store dependencies. - if (FutureSideEffects && ((!isUnorderedLoadStore(BaseInst) && - !isSafeToSpeculativelyExecute(BaseInst)) || - (!isUnorderedLoadStore(RootInst) && - !isSafeToSpeculativelyExecute(RootInst)))) { + if (FutureSideEffects && + ((!isUnorderedLoadStore(BaseInst) && + !isSafeToSpeculativelyExecute( + BaseInst, /* PreservesOpCharacteristics */ true)) || + (!isUnorderedLoadStore(RootInst) && + !isSafeToSpeculativelyExecute( + RootInst, /* PreservesOpCharacteristics */ true)))) { LLVM_DEBUG(dbgs() << "LRR: iteration root match failed at " << *BaseInst << " vs. " << *RootInst << " (side effects prevent reordering)\n"); diff --git a/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp b/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp --- a/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp +++ b/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp @@ -295,7 +295,9 @@ unsigned NotHoistedInstCount = 0; for (const auto &I : FromBlock) { const InstructionCost Cost = ComputeSpeculationCost(&I, *TTI); - if (Cost.isValid() && isSafeToSpeculativelyExecute(&I) && + if (Cost.isValid() && + isSafeToSpeculativelyExecute(&I, + /* PreservesOpCharacteristics */ false) && AllPrecedingUsesFromBlockHoisted(&I)) { TotalSpeculationCost += Cost; if (TotalSpeculationCost > SpecExecMaxSpeculationCost) diff --git a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp --- a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp +++ b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp @@ -364,7 +364,7 @@ // Check if there exists instructions which may throw, may synchonize, or may // never return, from I to InsertPoint. - if (!isSafeToSpeculativelyExecute(&I)) + if (!isSafeToSpeculativelyExecute(&I, /* PreservesOpCharacteristics */ true)) if (llvm::any_of(InstsToCheck, [](Instruction *I) { if (I->mayThrow()) return true; diff --git a/llvm/lib/Transforms/Utils/FlattenCFG.cpp b/llvm/lib/Transforms/Utils/FlattenCFG.cpp --- a/llvm/lib/Transforms/Utils/FlattenCFG.cpp +++ b/llvm/lib/Transforms/Utils/FlattenCFG.cpp @@ -188,7 +188,11 @@ for (BasicBlock::iterator BI = Pred->begin(), BE = PBI->getIterator(); BI != BE;) { Instruction *CI = &*BI++; - if (isa(CI) || !isSafeToSpeculativelyExecute(CI)) + // TODO: Can we set PreservesOpCharacteristics to true? Concern is + // `PreservesOpCharacteristics` might be based on dominating conditions + // which this transform might destroy. + if (isa(CI) || !isSafeToSpeculativelyExecute( + CI, /* PreservesOpCharacteristics */ false)) return false; } } else { @@ -474,7 +478,8 @@ for (BasicBlock::iterator BI(PBI2), BE(PTI2); BI != BE; ++BI) { Instruction *CI = &*BI; if (isa(CI) || CI->mayHaveSideEffects() || - !isSafeToSpeculativelyExecute(CI)) + !isSafeToSpeculativelyExecute(CI, + /* PreservesOpCharacteristics */ true)) return false; } diff --git a/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp b/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp --- a/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp +++ b/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp @@ -705,7 +705,8 @@ for (BasicBlock::iterator I = Begin; I != End; ++I) { - if (!isSafeToSpeculativelyExecute(&*I)) + if (!isSafeToSpeculativelyExecute(&*I, + /* PreservesOpCharacteristics */ true)) return false; if (isa(I)) diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -384,7 +384,8 @@ static InstructionCost computeSpeculationCost(const User *I, const TargetTransformInfo &TTI) { assert((!isa(I) || - isSafeToSpeculativelyExecute(cast(I))) && + isSafeToSpeculativelyExecute( + cast(I), /* PreservesOpCharacteristics */ false)) && "Instruction is not safe to speculatively execute!"); return TTI.getInstructionCost(I, TargetTransformInfo::TCK_SizeAndLatency); } @@ -446,7 +447,7 @@ // Okay, it looks like the instruction IS in the "condition". Check to // see if it's a cheap instruction to unconditionally compute, and if it // only uses stuff defined outside of the condition. If so, hoist it out. - if (!isSafeToSpeculativelyExecute(I)) + if (!isSafeToSpeculativelyExecute(I, /* PreservesOpCharacteristics */ false)) return false; Cost += computeSpeculationCost(I, TTI); @@ -1461,7 +1462,8 @@ // Reordering across an instruction which does not necessarily transfer // control to the next instruction is speculation. - if ((Flags & SkipImplicitControlFlow) && !isSafeToSpeculativelyExecute(I)) + if ((Flags & SkipImplicitControlFlow) && + !isSafeToSpeculativelyExecute(I, /* PreservesOpCharacteristics */ true)) return false; // Hoisting of llvm.deoptimize is only legal together with the next return @@ -2261,7 +2263,8 @@ int Idx = 0; bool Profitable = false; while (Idx < ScanIdx) { - if (!isSafeToSpeculativelyExecute((*LRI)[0])) { + if (!isSafeToSpeculativelyExecute( + (*LRI)[0], /* PreservesOpCharacteristics */ true)) { Profitable = true; break; } @@ -2919,7 +2922,8 @@ return false; // Don't hoist the instruction if it's unsafe or expensive. - if (!isSafeToSpeculativelyExecute(&I) && + if (!isSafeToSpeculativelyExecute(&I, + /* PreservesOpCharacteristics */ false) && !(HoistCondStores && (SpeculatedStoreValue = isSafeToSpeculateStore( &I, BB, ThenBB, EndBB)))) return false; @@ -3764,7 +3768,8 @@ if (isa(I) || isa(I)) continue; // I must be safe to execute unconditionally. - if (!isSafeToSpeculativelyExecute(&I)) + if (!isSafeToSpeculativelyExecute(&I, + /* PreservesOpCharacteristics */ false)) return false; SawVectorOp |= isVectorOp(I); diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp --- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp +++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp @@ -4450,7 +4450,8 @@ case Instruction::URem: // TODO: We can use the loop-preheader as context point here and get // context sensitive reasoning - return !isSafeToSpeculativelyExecute(I); + return !isSafeToSpeculativelyExecute(I, + /* PreservesOpCharacteristics */ true); case Instruction::Call: return Legal->isMaskRequired(I); } @@ -4463,7 +4464,8 @@ I->getOpcode() == Instruction::SDiv || I->getOpcode() == Instruction::SRem || I->getOpcode() == Instruction::URem); - assert(!isSafeToSpeculativelyExecute(I)); + assert( + !isSafeToSpeculativelyExecute(I, /* PreservesOpCharacteristics */ true)); const TTI::TargetCostKind CostKind = TTI::TCK_RecipThroughput; diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp --- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp +++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp @@ -11593,7 +11593,9 @@ if (!isGuaranteedToTransferExecutionToSuccessor(BundleMember->Inst)) { for (Instruction *I = BundleMember->Inst->getNextNode(); I != ScheduleEnd; I = I->getNextNode()) { - if (isSafeToSpeculativelyExecute(I, &*BB->begin(), SLP->AC)) + if (isSafeToSpeculativelyExecute( + I, /* PreservesOpCharacteristics */ true, &*BB->begin(), + SLP->AC)) continue; // Add the dependency diff --git a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp --- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp +++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp @@ -557,7 +557,7 @@ bool VectorCombine::foldExtractExtract(Instruction &I) { // It is not safe to transform things like div, urem, etc. because we may // create undefined behavior when executing those on unknown vector elements. - if (!isSafeToSpeculativelyExecute(&I)) + if (!isSafeToSpeculativelyExecute(&I, /* PreservesOpCharacteristics */ true)) return false; Instruction *I0, *I1; diff --git a/llvm/test/Transforms/LICM/speculate-div.ll b/llvm/test/Transforms/LICM/speculate-div.ll --- a/llvm/test/Transforms/LICM/speculate-div.ll +++ b/llvm/test/Transforms/LICM/speculate-div.ll @@ -4,7 +4,7 @@ declare void @maythrow() declare void @use(i16) -define void @sdiv_not_ok(i16 %n, i16 %xx) { +define void @sdiv_not_ok(i16 %n, i16 noundef %xx) { ; CHECK-LABEL: @sdiv_not_ok( ; CHECK-NEXT: entry: ; CHECK-NEXT: [[X:%.*]] = or i16 [[XX:%.*]], 1 @@ -25,7 +25,7 @@ br label %loop } -define void @srem_not_ok2(i16 %nn, i16 %x) { +define void @srem_not_ok2(i16 %nn, i16 noundef %x) { ; CHECK-LABEL: @srem_not_ok2( ; CHECK-NEXT: entry: ; CHECK-NEXT: [[N:%.*]] = and i16 [[NN:%.*]], 1323 @@ -46,15 +46,15 @@ br label %loop } -define void @sdiv_ok(i16 %n, i16 %xx) { +define void @sdiv_ok(i16 %n, i16 noundef %xx) { ; CHECK-LABEL: @sdiv_ok( ; CHECK-NEXT: entry: ; CHECK-NEXT: [[XO:%.*]] = or i16 [[XX:%.*]], 1 ; CHECK-NEXT: [[X:%.*]] = and i16 [[XO]], 123 +; CHECK-NEXT: [[DIV:%.*]] = sdiv i16 [[N:%.*]], [[X]] ; CHECK-NEXT: br label [[LOOP:%.*]] ; CHECK: loop: ; CHECK-NEXT: call void @maythrow() -; CHECK-NEXT: [[DIV:%.*]] = sdiv i16 [[N:%.*]], [[X]] ; CHECK-NEXT: call void @use(i16 [[DIV]]) ; CHECK-NEXT: br label [[LOOP]] ; @@ -69,15 +69,15 @@ br label %loop } -define void @srem_ok2(i16 %nn, i16 %xx) { +define void @srem_ok2(i16 noundef %nn, i16 noundef %xx) { ; CHECK-LABEL: @srem_ok2( ; CHECK-NEXT: entry: ; CHECK-NEXT: [[N:%.*]] = and i16 [[NN:%.*]], 123 ; CHECK-NEXT: [[X:%.*]] = or i16 [[XX:%.*]], 1 +; CHECK-NEXT: [[DIV:%.*]] = srem i16 [[N]], [[X]] ; CHECK-NEXT: br label [[LOOP:%.*]] ; CHECK: loop: ; CHECK-NEXT: call void @maythrow() -; CHECK-NEXT: [[DIV:%.*]] = srem i16 [[N]], [[X]] ; CHECK-NEXT: call void @use(i16 [[DIV]]) ; CHECK-NEXT: br label [[LOOP]] ; @@ -92,7 +92,53 @@ br label %loop } -define void @udiv_not_ok(i16 %n, i16 %xx) { +define void @sdiv_not_ok3_maybe_poison_denum(i16 noundef %nn, i16 %xx) { +; CHECK-LABEL: @sdiv_not_ok3_maybe_poison_denum( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[N:%.*]] = and i16 [[NN:%.*]], 123 +; CHECK-NEXT: [[X:%.*]] = or i16 [[XX:%.*]], 1 +; CHECK-NEXT: br label [[LOOP:%.*]] +; CHECK: loop: +; CHECK-NEXT: call void @maythrow() +; CHECK-NEXT: [[DIV:%.*]] = sdiv i16 [[N]], [[X]] +; CHECK-NEXT: call void @use(i16 [[DIV]]) +; CHECK-NEXT: br label [[LOOP]] +; +entry: + %n = and i16 %nn, 123 + %x = or i16 %xx, 1 + br label %loop +loop: + call void @maythrow() + %div = sdiv i16 %n, %x + call void @use(i16 %div) + br label %loop +} + +define void @sdiv_not_ok3_maybe_poison_num(i16 %nn, i16 noundef %xx) { +; CHECK-LABEL: @sdiv_not_ok3_maybe_poison_num( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[N:%.*]] = and i16 [[NN:%.*]], 123 +; CHECK-NEXT: [[X:%.*]] = or i16 [[XX:%.*]], 1 +; CHECK-NEXT: br label [[LOOP:%.*]] +; CHECK: loop: +; CHECK-NEXT: call void @maythrow() +; CHECK-NEXT: [[DIV:%.*]] = sdiv i16 [[N]], [[X]] +; CHECK-NEXT: call void @use(i16 [[DIV]]) +; CHECK-NEXT: br label [[LOOP]] +; +entry: + %n = and i16 %nn, 123 + %x = or i16 %xx, 1 + br label %loop +loop: + call void @maythrow() + %div = sdiv i16 %n, %x + call void @use(i16 %div) + br label %loop +} + +define void @udiv_not_ok(i16 %n, i16 noundef %xx) { ; CHECK-LABEL: @udiv_not_ok( ; CHECK-NEXT: entry: ; CHECK-NEXT: [[X:%.*]] = xor i16 [[XX:%.*]], 1 @@ -113,10 +159,31 @@ br label %loop } -define void @udiv_ok(i16 %n, i16 %xx) { +define void @udiv_ok(i16 %n, i16 noundef %xx) { ; CHECK-LABEL: @udiv_ok( ; CHECK-NEXT: entry: ; CHECK-NEXT: [[X:%.*]] = or i16 [[XX:%.*]], 1 +; CHECK-NEXT: [[DIV:%.*]] = udiv i16 [[N:%.*]], [[X]] +; CHECK-NEXT: br label [[LOOP:%.*]] +; CHECK: loop: +; CHECK-NEXT: call void @maythrow() +; CHECK-NEXT: call void @use(i16 [[DIV]]) +; CHECK-NEXT: br label [[LOOP]] +; +entry: + %x = or i16 %xx, 1 + br label %loop +loop: + call void @maythrow() + %div = udiv i16 %n, %x + call void @use(i16 %div) + br label %loop +} + +define void @urem_not_ok_maybe_poison(i16 %n, i16 %xx) { +; CHECK-LABEL: @urem_not_ok_maybe_poison( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[X:%.*]] = or i16 [[XX:%.*]], 1 ; CHECK-NEXT: br label [[LOOP:%.*]] ; CHECK: loop: ; CHECK-NEXT: call void @maythrow()