Skip to content

[ValueTracking] Handle not cond to assume. #127140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[ValueTracking] Handle not cond to assume. #127140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 22 additions & 16 deletions llvm/lib/Analysis/ValueTracking.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,16 +441,28 @@ void llvm::computeKnownBitsFromRangeMetadata(const MDNode &Ranges,
}

static bool isEphemeralValueOf(const Instruction *I, const Value *E) {
SmallVector<const Value *, 16> WorkSet(1, I);
SmallPtrSet<const Value *, 32> Visited;
SmallPtrSet<const Value *, 16> EphValues;

// The instruction defining an assumption's condition itself is always
// considered ephemeral to that assumption (even if it has other
// non-ephemeral users). See r246696's test case for an example.
if (is_contained(I->operands(), E))
return true;

SmallPtrSet<const Value *, 32> Visited{I};
SmallPtrSet<const Value *, 16> EphValues{I};

Value *X;
if (match(I, m_Intrinsic<Intrinsic::assume>(m_Not(m_Value(X))))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am ok with the IR diff. But I am a bit confused by this line. Do we really need to specially handle assume(not(cond))?

That is, we should also match trunc here if we want to remove !IsAssume && from the following code:

llvm-project/llvm/lib/Analysis/ValueTracking.cpp

Line 10346 in 70b95ca

} else if (!IsAssume && match(V, m_Trunc(m_Value(X)))) {

Copy link
Contributor Author

@andjo403 andjo403 Feb 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the !IsAssume && for the trunc check is only to avoid to add the same value twice to the AssumptionCache as it is added by the

llvm-project/llvm/lib/Analysis/ValueTracking.cpp

Lines 10229 to 10232 in 70b95ca

if (match(I, m_CombineOr(m_PtrToInt(m_Value(Op)), m_Trunc(m_Value(Op))))) {
if (isa<Instruction>(Op) || isa<Argument>(Op))
InsertAffected(Op);
}

via

llvm-project/llvm/lib/Analysis/ValueTracking.cpp

Lines 10261 to 10262 in 70b95ca

if (IsAssume) {
AddAffected(V);

This was the only way that I was able to think of how to solve supporting not in assumes.

if (X == E)
return true;
auto *Not = cast<Instruction>(I->getOperand(0));
Visited.insert(Not);
EphValues.insert(Not);
I = Not;
}

SmallVector<const Value *, 16> WorkSet(I->operands());

while (!WorkSet.empty()) {
const Value *V = WorkSet.pop_back_val();
if (!Visited.insert(V).second)
Expand All @@ -463,13 +475,11 @@ static bool isEphemeralValueOf(const Instruction *I, const Value *E) {
if (V == E)
return true;

if (V == I || (isa<Instruction>(V) &&
!cast<Instruction>(V)->mayHaveSideEffects() &&
!cast<Instruction>(V)->isTerminator())) {
EphValues.insert(V);
if (const User *U = dyn_cast<User>(V))
append_range(WorkSet, U->operands());
}
if (const auto *II = dyn_cast<Instruction>(V))
if (!II->mayHaveSideEffects() && !II->isTerminator()) {
EphValues.insert(V);
append_range(WorkSet, II->operands());
}
}
}

Expand Down Expand Up @@ -10258,11 +10268,8 @@ void llvm::findValuesAffectedByCondition(
CmpPredicate Pred;
Value *A, *B, *X;

if (IsAssume) {
if (IsAssume)
AddAffected(V);
if (match(V, m_Not(m_Value(X))))
AddAffected(X);
}

if (match(V, m_LogicalOp(m_Value(A), m_Value(B)))) {
// assume(A && B) is split to -> assume(A); assume(B);
Expand Down Expand Up @@ -10347,8 +10354,7 @@ void llvm::findValuesAffectedByCondition(
// Assume is checked here as X is already added above for assumes in
// addValueAffectedByCondition
AddAffected(X);
} else if (!IsAssume && match(V, m_Not(m_Value(X)))) {
// Assume is checked here to avoid issues with ephemeral values
} else if (match(V, m_Not(m_Value(X)))) {
Worklist.push_back(X);
}
}
Expand Down
3 changes: 1 addition & 2 deletions llvm/test/Transforms/InstCombine/assume.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -983,8 +983,7 @@ define i1 @not_cond_use(i8 %x) {
; CHECK-NEXT: tail call void @use(i1 [[CMP]])
; CHECK-NEXT: [[NOT:%.*]] = xor i1 [[CMP]], true
; CHECK-NEXT: tail call void @llvm.assume(i1 [[NOT]])
; CHECK-NEXT: [[RVAL:%.*]] = icmp eq i8 [[X]], 0
; CHECK-NEXT: ret i1 [[RVAL]]
; CHECK-NEXT: ret i1 false
;
%cmp = icmp eq i8 %x, 0
tail call void @use(i1 %cmp)
Expand Down