Skip to content

Commit

Permalink
[ValueTracking] Use isSafeToSpeculativelyExecuteWithVariableReplaced(…
Browse files Browse the repository at this point in the history
…) in more places (llvm#109149)

This replaces some uses of isSafeToSpeculativelyExecute() with
isSafeToSpeculativelyExecuteWithVariableReplaced(), in cases where we
are guarding against operand changes rather plain speculation.

I believe that this is NFC with the current implementation of the
function (as it only does something different from loads), but this
makes us more defensive against future generalizations.
  • Loading branch information
nikic authored Sep 19, 2024
1 parent 4e37816 commit dc6876f
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 4 deletions.
3 changes: 2 additions & 1 deletion llvm/lib/Analysis/LazyValueInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1572,7 +1572,8 @@ ValueLatticeElement LazyValueInfoImpl::getValueAtUse(const Use &U) {
// 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() ||
!isSafeToSpeculativelyExecuteWithVariableReplaced(CurrI))
break;
CurrU = &*CurrI->use_begin();
}
Expand Down
5 changes: 3 additions & 2 deletions llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,8 @@ Instruction *InstCombinerImpl::visitExtractElementInst(ExtractElementInst &EI) {
// it may make the operand poison.
BinaryOperator *BO;
if (match(SrcVec, m_BinOp(BO)) && cheapToScalarize(SrcVec, Index) &&
(HasKnownValidIndex || isSafeToSpeculativelyExecute(BO))) {
(HasKnownValidIndex ||
isSafeToSpeculativelyExecuteWithVariableReplaced(BO))) {
// extelt (binop X, Y), Index --> binop (extelt X, Index), (extelt Y, Index)
Value *X = BO->getOperand(0), *Y = BO->getOperand(1);
Value *E0 = Builder.CreateExtractElement(X, Index);
Expand Down Expand Up @@ -2777,7 +2778,7 @@ Instruction *InstCombinerImpl::simplifyBinOpSplats(ShuffleVectorInst &SVI) {
return nullptr;

auto *BinOp = cast<BinaryOperator>(Op0);
if (!isSafeToSpeculativelyExecute(BinOp))
if (!isSafeToSpeculativelyExecuteWithVariableReplaced(BinOp))
return nullptr;

Value *NewBO = Builder.CreateBinOp(BinOp->getOpcode(), X, Y);
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2105,7 +2105,7 @@ Instruction *InstCombinerImpl::foldVectorBinop(BinaryOperator &Inst) {
// 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 (!isSafeToSpeculativelyExecuteWithVariableReplaced(&Inst))
return nullptr;

auto createBinOpShuffle = [&](Value *X, Value *Y, ArrayRef<int> M) {
Expand Down

0 comments on commit dc6876f

Please sign in to comment.