Skip to content

[ConstraintElimination] Add additional facts for bitwise AND OR #132124

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

Open
leewei05 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[ConstraintElimination] Add additional facts for bitwise AND OR #132124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
66839d8
Add bitwise and or tests
leewei05 Mar 6, 2025
e4a8b21
Add additional facts
leewei05 Mar 19, 2025
3eadf70
Update tests
leewei05 Mar 24, 2025
40fc765
Add negative test cases
leewei05 Mar 28, 2025
3368e4a
Add mixed bitwise and or test cases
leewei05 Mar 28, 2025
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
24 changes: 24 additions & 0 deletions llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1605,6 +1605,30 @@ void ConstraintInfo::addFact(CmpInst::Predicate Pred, Value *A, Value *B,
unsigned NumIn, unsigned NumOut,
SmallVectorImpl<StackEntry> &DFSInStack) {
addFactImpl(Pred, A, B, NumIn, NumOut, DFSInStack, false);

Value *LHS;
Value *RHS;
if (match(A, m_Or(m_Value(LHS), m_Value(RHS)))) {
// (LHS | RHS >= 0) => LHS >= 0 && RHS >= 0
Copy link
Member

@dtcxzyw dtcxzyw Mar 20, 2025

Choose a reason for hiding this comment

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

X <=/>= 0 are not canonical forms. Do we need to handle them?

Copy link
Contributor Author

@leewei05 leewei05 Mar 20, 2025

Choose a reason for hiding this comment

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

@dtcxzyw I wanted to handle as many patterns as possible. Also, constraint elimination adds certain facts with ICMP_SGE and I want to make sure that those facts can be handled as well. Even if X <=/>= 0 are not canonical forms, can these patterns exist after running InstCombine?

Btw, is there a documentation for these canonical forms? Or these canonical forms can only be found in InstCombine code.

Copy link
Contributor

@regehr regehr Mar 20, 2025

Choose a reason for hiding this comment

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

it looks like instcombine reliably canonicalizes these, so I don't think you need to match the non-canonical forms

Copy link
Contributor Author

@leewei05 leewei05 Mar 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@dtcxzyw @regehr

Yes, we need to handle <=/>= despite them not being canonical, as ConstraintElimination can still use them for optimizations. The pass doesn’t limit facts to canonical forms—e.g., in the example below, the end block adds a fact from an inverted predicate (%1 <= -1). This applies to both AND and OR cases.

define void @test_decompose_bitwise_and(i4 %x, i4 %y) {
entry:
  %1 = and i4 %y, %x
  ; canonicalize form after InstCombine:
  ; %and = icmp sgt i4 %1, -1
  %and = icmp sge i4 %1, 0
  br i1 %and, label %then, label %end

then:
  ; fact: %and > -1
  ret void

end:
  ; fact: %and <= -1
  %f.1 = icmp slt i4 %x, 0
  %f.2 = icmp slt i4 %y, 0
  call void @use(i1 %f.1)
  call void @use(i1 %f.2)
  ret void
}

AND case: https://llvm.godbolt.org/z/7h86PqacE
AND Alive2: https://alive2.llvm.org/ce/z/9FY5Tt

OR case: https://llvm.godbolt.org/z/fdzrbTn4M
OR Alive2: https://alive2.llvm.org/ce/z/Cog6xS

I hope it makes sense. I'll update the test cases accordingly as well.

// (LHS | RHS > -1) => LHS >= 0 && RHS >= 0
if ((match(B, m_Zero()) && Pred == CmpInst::ICMP_SGE) ||
(match(B, m_AllOnes()) && Pred == CmpInst::ICMP_SGT)) {
addFact(CmpInst::ICMP_SGE, LHS, ConstantInt::get(LHS->getType(), 0),
NumIn, NumOut, DFSInStack);
addFact(CmpInst::ICMP_SGE, RHS, ConstantInt::get(RHS->getType(), 0),
NumIn, NumOut, DFSInStack);
Copy link
Contributor

@fhahn fhahn Mar 20, 2025

Choose a reason for hiding this comment

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

is there benefit of continuing to add the constraint for A, B below or can we return early? Would be good to ensure sufficient test coverage

Copy link
Contributor Author

@leewei05 leewei05 Mar 25, 2025

Choose a reason for hiding this comment

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

Yes, keeping the original A, B constraint is beneficial. For OR, it keeps cases where LHS | RHS >= 0 holds, while the derived LHS >= 0 && RHS >= 0 strengthens it. For AND, it matches exactly and ties to the code. The recursive addFact also supports nested OR/AND, adding more facts. I’ll ensure test coverage for single-level, nested, and edge cases. Does this answer your question?

}
} else if (match(A, m_And(m_Value(LHS), m_Value(RHS)))) {
// (LHS & RHS < 0) => LHS < 0 && RHS < 0
// (LHS & RHS <= -1) => LHS < 0 && RHS < 0
if ((match(B, m_Zero()) && Pred == CmpInst::ICMP_SLT) ||
(match(B, m_AllOnes()) && Pred == CmpInst::ICMP_SLE)) {
addFact(CmpInst::ICMP_SLT, LHS, ConstantInt::get(LHS->getType(), 0),
NumIn, NumOut, DFSInStack);
addFact(CmpInst::ICMP_SLT, RHS, ConstantInt::get(RHS->getType(), 0),
NumIn, NumOut, DFSInStack);
}
}
// If the Pred is eq/ne, also add the fact to signed system.
if (CmpInst::isEquality(Pred))
addFactImpl(Pred, A, B, NumIn, NumOut, DFSInStack, true);
Expand Down
205 changes: 205 additions & 0 deletions llvm/test/Transforms/ConstraintElimination/and.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,3 +603,208 @@ exit:

ret i1 %r.10
}

