[SimplifyCFG] Handle trapping aggregates (PR49839)

Handle the fact that not only constant expressions, but also
constant aggregates containing expressions can trap.

This still doesn't fix the original C reproducer, probably due to
more issues remaining in other passes.
This commit is contained in:
Nikita Popov 2022-06-13 14:55:14 +02:00
parent 9ecf423453
commit 571c713144
2 changed files with 28 additions and 25 deletions

View File

@ -382,6 +382,13 @@ static InstructionCost computeSpeculationCost(const User *I,
return TTI.getUserCost(I, TargetTransformInfo::TCK_SizeAndLatency); 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, /// If we have a merge point of an "if condition" as accepted above,
/// return true if the specified value dominates the block. We /// return true if the specified value dominates the block. We
/// don't handle the true generality of domination here, just a special case /// don't handle the true generality of domination here, just a special case
@ -416,10 +423,7 @@ static bool dominatesMergePoint(Value *V, BasicBlock *BB,
if (!I) { if (!I) {
// Non-instructions all dominate instructions, but not all constantexprs // Non-instructions all dominate instructions, but not all constantexprs
// can be executed unconditionally. // can be executed unconditionally.
if (ConstantExpr *C = dyn_cast<ConstantExpr>(V)) return !canTrap(V);
if (C->canTrap())
return false;
return true;
} }
BasicBlock *PBB = I->getParent(); BasicBlock *PBB = I->getParent();
@ -2675,15 +2679,15 @@ static bool validateAndCostRequiredSelects(BasicBlock *BB, BasicBlock *ThenBB,
passingValueIsAlwaysUndefined(ThenV, &PN)) passingValueIsAlwaysUndefined(ThenV, &PN))
return false; return false;
if (canTrap(OrigV) || canTrap(ThenV))
return false;
HaveRewritablePHIs = true; HaveRewritablePHIs = true;
ConstantExpr *OrigCE = dyn_cast<ConstantExpr>(OrigV); ConstantExpr *OrigCE = dyn_cast<ConstantExpr>(OrigV);
ConstantExpr *ThenCE = dyn_cast<ConstantExpr>(ThenV); ConstantExpr *ThenCE = dyn_cast<ConstantExpr>(ThenV);
if (!OrigCE && !ThenCE) if (!OrigCE && !ThenCE)
continue; // Known safe and cheap. continue; // Known cheap (FIXME: Maybe not true for aggregates).
if ((ThenCE && !isSafeToSpeculativelyExecute(ThenCE)) ||
(OrigCE && !isSafeToSpeculativelyExecute(OrigCE)))
return false;
InstructionCost OrigCost = OrigCE ? computeSpeculationCost(OrigCE, TTI) : 0; InstructionCost OrigCost = OrigCE ? computeSpeculationCost(OrigCE, TTI) : 0;
InstructionCost ThenCost = ThenCE ? computeSpeculationCost(ThenCE, TTI) : 0; InstructionCost ThenCost = ThenCE ? computeSpeculationCost(ThenCE, TTI) : 0;
InstructionCost MaxCost = InstructionCost MaxCost =
@ -3591,12 +3595,10 @@ bool llvm::FoldBranchToCommonDest(BranchInst *BI, DomTreeUpdater *DTU,
// Cond is known to be a compare or binary operator. Check to make sure that // Cond is known to be a compare or binary operator. Check to make sure that
// neither operand is a potentially-trapping constant expression. // neither operand is a potentially-trapping constant expression.
if (ConstantExpr *CE = dyn_cast<ConstantExpr>(Cond->getOperand(0))) if (canTrap(Cond->getOperand(0)))
if (CE->canTrap()) return false;
return false; if (canTrap(Cond->getOperand(1)))
if (ConstantExpr *CE = dyn_cast<ConstantExpr>(Cond->getOperand(1))) return false;
if (CE->canTrap())
return false;
// Finally, don't infinitely unroll conditional loops. // Finally, don't infinitely unroll conditional loops.
if (is_contained(successors(BB), BB)) if (is_contained(successors(BB), BB))
@ -4105,9 +4107,8 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
if (tryWidenCondBranchToCondBranch(PBI, BI, DTU)) if (tryWidenCondBranchToCondBranch(PBI, BI, DTU))
return true; return true;
if (auto *CE = dyn_cast<ConstantExpr>(BI->getCondition())) if (canTrap(BI->getCondition()))
if (CE->canTrap()) return false;
return false;
// If both branches are conditional and both contain stores to the same // If both branches are conditional and both contain stores to the same
// address, remove the stores from the conditionals and create a conditional // address, remove the stores from the conditionals and create a conditional
@ -4164,15 +4165,13 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
PHINode *PN = cast<PHINode>(II); PHINode *PN = cast<PHINode>(II);
Value *BIV = PN->getIncomingValueForBlock(BB); Value *BIV = PN->getIncomingValueForBlock(BB);
if (ConstantExpr *CE = dyn_cast<ConstantExpr>(BIV)) if (canTrap(BIV))
if (CE->canTrap()) return false;
return false;
unsigned PBBIdx = PN->getBasicBlockIndex(PBI->getParent()); unsigned PBBIdx = PN->getBasicBlockIndex(PBI->getParent());
Value *PBIV = PN->getIncomingValue(PBBIdx); Value *PBIV = PN->getIncomingValue(PBBIdx);
if (ConstantExpr *CE = dyn_cast<ConstantExpr>(PBIV)) if (canTrap(PBIV))
if (CE->canTrap()) return false;
return false;
} }
// Finally, if everything is ok, fold the branches to logical ops. // Finally, if everything is ok, fold the branches to logical ops.

View File

@ -72,8 +72,12 @@ bb10:
define <1 x i64> @trapping_const_agg(i1 %c) { define <1 x i64> @trapping_const_agg(i1 %c) {
; CHECK-LABEL: @trapping_const_agg( ; CHECK-LABEL: @trapping_const_agg(
; CHECK-NEXT: entry: ; CHECK-NEXT: entry:
; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[C:%.*]], <1 x i64> <i64 srem (i64 1, i64 ptrtoint (i32* @g to i64))>, <1 x i64> zeroinitializer ; CHECK-NEXT: br i1 [[C:%.*]], label [[IF:%.*]], label [[END:%.*]]
; CHECK-NEXT: ret <1 x i64> [[SPEC_SELECT]] ; CHECK: if:
; CHECK-NEXT: br label [[END]]
; CHECK: end:
; CHECK-NEXT: [[PHI:%.*]] = phi <1 x i64> [ zeroinitializer, [[ENTRY:%.*]] ], [ <i64 srem (i64 1, i64 ptrtoint (i32* @g to i64))>, [[IF]] ]
; CHECK-NEXT: ret <1 x i64> [[PHI]]
; ;
entry: entry:
br i1 %c, label %if, label %end br i1 %c, label %if, label %end