From: Nikita Popov Date: Mon, 2 Jan 2023 13:37:02 +0000 (+0100) Subject: [ValueTracking] Use SmallVector for non-undef/poison ops X-Git-Tag: upstream/17.0.6~22383 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=3f04553e5c16f5ba57e9a82edc190770f5a062bb;p=platform%2Fupstream%2Fllvm.git [ValueTracking] Use SmallVector for non-undef/poison ops The way these APIs are used, there isn't really a benefit to deduplicating the ops as part of the API. The only place that benefits from this is PoisonChecking, and for that particular use the assertion emission was potentially non-deterministic. We should populate a vector for deterministic order and then deduplicate via a separate set. --- diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h index 2419b89..90caf42 100644 --- a/llvm/include/llvm/Analysis/ValueTracking.h +++ b/llvm/include/llvm/Analysis/ValueTracking.h @@ -617,13 +617,13 @@ bool propagatesPoison(const Use &PoisonOp); /// Insert operands of I into Ops such that I will trigger undefined behavior /// if I is executed and that operand has a poison value. void getGuaranteedNonPoisonOps(const Instruction *I, - SmallPtrSetImpl &Ops); + SmallVectorImpl &Ops); /// Insert operands of I into Ops such that I will trigger undefined behavior /// if I is executed and that operand is not a well-defined value /// (i.e. has undef bits or poison). void getGuaranteedWellDefinedOps(const Instruction *I, - SmallPtrSetImpl &Ops); + SmallVectorImpl &Ops); /// Return true if the given instruction must trigger undefined behavior /// when I is executed with any operands which appear in KnownPoison holding diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp index 03cbb6c..b563154 100644 --- a/llvm/lib/Analysis/ValueTracking.cpp +++ b/llvm/lib/Analysis/ValueTracking.cpp @@ -5710,49 +5710,49 @@ bool llvm::propagatesPoison(const Use &PoisonOp) { } void llvm::getGuaranteedWellDefinedOps( - const Instruction *I, SmallPtrSetImpl &Operands) { + const Instruction *I, SmallVectorImpl &Operands) { switch (I->getOpcode()) { case Instruction::Store: - Operands.insert(cast(I)->getPointerOperand()); + Operands.push_back(cast(I)->getPointerOperand()); break; case Instruction::Load: - Operands.insert(cast(I)->getPointerOperand()); + Operands.push_back(cast(I)->getPointerOperand()); break; // Since dereferenceable attribute imply noundef, atomic operations // also implicitly have noundef pointers too case Instruction::AtomicCmpXchg: - Operands.insert(cast(I)->getPointerOperand()); + Operands.push_back(cast(I)->getPointerOperand()); break; case Instruction::AtomicRMW: - Operands.insert(cast(I)->getPointerOperand()); + Operands.push_back(cast(I)->getPointerOperand()); break; case Instruction::Call: case Instruction::Invoke: { const CallBase *CB = cast(I); if (CB->isIndirectCall()) - Operands.insert(CB->getCalledOperand()); + Operands.push_back(CB->getCalledOperand()); for (unsigned i = 0; i < CB->arg_size(); ++i) { if (CB->paramHasAttr(i, Attribute::NoUndef) || CB->paramHasAttr(i, Attribute::Dereferenceable)) - Operands.insert(CB->getArgOperand(i)); + Operands.push_back(CB->getArgOperand(i)); } break; } case Instruction::Ret: if (I->getFunction()->hasRetAttribute(Attribute::NoUndef)) - Operands.insert(I->getOperand(0)); + Operands.push_back(I->getOperand(0)); break; case Instruction::Switch: - Operands.insert(cast(I)->getCondition()); + Operands.push_back(cast(I)->getCondition()); break; case Instruction::Br: { auto *BR = cast(I); if (BR->isConditional()) - Operands.insert(BR->getCondition()); + Operands.push_back(BR->getCondition()); break; } default: @@ -5761,7 +5761,7 @@ void llvm::getGuaranteedWellDefinedOps( } void llvm::getGuaranteedNonPoisonOps(const Instruction *I, - SmallPtrSetImpl &Operands) { + SmallVectorImpl &Operands) { getGuaranteedWellDefinedOps(I, Operands); switch (I->getOpcode()) { // Divisors of these operations are allowed to be partially undef. @@ -5769,7 +5769,7 @@ void llvm::getGuaranteedNonPoisonOps(const Instruction *I, case Instruction::SDiv: case Instruction::URem: case Instruction::SRem: - Operands.insert(I->getOperand(1)); + Operands.push_back(I->getOperand(1)); break; default: break; @@ -5778,7 +5778,7 @@ void llvm::getGuaranteedNonPoisonOps(const Instruction *I, bool llvm::mustTriggerUB(const Instruction *I, const SmallSet& KnownPoison) { - SmallPtrSet NonPoisonOps; + SmallVector NonPoisonOps; getGuaranteedNonPoisonOps(I, NonPoisonOps); for (const auto *V : NonPoisonOps) @@ -5826,9 +5826,9 @@ static bool programUndefinedIfUndefOrPoison(const Value *V, if (--ScanLimit == 0) break; - SmallPtrSet WellDefinedOps; + SmallVector WellDefinedOps; getGuaranteedWellDefinedOps(&I, WellDefinedOps); - if (WellDefinedOps.contains(V)) + if (is_contained(WellDefinedOps, V)) return true; if (!isGuaranteedToTransferExecutionToSuccessor(&I)) diff --git a/llvm/lib/Transforms/Instrumentation/PoisonChecking.cpp b/llvm/lib/Transforms/Instrumentation/PoisonChecking.cpp index 54dd20f..42e7cd8 100644 --- a/llvm/lib/Transforms/Instrumentation/PoisonChecking.cpp +++ b/llvm/lib/Transforms/Instrumentation/PoisonChecking.cpp @@ -276,10 +276,13 @@ static bool rewrite(Function &F) { // Note: There are many more sources of documented UB, but this pass only // attempts to find UB triggered by propagation of poison. - SmallPtrSet NonPoisonOps; + SmallVector NonPoisonOps; + SmallPtrSet SeenNonPoisonOps; getGuaranteedNonPoisonOps(&I, NonPoisonOps); for (const Value *Op : NonPoisonOps) - CreateAssertNot(B, getPoisonFor(ValToPoison, const_cast(Op))); + if (SeenNonPoisonOps.insert(Op).second) + CreateAssertNot(B, + getPoisonFor(ValToPoison, const_cast(Op))); if (LocalCheck) if (auto *RI = dyn_cast(&I))