Copy link
Member

@dtcxzyw dtcxzyw Mar 20, 2025

Choose a reason for hiding this comment

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

Can you add some negative tests?

define void @test_decompose_bitwise_and(i4 %x, i4 %y) {
; CHECK-LABEL: @test_decompose_bitwise_and(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = and i4 [[Y:%.*]], [[X:%.*]]
; CHECK-NEXT: [[AND:%.*]] = icmp slt i4 [[TMP0]], 0
; CHECK-NEXT: br i1 [[AND]], label [[BB1:%.*]], label [[EXIT:%.*]]
; CHECK: then:
; CHECK-NEXT: call void @use(i1 true)
; CHECK-NEXT: call void @use(i1 true)
; CHECK-NEXT: ret void
; CHECK: end:
; CHECK-NEXT: ret void
;
entry:
%and.1 = and i4 %y, %x
%c.1= icmp slt i4 %and.1, 0
br i1 %c.1, label %then, label %end

then:
; fact: %and.1 < 0
%t.1 = icmp slt i4 %x, 0
%t.2 = icmp slt i4 %y, 0
call void @use(i1 %t.1)
call void @use(i1 %t.2)
ret void

end:
ret void
}

define void @test_decompose_bitwise_and2(i4 %x, i4 %y) {
; CHECK-LABEL: @test_decompose_bitwise_and2(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = and i4 [[X:%.*]], [[Y:%.*]]
; CHECK-NEXT: [[AND_NOT:%.*]] = icmp sgt i4 [[TMP0]], -1
; CHECK-NEXT: br i1 [[AND_NOT]], label [[END:%.*]], label [[THEN:%.*]]
; CHECK: then:
; CHECK-NEXT: ret void
; CHECK: end:
; CHECK-NEXT: call void @use(i1 true)
; CHECK-NEXT: call void @use(i1 true)
; CHECK-NEXT: ret void
;
entry:
%and.1 = and i4 %x, %y
%c.1 = icmp sgt i4 %and.1, -1
br i1 %c.1, label %then, label %end

then:
; fact: %and.1 > -1
ret void

end:
; fact: %and.1 <= -1
%t.1 = icmp slt i4 %x, 0
%t.2 = icmp slt i4 %y, 0
call void @use(i1 %t.1)
call void @use(i1 %t.2)
ret void
}

define void @test_decompose_nested_bitwise_and(i4 %x, i4 %y, i4 %z, i4 %w) {
; CHECK-LABEL: @test_decompose_nested_bitwise_and(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = and i4 [[Y:%.*]], [[X:%.*]]
; CHECK-NEXT: [[TMP1:%.*]] = and i4 [[TMP0]], [[Z:%.*]]
; CHECK-NEXT: [[TMP2:%.*]] = and i4 [[TMP1]], [[W:%.*]]
; CHECK-NEXT: [[AND:%.*]] = icmp slt i4 [[TMP2]], 0
; CHECK-NEXT: br i1 [[AND]], label [[BB1:%.*]], label [[EXIT:%.*]]
; CHECK: then:
; CHECK-NEXT: call void @use(i1 true)
; CHECK-NEXT: call void @use(i1 true)
; CHECK-NEXT: call void @use(i1 true)
; CHECK-NEXT: call void @use(i1 true)
; CHECK-NEXT: ret void
; CHECK: end:
; CHECK-NEXT: ret void
;
entry:
%and.1 = and i4 %y, %x
%and.2 = and i4 %and.1, %z
%and.3 = and i4 %and.2, %w
%c.1= icmp slt i4 %and.3, 0
br i1 %c.1, label %then, label %end

then:
; fact: %and.3 < 0
%t.1 = icmp slt i4 %x, 0
%t.2 = icmp slt i4 %y, 0
%t.3 = icmp slt i4 %z, 0
%t.4 = icmp slt i4 %w, 0
call void @use(i1 %t.1)
call void @use(i1 %t.2)
call void @use(i1 %t.3)
call void @use(i1 %t.4)
ret void

end:
ret void
}

define void @test_decompose_nested_bitwise_and2(i4 %x, i4 %y, i4 %z) {
; CHECK-LABEL: @test_decompose_nested_bitwise_and2(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = and i4 [[X:%.*]], [[Y:%.*]]
; CHECK-NEXT: [[TMP1:%.*]] = and i4 [[TMP0]], [[Z:%.*]]
; CHECK-NEXT: [[AND_2_NOT:%.*]] = icmp sgt i4 [[TMP1]], -1
; CHECK-NEXT: br i1 [[AND_2_NOT]], label [[F:%.*]], label [[T:%.*]]
; CHECK: then:
; CHECK-NEXT: ret void
; CHECK: end:
; CHECK-NEXT: call void @use(i1 true)
; CHECK-NEXT: call void @use(i1 true)
; CHECK-NEXT: call void @use(i1 true)
; CHECK-NEXT: ret void
;
entry:
%and.1 = and i4 %x, %y
%and.2 = and i4 %and.1, %z
%c.1 = icmp sgt i4 %and.2, -1
br i1 %c.1, label %then, label %end

then:
; fact: %and.2 > -1
ret void

end:
; fact: %and.2 <= -1 same as %and.2 < 0
%t.1 = icmp slt i4 %x, 0
%t.2 = icmp slt i4 %y, 0
%t.3 = icmp slt i4 %z, 0
call void @use(i1 %t.1)
call void @use(i1 %t.2)
call void @use(i1 %t.3)
ret void
}

define void @test_decompose_bitwise_and_negative(i4 %x, i4 %y) {
; CHECK-LABEL: @test_decompose_bitwise_and_negative(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[AND_1:%.*]] = and i4 [[Y:%.*]], [[X:%.*]]
; CHECK-NEXT: [[C_1:%.*]] = icmp slt i4 [[AND_1]], 0
; CHECK-NEXT: br i1 [[C_1]], label [[THEN:%.*]], label [[END:%.*]]
; CHECK: then:
; CHECK-NEXT: ret void
; CHECK: end:
; CHECK-NEXT: [[C_2:%.*]] = icmp sgt i4 [[X]], 0
; CHECK-NEXT: [[C_3:%.*]] = icmp sgt i4 [[Y]], 0
; CHECK-NEXT: call void @use(i1 [[C_2]])
; CHECK-NEXT: call void @use(i1 [[C_3]])
; CHECK-NEXT: ret void
;
entry:
%and.1 = and i4 %y, %x
%c.1= icmp slt i4 %and.1, 0
br i1 %c.1, label %then, label %end

then:
; fact: %and.1 < 0
ret void

end:
; fact: %and.1 >= 0
; %c.2, %c.3 should only be replaced in the bitwise OR case
%c.2 = icmp sgt i4 %x, 0
%c.3 = icmp sgt i4 %y, 0
call void @use(i1 %c.2)
call void @use(i1 %c.3)
ret void
}

define void @test_decompose_bitwise_and_negative_2(i4 %x, i4 %y) {
; CHECK-LABEL: @test_decompose_bitwise_and_negative_2(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[AND_1:%.*]] = and i4 [[X:%.*]], [[Y:%.*]]
; CHECK-NEXT: [[C_1:%.*]] = icmp sgt i4 [[AND_1]], -1
; CHECK-NEXT: br i1 [[C_1]], label [[THEN:%.*]], label [[END:%.*]]
; CHECK: then:
; CHECK-NEXT: [[C_2:%.*]] = icmp sgt i4 [[X]], 0
; CHECK-NEXT: [[C_3:%.*]] = icmp sgt i4 [[Y]], 0
; CHECK-NEXT: call void @use(i1 [[C_2]])
; CHECK-NEXT: call void @use(i1 [[C_3]])
; CHECK-NEXT: ret void
; CHECK: end:
; CHECK-NEXT: ret void
;
entry:
%and.1 = and i4 %x, %y
%c.1 = icmp sgt i4 %and.1, -1
br i1 %c.1, label %then, label %end

then:
; fact: %and.1 > -1
; %c.1, %c.2 should only be replaced in the bitwise OR case
%c.2 = icmp sgt i4 %x, 0
%c.3 = icmp sgt i4 %y, 0
call void @use(i1 %c.2)
call void @use(i1 %c.3)
ret void

end:
; fact: %and.1 <= -1
ret void
}
Loading