[ValueTracking] Make isGuaranteedNotToBeUndef() more precise

[nikic]

Currently isGuaranteedNotToBeUndef() is the same as isGuaranteedNotToBeUndefOrPoison(). This function is used in places where we only care about undef (due to multi-use issues), not poison.

Make it more precise by only considering instructions that can create undef (like loads or call), and ignore those that can only create poison. In particular, we can ignore poison-generating flags.

This means that inferring more flags has less chance to pessimize other transforms.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

Currently isGuaranteedNotToBeUndef() is the same as isGuaranteedNotToBeUndefOrPoison(). This function is used in places where we only care about undef (due to multi-use issues), not poison.

Make it more precise by only considering instructions that can create undef (like loads or call), and ignore those that can only create poison. In particular, we can ignore poison-generating flags.

This means that inferring more flags has less chance to pessimize other transforms.

---

Full diff: https://github.com/llvm/llvm-project/pull/76160.diff

2 Files Affected:

• (modified) llvm/lib/Analysis/ValueTracking.cpp (+51-30)
• (modified) llvm/test/Transforms/CorrelatedValuePropagation/cond-at-use.ll (+1-2)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp index 2ce660b9a858eb..276e9bde052268 100644 --- a/llvm/lib/Analysis/ValueTracking.cpp +++ b/llvm/lib/Analysis/ValueTracking.cpp @@ -6560,10 +6560,25 @@ static bool shiftAmountKnownInRange(const Value *ShiftAmount) { return Safe; } -static bool canCreateUndefOrPoison(const Operator *Op, bool PoisonOnly, +enum class UndefPoisonKind { + PoisonOnly = (1 << 0), + UndefOnly = (1 << 1), + UndefOrPoison = PoisonOnly | UndefOnly, +}; + +static bool includesPoison(UndefPoisonKind Kind) { + return (unsigned(Kind) & unsigned(UndefPoisonKind::PoisonOnly)) != 0; +} + +static bool includesUndef(UndefPoisonKind Kind) { + return (unsigned(Kind) & unsigned(UndefPoisonKind::UndefOnly)) != 0; +} + +static bool canCreateUndefOrPoison(const Operator *Op, UndefPoisonKind Kind, bool ConsiderFlagsAndMetadata) { - if (ConsiderFlagsAndMetadata && Op->hasPoisonGeneratingFlagsOrMetadata()) + if (ConsiderFlagsAndMetadata && includesPoison(Kind) && + Op->hasPoisonGeneratingFlagsOrMetadata()) return true; unsigned Opcode = Op->getOpcode(); @@ -6573,7 +6588,7 @@ static bool canCreateUndefOrPoison(const Operator *Op, bool PoisonOnly, case Instruction::Shl: case Instruction::AShr: case Instruction::LShr: - return !shiftAmountKnownInRange(Op->getOperand(1)); + return includesPoison(Kind) && !shiftAmountKnownInRange(Op->getOperand(1)); case Instruction::FPToSI: case Instruction::FPToUI: // fptosi/ui yields poison if the resulting value does not fit in the @@ -6614,7 +6629,8 @@ static bool canCreateUndefOrPoison(const Operator *Op, bool PoisonOnly, return false; case Intrinsic::sshl_sat: case Intrinsic::ushl_sat: - return !shiftAmountKnownInRange(II->getArgOperand(1)); + return includesPoison(Kind) && + !shiftAmountKnownInRange(II->getArgOperand(1)); case Intrinsic::fma: case Intrinsic::fmuladd: case Intrinsic::sqrt: @@ -6669,15 +6685,16 @@ static bool canCreateUndefOrPoison(const Operator *Op, bool PoisonOnly, auto *VTy = cast<VectorType>(Op->getOperand(0)->getType()); unsigned IdxOp = Op->getOpcode() == Instruction::InsertElement ? 2 : 1; auto *Idx = dyn_cast<ConstantInt>(Op->getOperand(IdxOp)); - if (!Idx || Idx->getValue().uge(VTy->getElementCount().getKnownMinValue())) - return true; + if (includesPoison(Kind)) + return !Idx || + Idx->getValue().uge(VTy->getElementCount().getKnownMinValue()); return false; } case Instruction::ShuffleVector: { ArrayRef<int> Mask = isa<ConstantExpr>(Op) ? cast<ConstantExpr>(Op)->getShuffleMask() : cast<ShuffleVectorInst>(Op)->getShuffleMask(); - return is_contained(Mask, PoisonMaskElem); + return includesPoison(Kind) && is_contained(Mask, PoisonMaskElem); } case Instruction::FNeg: case Instruction::PHI: @@ -6713,17 +6730,17 @@ static bool canCreateUndefOrPoison(const Operator *Op, bool PoisonOnly, bool llvm::canCreateUndefOrPoison(const Operator *Op, bool ConsiderFlagsAndMetadata) { - return ::canCreateUndefOrPoison(Op, /*PoisonOnly=*/false, + return ::canCreateUndefOrPoison(Op, UndefPoisonKind::UndefOrPoison, ConsiderFlagsAndMetadata); } bool llvm::canCreatePoison(const Operator *Op, bool ConsiderFlagsAndMetadata) { - return ::canCreateUndefOrPoison(Op, /*PoisonOnly=*/true, + return ::canCreateUndefOrPoison(Op, UndefPoisonKind::PoisonOnly, ConsiderFlagsAndMetadata); } -static bool directlyImpliesPoison(const Value *ValAssumedPoison, - const Value *V, unsigned Depth) { +static bool directlyImpliesPoison(const Value *ValAssumedPoison, const Value *V, + unsigned Depth) { if (ValAssumedPoison == V) return true; @@ -6775,14 +6792,11 @@ bool llvm::impliesPoison(const Value *ValAssumedPoison, const Value *V) { return ::impliesPoison(ValAssumedPoison, V, /* Depth */ 0); } -static bool programUndefinedIfUndefOrPoison(const Value *V, - bool PoisonOnly); +static bool programUndefinedIfUndefOrPoison(const Value *V, bool PoisonOnly); -static bool isGuaranteedNotToBeUndefOrPoison(const Value *V, - AssumptionCache *AC, - const Instruction *CtxI, - const DominatorTree *DT, - unsigned Depth, bool PoisonOnly) { +static bool isGuaranteedNotToBeUndefOrPoison( + const Value *V, AssumptionCache *AC, const Instruction *CtxI, + const DominatorTree *DT, unsigned Depth, UndefPoisonKind Kind) { if (Depth >= MaxAnalysisRecursionDepth) return false; @@ -6797,16 +6811,19 @@ static bool isGuaranteedNotToBeUndefOrPoison(const Value *V, } if (auto *C = dyn_cast<Constant>(V)) { + if (isa<PoisonValue>(C)) + return !includesPoison(Kind); + if (isa<UndefValue>(C)) - return PoisonOnly && !isa<PoisonValue>(C); + return !includesUndef(Kind); if (isa<ConstantInt>(C) || isa<GlobalVariable>(C) || isa<ConstantFP>(V) || isa<ConstantPointerNull>(C) || isa<Function>(C)) return true; if (C->getType()->isVectorTy() && !isa<ConstantExpr>(C)) - return (PoisonOnly ? !C->containsPoisonElement() - : !C->containsUndefOrPoisonElement()) && + return (!includesUndef(Kind) ? !C->containsPoisonElement() + : !C->containsUndefOrPoisonElement()) && !C->containsConstantExpression(); } @@ -6825,7 +6842,7 @@ static bool isGuaranteedNotToBeUndefOrPoison(const Value *V, auto OpCheck = [&](const Value *V) { return isGuaranteedNotToBeUndefOrPoison(V, AC, CtxI, DT, Depth + 1, - PoisonOnly); + Kind); }; if (auto *Opr = dyn_cast<Operator>(V)) { @@ -6847,14 +6864,16 @@ static bool isGuaranteedNotToBeUndefOrPoison(const Value *V, for (unsigned i = 0; i < Num; ++i) { auto *TI = PN->getIncomingBlock(i)->getTerminator(); if (!isGuaranteedNotToBeUndefOrPoison(PN->getIncomingValue(i), AC, TI, - DT, Depth + 1, PoisonOnly)) { + DT, Depth + 1, Kind)) { IsWellDefined = false; break; } } if (IsWellDefined) return true; - } else if (!canCreateUndefOrPoison(Opr) && all_of(Opr->operands(), OpCheck)) + } else if (!::canCreateUndefOrPoison(Opr, Kind, + /*ConsiderFlagsAndMetadata*/ true) && + all_of(Opr->operands(), OpCheck)) return true; } @@ -6864,7 +6883,7 @@ static bool isGuaranteedNotToBeUndefOrPoison(const Value *V, I->hasMetadata(LLVMContext::MD_dereferenceable_or_null)) return true; - if (programUndefinedIfUndefOrPoison(V, PoisonOnly)) + if (programUndefinedIfUndefOrPoison(V, !includesUndef(Kind))) return true; // CxtI may be null or a cloned instruction. @@ -6896,7 +6915,7 @@ static bool isGuaranteedNotToBeUndefOrPoison(const Value *V, if (Cond) { if (Cond == V) return true; - else if (PoisonOnly && isa<Operator>(Cond)) { + else if (!includesUndef(Kind) && isa<Operator>(Cond)) { // For poison, we can analyze further auto *Opr = cast<Operator>(Cond); if (any_of(Opr->operands(), @@ -6918,20 +6937,22 @@ bool llvm::isGuaranteedNotToBeUndefOrPoison(const Value *V, AssumptionCache *AC, const Instruction *CtxI, const DominatorTree *DT, unsigned Depth) { - return ::isGuaranteedNotToBeUndefOrPoison(V, AC, CtxI, DT, Depth, false); + return ::isGuaranteedNotToBeUndefOrPoison(V, AC, CtxI, DT, Depth, + UndefPoisonKind::UndefOrPoison); } bool llvm::isGuaranteedNotToBePoison(const Value *V, AssumptionCache *AC, const Instruction *CtxI, const DominatorTree *DT, unsigned Depth) { - return ::isGuaranteedNotToBeUndefOrPoison(V, AC, CtxI, DT, Depth, true); + return ::isGuaranteedNotToBeUndefOrPoison(V, AC, CtxI, DT, Depth, + UndefPoisonKind::PoisonOnly); } bool llvm::isGuaranteedNotToBeUndef(const Value *V, AssumptionCache *AC, const Instruction *CtxI, const DominatorTree *DT, unsigned Depth) { - // TODO: This is currently equivalent to isGuaranteedNotToBeUndefOrPoison(). - return ::isGuaranteedNotToBeUndefOrPoison(V, AC, CtxI, DT, Depth, false); + return ::isGuaranteedNotToBeUndefOrPoison(V, AC, CtxI, DT, Depth, + UndefPoisonKind::UndefOnly); } /// Return true if undefined behavior would provably be executed on the path to diff --git a/llvm/test/Transforms/CorrelatedValuePropagation/cond-at-use.ll b/llvm/test/Transforms/CorrelatedValuePropagation/cond-at-use.ll index 546baf086cdbb0..0b95139f3dcbaa 100644 --- a/llvm/test/Transforms/CorrelatedValuePropagation/cond-at-use.ll +++ b/llvm/test/Transforms/CorrelatedValuePropagation/cond-at-use.ll @@ -571,9 +571,8 @@ define i16 @cond_value_may_not_well_defined(i16 %x) { define i16 @and_elide_poison_flags(i16 noundef %a) { ; CHECK-LABEL: @and_elide_poison_flags( ; CHECK-NEXT: [[X:%.*]] = add nuw i16 [[A:%.*]], 1 -; CHECK-NEXT: [[AND:%.*]] = and i16 [[X]], 7 ; CHECK-NEXT: [[CMP:%.*]] = icmp ult i16 [[X]], 8 -; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i16 [[AND]], i16 24 +; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i16 [[X]], i16 24 ; CHECK-NEXT: ret i16 [[SEL]] ; %x = add nuw i16 %a, 1 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[nunoplopes]

LGTM, thank you!