[llvm] 8ee913d - [IR] Remove Constant::canTrap() (NFC)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 6 01:37:18 PDT 2022
Author: Nikita Popov
Date: 2022-07-06T10:36:47+02:00
New Revision: 8ee913d83b170729300a2381158c77acdb3ac8f8
URL: https://github.com/llvm/llvm-project/commit/8ee913d83b170729300a2381158c77acdb3ac8f8
DIFF: https://github.com/llvm/llvm-project/commit/8ee913d83b170729300a2381158c77acdb3ac8f8.diff
LOG: [IR] Remove Constant::canTrap() (NFC)
As integer div/rem constant expressions are no longer supported,
constants can no longer trap and are always safe to speculate.
Remove the Constant::canTrap() method and its usages.
Added:
Modified:
llvm/include/llvm/IR/Constant.h
llvm/lib/Analysis/InstructionSimplify.cpp
llvm/lib/Analysis/ValueTracking.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
llvm/lib/IR/Constants.cpp
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
llvm/lib/Transforms/IPO/GlobalOpt.cpp
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/Constant.h b/llvm/include/llvm/IR/Constant.h
index a97372ebbad28..09fb2c98bff4b 100644
--- a/llvm/include/llvm/IR/Constant.h
+++ b/llvm/include/llvm/IR/Constant.h
@@ -115,10 +115,6 @@ class Constant : public User {
/// any constant expressions.
bool containsConstantExpression() const;
- /// Return true if evaluation of this constant could trap. This is true for
- /// things like constant expressions that could divide by zero.
- bool canTrap() const;
-
/// Return true if the value can vary between threads.
bool isThreadDependent() const;
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 013e4d6489fa8..6722c4396332c 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -4849,12 +4849,6 @@ static Value *simplifyPHINode(PHINode *PN, ArrayRef<Value *> IncomingValues,
return UndefValue::get(PN->getType());
if (HasUndefInput) {
- // We cannot start executing a trapping constant expression on more control
- // flow paths.
- auto *C = dyn_cast<Constant>(CommonValue);
- if (C && C->canTrap())
- return nullptr;
-
// If we have a PHI node like phi(X, undef, X), where X is defined by some
// instruction, we cannot return X as the result of the PHI node unless it
// dominates the PHI block.
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 05d5e47bb8d74..0e8c190cc9af0 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -4715,11 +4715,6 @@ bool llvm::isSafeToSpeculativelyExecuteWithOpcode(unsigned Opcode,
}
#endif
- for (unsigned i = 0, e = Inst->getNumOperands(); i != e; ++i)
- if (Constant *C = dyn_cast<Constant>(Inst->getOperand(i)))
- if (C->canTrap())
- return false;
-
switch (Opcode) {
default:
return true;
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 2b63359c2b1b6..76105b5c43c09 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -27,7 +27,6 @@
#include "llvm/Analysis/EHPersonalities.h"
#include "llvm/Analysis/LazyBlockFrequencyInfo.h"
#include "llvm/Analysis/LegacyDivergenceAnalysis.h"
-#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/OptimizationRemarkEmitter.h"
#include "llvm/Analysis/ProfileSummaryInfo.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
@@ -345,47 +344,6 @@ void SelectionDAGISel::getAnalysisUsage(AnalysisUsage &AU) const {
MachineFunctionPass::getAnalysisUsage(AU);
}
-/// SplitCriticalSideEffectEdges - Look for critical edges with a PHI value that
-/// may trap on it. In this case we have to split the edge so that the path
-/// through the predecessor block that doesn't go to the phi block doesn't
-/// execute the possibly trapping instruction. If available, we pass domtree
-/// and loop info to be updated when we split critical edges. This is because
-/// SelectionDAGISel preserves these analyses.
-/// This is required for correctness, so it must be done at -O0.
-///
-static void SplitCriticalSideEffectEdges(Function &Fn, DominatorTree *DT,
- LoopInfo *LI) {
- // Loop for blocks with phi nodes.
- for (BasicBlock &BB : Fn) {
- PHINode *PN = dyn_cast<PHINode>(BB.begin());
- if (!PN) continue;
-
- ReprocessBlock:
- // For each block with a PHI node, check to see if any of the input values
- // are potentially trapping constant expressions. Constant expressions are
- // the only potentially trapping value that can occur as the argument to a
- // PHI.
- for (BasicBlock::iterator I = BB.begin(); (PN = dyn_cast<PHINode>(I)); ++I)
- for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
- Constant *C = dyn_cast<Constant>(PN->getIncomingValue(i));
- if (!C || !C->canTrap()) continue;
-
- // The only case we have to worry about is when the edge is critical.
- // Since this block has a PHI Node, we assume it has multiple input
- // edges: check to see if the pred has multiple successors.
- BasicBlock *Pred = PN->getIncomingBlock(i);
- if (Pred->getTerminator()->getNumSuccessors() == 1)
- continue;
-
- // Okay, we have to split this edge.
- SplitCriticalEdge(
- Pred->getTerminator(), GetSuccessorNumber(Pred, &BB),
- CriticalEdgeSplittingOptions(DT, LI).setMergeIdenticalEdges());
- goto ReprocessBlock;
- }
- }
-}
-
static void computeUsesMSVCFloatingPoint(const Triple &TT, const Function &F,
MachineModuleInfo &MMI) {
// Only needed for MSVC
@@ -447,8 +405,6 @@ bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
ORE = std::make_unique<OptimizationRemarkEmitter>(&Fn);
auto *DTWP = getAnalysisIfAvailable<DominatorTreeWrapperPass>();
DominatorTree *DT = DTWP ? &DTWP->getDomTree() : nullptr;
- auto *LIWP = getAnalysisIfAvailable<LoopInfoWrapperPass>();
- LoopInfo *LI = LIWP ? &LIWP->getLoopInfo() : nullptr;
auto *PSI = &getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
BlockFrequencyInfo *BFI = nullptr;
if (PSI && PSI->hasProfileSummary() && OptLevel != CodeGenOpt::None)
@@ -456,8 +412,6 @@ bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
LLVM_DEBUG(dbgs() << "\n\n\n=== " << Fn.getName() << "\n");
- SplitCriticalSideEffectEdges(const_cast<Function &>(Fn), DT, LI);
-
CurDAG->init(*MF, *ORE, this, LibInfo,
getAnalysisIfAvailable<LegacyDivergenceAnalysis>(), PSI, BFI);
FuncInfo->set(Fn, *MF, CurDAG);
diff --git a/llvm/lib/IR/Constants.cpp b/llvm/lib/IR/Constants.cpp
index 0fb0db37460c3..650b076f8d168 100644
--- a/llvm/lib/IR/Constants.cpp
+++ b/llvm/lib/IR/Constants.cpp
@@ -559,10 +559,6 @@ void llvm::deleteConstant(Constant *C) {
}
}
-bool Constant::canTrap() const {
- return false;
-}
-
/// Check if C contains a GlobalValue for which Predicate is true.
static bool
ConstHasGlobalValuePredicate(const Constant *C,
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 4d99ce7e3175e..4b4fdaad3a147 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -5526,8 +5526,7 @@ struct AAValueSimplifyImpl : AAValueSimplify {
if (SimpleV.getValue())
EffectiveV = SimpleV.getValue();
if (auto *C = dyn_cast<Constant>(EffectiveV))
- if (!C->canTrap())
- return C;
+ return C;
if (CtxI && AA::isValidAtPosition(AA::ValueAndContext(*EffectiveV, *CtxI),
A.getInfoCache()))
return ensureType(A, *EffectiveV, Ty, CtxI, Check);
diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index 1a1bde4f0668f..1ad6e2b2a1d24 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -1584,11 +1584,6 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
}
Value *StoredOnceValue = GS.getStoredOnceValue();
if (GS.StoredType == GlobalStatus::StoredOnce && StoredOnceValue) {
- // Avoid speculating constant expressions that might trap (div/rem).
- auto *SOVConstant = dyn_cast<Constant>(StoredOnceValue);
- if (SOVConstant && SOVConstant->canTrap())
- return Changed;
-
Function &StoreFn =
const_cast<Function &>(*GS.StoredOnceStore->getFunction());
bool CanHaveNonUndefGlobalInitializer =
@@ -1601,6 +1596,7 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
// This is restricted to address spaces that allow globals to have
// initializers. NVPTX, for example, does not support initializers for
// shared memory (AS 3).
+ auto *SOVConstant = dyn_cast<Constant>(StoredOnceValue);
if (SOVConstant && isa<UndefValue>(GV->getInitializer()) &&
DL.getTypeAllocSize(SOVConstant->getType()) ==
DL.getTypeAllocSize(GV->getValueType()) &&
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index acfab42d957d4..9f6d36b85522d 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -1436,7 +1436,7 @@ Instruction *InstCombinerImpl::foldICmpWithConstant(ICmpInst &Cmp) {
// icmp(phi(C1, C2, ...), C) -> phi(icmp(C1, C), icmp(C2, C), ...).
Constant *C = dyn_cast<Constant>(Op1);
- if (!C || C->canTrap())
+ if (!C)
return nullptr;
if (auto *Phi = dyn_cast<PHINode>(Op0))
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 653ebd9bf46d2..322046e7ef6f0 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -382,13 +382,6 @@ static InstructionCost computeSpeculationCost(const User *I,
return TTI.getUserCost(I, TargetTransformInfo::TCK_SizeAndLatency);
}
-/// Check whether this is a potentially trapping constant.
-static bool canTrap(const Value *V) {
- if (auto *C = dyn_cast<Constant>(V))
- return C->canTrap();
- return false;
-}
-
/// If we have a merge point of an "if condition" as accepted above,
/// return true if the specified value dominates the block. We
/// don't handle the true generality of domination here, just a special case
@@ -421,9 +414,9 @@ static bool dominatesMergePoint(Value *V, BasicBlock *BB,
Instruction *I = dyn_cast<Instruction>(V);
if (!I) {
- // Non-instructions all dominate instructions, but not all constantexprs
- // can be executed unconditionally.
- return !canTrap(V);
+ // Non-instructions dominate all instructions and can be executed
+ // unconditionally.
+ return true;
}
BasicBlock *PBB = I->getParent();
@@ -2680,9 +2673,6 @@ static bool validateAndCostRequiredSelects(BasicBlock *BB, BasicBlock *ThenBB,
passingValueIsAlwaysUndefined(ThenV, &PN))
return false;
- if (canTrap(OrigV) || canTrap(ThenV))
- return false;
-
HaveRewritablePHIs = true;
ConstantExpr *OrigCE = dyn_cast<ConstantExpr>(OrigV);
ConstantExpr *ThenCE = dyn_cast<ConstantExpr>(ThenV);
@@ -3582,13 +3572,6 @@ bool llvm::FoldBranchToCommonDest(BranchInst *BI, DomTreeUpdater *DTU,
Cond->getParent() != BB || !Cond->hasOneUse())
return false;
- // Cond is known to be a compare or binary operator. Check to make sure that
- // neither operand is a potentially-trapping constant expression.
- if (canTrap(Cond->getOperand(0)))
- return false;
- if (canTrap(Cond->getOperand(1)))
- return false;
-
// Finally, don't infinitely unroll conditional loops.
if (is_contained(successors(BB), BB))
return false;
@@ -4096,9 +4079,6 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
if (tryWidenCondBranchToCondBranch(PBI, BI, DTU))
return true;
- if (canTrap(BI->getCondition()))
- return false;
-
// If both branches are conditional and both contain stores to the same
// address, remove the stores from the conditionals and create a conditional
// merged store at the end.
@@ -4140,10 +4120,6 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
// insertion of a large number of select instructions. For targets
// without predication/cmovs, this is a big pessimization.
- // Also do not perform this transformation if any phi node in the common
- // destination block can trap when reached by BB or PBB (PR17073). In that
- // case, it would be unsafe to hoist the operation into a select instruction.
-
BasicBlock *CommonDest = PBI->getSuccessor(PBIOp);
BasicBlock *RemovedDest = PBI->getSuccessor(PBIOp ^ 1);
unsigned NumPhis = 0;
@@ -4151,16 +4127,6 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
++II, ++NumPhis) {
if (NumPhis > 2) // Disable this xform.
return false;
-
- PHINode *PN = cast<PHINode>(II);
- Value *BIV = PN->getIncomingValueForBlock(BB);
- if (canTrap(BIV))
- return false;
-
- unsigned PBBIdx = PN->getBasicBlockIndex(PBI->getParent());
- Value *PBIV = PN->getIncomingValue(PBBIdx);
- if (canTrap(PBIV))
- return false;
}
// Finally, if everything is ok, fold the branches to logical ops.
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
index 8b5aca8d2ade2..59954429969e4 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
@@ -386,20 +386,6 @@ static bool isUniformLoopNest(Loop *Lp, Loop *OuterLp) {
return true;
}
-/// Check whether it is safe to if-convert this phi node.
-///
-/// Phi nodes with constant expressions that can trap are not safe to if
-/// convert.
-static bool canIfConvertPHINodes(BasicBlock *BB) {
- for (PHINode &Phi : BB->phis()) {
- for (Value *V : Phi.incoming_values())
- if (auto *C = dyn_cast<Constant>(V))
- if (C->canTrap())
- return false;
- }
- return true;
-}
-
static Type *convertPointerToIntegerType(const DataLayout &DL, Type *Ty) {
if (Ty->isPointerTy())
return DL.getIntPtrType(Ty);
@@ -1097,13 +1083,6 @@ bool LoopVectorizationLegality::blockCanBePredicated(
SmallPtrSetImpl<const Instruction *> &MaskedOp,
SmallPtrSetImpl<Instruction *> &ConditionalAssumes) const {
for (Instruction &I : *BB) {
- // Check that we don't have a constant expression that can trap as operand.
- for (Value *Operand : I.operands()) {
- if (auto *C = dyn_cast<Constant>(Operand))
- if (C->canTrap())
- return false;
- }
-
// We can predicate blocks with calls to assume, as long as we drop them in
// case we flatten the CFG via predication.
if (match(&I, m_Intrinsic<Intrinsic::assume>())) {
@@ -1211,13 +1190,6 @@ bool LoopVectorizationLegality::canVectorizeWithIfConvert() {
BB->getTerminator());
return false;
}
- } else if (BB != Header && !canIfConvertPHINodes(BB)) {
- reportVectorizationFailure(
- "Control flow cannot be substituted for a select",
- "control flow cannot be substituted for a select",
- "NoCFGForSelect", ORE, TheLoop,
- BB->getTerminator());
- return false;
}
}
More information about the llvm-commits
mailing list