Skip to content
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.

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

[InstCombine] Re-enable DomCondCache in foldICmpUsingKnownBits #112742

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Oct 17, 2024

TODO: Fix IndVarSimplify regressions

@dtcxzyw dtcxzyw changed the title [InstCombine] Reenable DomCondCache in foldICmpUsingKnownBits [InstCombine] Re-enable DomCondCache in foldICmpUsingKnownBits Oct 17, 2024
dtcxzyw added a commit that referenced this pull request Oct 18, 2024
This following pattern is common in loop headers:
```
  %101 = sub nuw i64 %78, %98
  %103 = icmp eq i64 %78, %98
  br i1 %103, label %.thread.i.i, label %.preheader.preheader.i.i

.preheader.preheader.i.i:
  %invariant.umin.i.i = call i64 @llvm.umin.i64(i64 %101, i64 9)
  %umax.i = call i64 @llvm.umax.i64(i64 %invariant.umin.i.i, i64 1)
  br label %.preheader.i.i

.preheader.i.i:
  ...
  %116 = add nuw nsw i64 %.011.i.i, 1
  %exitcond.not.i = icmp eq i64 %116, %umax.i
  br i1 %exitcond.not.i, label %.critedge.i.i, label %.preheader.i.i
```
As `%78` is not equal to `%98` in BB `.preheader.preheader.i.i`, we can
prove `%101` is non-zero. Then we can simplify the loop exit condition.

Addresses regression introduced by
#112742.
EricWF pushed a commit to efcs/llvm-project that referenced this pull request Oct 22, 2024
This following pattern is common in loop headers:
```
  %101 = sub nuw i64 %78, %98
  %103 = icmp eq i64 %78, %98
  br i1 %103, label %.thread.i.i, label %.preheader.preheader.i.i

.preheader.preheader.i.i:
  %invariant.umin.i.i = call i64 @llvm.umin.i64(i64 %101, i64 9)
  %umax.i = call i64 @llvm.umax.i64(i64 %invariant.umin.i.i, i64 1)
  br label %.preheader.i.i

.preheader.i.i:
  ...
  %116 = add nuw nsw i64 %.011.i.i, 1
  %exitcond.not.i = icmp eq i64 %116, %umax.i
  br i1 %exitcond.not.i, label %.critedge.i.i, label %.preheader.i.i
```
As `%78` is not equal to `%98` in BB `.preheader.preheader.i.i`, we can
prove `%101` is non-zero. Then we can simplify the loop exit condition.

Addresses regression introduced by
llvm#112742.
dtcxzyw added a commit that referenced this pull request Nov 21, 2024
…e ops (#116983)

See the following case:
```
define i1 @test_logical_and_icmp_samesign(i8 %x) {
  %cmp1 = icmp ne i8 %x, 9
  %cmp2 = icmp samesign ult i8 %x, 11
  %and = select i1 %cmp1, i1 %cmp2, i1 false
  ret i1 %and
}
```
Currently we cannot convert this logical and into a bitwise and due to
the `samesign` flag. But if `%cmp2` evaluates to `poison`, we can infer
that `%cmp1` is either `poison` or `true` (`samesign` violation
indicates that X is negative). Therefore, `%and` still evaluates to
`poison`.

This patch converts a logical and into a bitwise and iff TV is poison
implies that Cond is either poison or true. Likewise, we convert a
logical or into a bitwise or iff FV is poison implies that Cond is
either poison or false.

Note:
1. This logic is implemented in InstCombine. Not sure whether it is
profitable to move it into ValueTracking and call `impliesPoison(TV/FV,
Sel)` instead.
2. We only handle the case that `ValAssumedPoison` is `icmp samesign
pred X, C1` and `V` is `icmp pred X, C2`. There are no suitable variants
for `isImpliedCondition` to pass the fact that X is [non-]negative.

Alive2: https://alive2.llvm.org/ce/z/eorFfa
Motivation: fix [a major
regression](dtcxzyw/llvm-opt-benchmark#1724 (comment))
to unblock #112742.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant