Skip to content

Commit

Permalink
Revert "RFC: [ValueTracking] Let analyses assume a value cannot be pa…
Browse files Browse the repository at this point in the history
…rtially poison"

This reverts commit 80faa8c.
  • Loading branch information
aqjune committed Apr 22, 2020
1 parent 80faa8c commit 5ceef26
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 24 deletions.
24 changes: 16 additions & 8 deletions llvm/include/llvm/Analysis/ValueTracking.h
Expand Up @@ -563,26 +563,34 @@ class Value;
const Loop *L);

/// Return true if this function can prove that I is guaranteed to yield
/// poison if at least one of its operands is poison.
bool propagatesPoison(const Instruction *I);
/// full-poison (all bits poison) if at least one of its operands are
/// full-poison (all bits poison).
///
/// The exact rules for how poison propagates through instructions have
/// not been settled as of 2015-07-10, so this function is conservative
/// and only considers poison to be propagated in uncontroversial
/// cases. There is no attempt to track values that may be only partially
/// poison.
bool propagatesFullPoison(const Instruction *I);

/// Return either nullptr or an operand of I such that I will trigger
/// undefined behavior if I is executed and that operand has a poison
/// value.
const Value *getGuaranteedNonPoisonOp(const Instruction *I);
/// undefined behavior if I is executed and that operand has a full-poison
/// value (all bits poison).
const Value *getGuaranteedNonFullPoisonOp(const Instruction *I);

/// Return true if the given instruction must trigger undefined behavior.
/// when I is executed with any operands which appear in KnownPoison holding
/// a poison value at the point of execution.
/// a full-poison value at the point of execution.
bool mustTriggerUB(const Instruction *I,
const SmallSet<const Value *, 16>& KnownPoison);

/// Return true if this function can prove that if PoisonI is executed
/// and yields a poison value, then that will trigger undefined behavior.
/// and yields a full-poison value (all bits poison), then that will
/// trigger undefined behavior.
///
/// Note that this currently only considers the basic block that is
/// the parent of I.
bool programUndefinedIfPoison(const Instruction *PoisonI);
bool programUndefinedIfFullPoison(const Instruction *PoisonI);

/// Return true if I can create poison from non-poison operands.
/// For vectors, canCreatePoison returns true if there is potential poison in
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/Analysis/ScalarEvolution.cpp
Expand Up @@ -6072,7 +6072,7 @@ bool ScalarEvolution::isSCEVExprNeverPoison(const Instruction *I) {
return false;

// Only proceed if we can prove that I does not yield poison.
if (!programUndefinedIfPoison(I))
if (!programUndefinedIfFullPoison(I))
return false;

// At this point we know that if I is executed, then it does not wrap
Expand Down Expand Up @@ -6152,7 +6152,7 @@ bool ScalarEvolution::isAddRecNeverPoison(const Instruction *I, const Loop *L) {
SmallVector<const Instruction *, 8> PoisonStack;

// We start by assuming \c I, the post-inc add recurrence, is poison. Only
// things that are known to be poison under that assumption go on the
// things that are known to be fully poison under that assumption go on the
// PoisonStack.
Pushed.insert(I);
PoisonStack.push_back(I);
Expand All @@ -6162,7 +6162,7 @@ bool ScalarEvolution::isAddRecNeverPoison(const Instruction *I, const Loop *L) {
const Instruction *Poison = PoisonStack.pop_back_val();

for (auto *PoisonUser : Poison->users()) {
if (propagatesPoison(cast<Instruction>(PoisonUser))) {
if (propagatesFullPoison(cast<Instruction>(PoisonUser))) {
if (Pushed.insert(cast<Instruction>(PoisonUser)).second)
PoisonStack.push_back(cast<Instruction>(PoisonUser));
} else if (auto *BI = dyn_cast<BranchInst>(PoisonUser)) {
Expand Down
12 changes: 6 additions & 6 deletions llvm/lib/Analysis/ValueTracking.cpp
Expand Up @@ -4720,7 +4720,7 @@ bool llvm::isGuaranteedNotToBeUndefOrPoison(const Value *V,
}

if (auto I = dyn_cast<Instruction>(V)) {
if (programUndefinedIfPoison(I) && I->getType()->isIntegerTy(1))
if (programUndefinedIfFullPoison(I) && I->getType()->isIntegerTy(1))
// Note: once we have an agreement that poison is a value-wise concept,
// we can remove the isIntegerTy(1) constraint.
return true;
Expand Down Expand Up @@ -4846,7 +4846,7 @@ bool llvm::isGuaranteedToExecuteForEveryIteration(const Instruction *I,
llvm_unreachable("Instruction not contained in its own parent basic block.");
}

bool llvm::propagatesPoison(const Instruction *I) {
bool llvm::propagatesFullPoison(const Instruction *I) {
// TODO: This should include all instructions apart from phis, selects and
// call-like instructions.
switch (I->getOpcode()) {
Expand Down Expand Up @@ -4880,7 +4880,7 @@ bool llvm::propagatesPoison(const Instruction *I) {
}
}

const Value *llvm::getGuaranteedNonPoisonOp(const Instruction *I) {
const Value *llvm::getGuaranteedNonFullPoisonOp(const Instruction *I) {
switch (I->getOpcode()) {
case Instruction::Store:
return cast<StoreInst>(I)->getPointerOperand();
Expand Down Expand Up @@ -4918,12 +4918,12 @@ const Value *llvm::getGuaranteedNonPoisonOp(const Instruction *I) {

bool llvm::mustTriggerUB(const Instruction *I,
const SmallSet<const Value *, 16>& KnownPoison) {
auto *NotPoison = getGuaranteedNonPoisonOp(I);
auto *NotPoison = getGuaranteedNonFullPoisonOp(I);
return (NotPoison && KnownPoison.count(NotPoison));
}


bool llvm::programUndefinedIfPoison(const Instruction *PoisonI) {
bool llvm::programUndefinedIfFullPoison(const Instruction *PoisonI) {
// We currently only look for uses of poison values within the same basic
// block, as that makes it easier to guarantee that the uses will be
// executed given that PoisonI is executed.
Expand Down Expand Up @@ -4956,7 +4956,7 @@ bool llvm::programUndefinedIfPoison(const Instruction *PoisonI) {
if (YieldsPoison.count(&I)) {
for (const User *User : I.users()) {
const Instruction *UserI = cast<Instruction>(User);
if (propagatesPoison(UserI))
if (propagatesFullPoison(UserI))
YieldsPoison.insert(User);
}
}
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/Transforms/Instrumentation/PoisonChecking.cpp
Expand Up @@ -283,7 +283,7 @@ 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.
if (Value *Op = const_cast<Value*>(getGuaranteedNonPoisonOp(&I)))
if (Value *Op = const_cast<Value*>(getGuaranteedNonFullPoisonOp(&I)))
CreateAssertNot(B, getPoisonFor(ValToPoison, Op));

if (LocalCheck)
Expand All @@ -294,7 +294,7 @@ static bool rewrite(Function &F) {
}

SmallVector<Value*, 4> Checks;
if (propagatesPoison(&I))
if (propagatesFullPoison(&I))
for (Value *V : I.operands())
Checks.push_back(getPoisonFor(ValToPoison, V));

Expand Down Expand Up @@ -344,7 +344,7 @@ PreservedAnalyses PoisonCheckingPass::run(Function &F,
- shufflevector - It would seem reasonable for an out of bounds mask element
to produce poison, but the LangRef does not state.
- and/or - It would seem reasonable for poison to propagate from both
arguments, but LangRef doesn't state and propagatesPoison doesn't
arguments, but LangRef doesn't state and propagatesFullPoison doesn't
include these two.
- all binary ops w/vector operands - The likely interpretation would be that
any element overflowing should produce poison for the entire result, but
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
Expand Up @@ -1813,7 +1813,7 @@ static bool mustExecuteUBIfPoisonOnPathTo(Instruction *Root,

// If we can't analyze propagation through this instruction, just skip it
// and transitive users. Safe as false is a conservative result.
if (!propagatesPoison(I) && I != Root)
if (!propagatesFullPoison(I) && I != Root)
continue;

if (KnownPoison.insert(I).second)
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
Expand Up @@ -1214,13 +1214,13 @@ bool SeparateConstOffsetFromGEP::reuniteExts(Instruction *I) {

// Add I to DominatingExprs if it's an add/sub that can't sign overflow.
if (match(I, m_NSWAdd(m_Value(LHS), m_Value(RHS)))) {
if (programUndefinedIfPoison(I)) {
if (programUndefinedIfFullPoison(I)) {
const SCEV *Key =
SE->getAddExpr(SE->getUnknown(LHS), SE->getUnknown(RHS));
DominatingAdds[Key].push_back(I);
}
} else if (match(I, m_NSWSub(m_Value(LHS), m_Value(RHS)))) {
if (programUndefinedIfPoison(I)) {
if (programUndefinedIfFullPoison(I)) {
const SCEV *Key =
SE->getAddExpr(SE->getUnknown(LHS), SE->getUnknown(RHS));
DominatingSubs[Key].push_back(I);
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Analysis/ScalarEvolution/flags-from-poison.ll
Expand Up @@ -205,7 +205,7 @@ exit:
ret void
}

; Demonstrate why we need a Visited set in llvm::programUndefinedIfPoison.
; Demonstrate why we need a Visited set in llvm::programUndefinedIfFullPoison.
define void @test-add-not-header5(float* %input, i32 %offset) {
; CHECK-LABEL: @test-add-not-header5
entry:
Expand Down

0 comments on commit 5ceef26

Please sign in to comment.