Skip to content

[InstCombine] Unable to reach fixed point in 1 iteration when new folds are unblocked by knownbits. #87015

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

Closed
goldsteinn opened this issue Mar 28, 2024 · 2 comments
Labels
invalid Resolved as invalid, i.e. not a bug llvm:instcombine

Comments

@goldsteinn
Copy link
Contributor

goldsteinn commented Mar 28, 2024

Take the following examples:

define i8 @src(i8 %x, i8 %y, i8 %z) {
  %cmp = icmp eq i8 %x, %y
  %conv16 = zext i1 %cmp to i16
  %conv8 = zext i1 %cmp to i8
  %r = add i8 %conv8, %z
  %cmp_assume = icmp eq i16 %conv16, 0
  call void @llvm.assume(i1 %cmp_assume)
  ret i8 %r
}

When run with opt -passes=instcombine we run into:

LLVM ERROR: Instruction Combining did not reach a fixpoint after 1 iterations

https://godbolt.org/z/6Exn4r8jc

The issue is because we try to transform the %r = add i8 %conv8, %z
before we get to the %cmp_assume = icmp eq i16 %conv16, 0.

But we end up transform %cmp_assume = icmp eq i16 %conv16, 0
-> %cmp_assume = icmp eq i1 %cmp, false which, with the assume,
we are able to use to prove %cmp is false and thus %conv8 is 0
and then on iteration two simplifying the %r = add i8 %conv8, %z.

This particular cases is pretty easy to solve by supporting (zext/sext op)
in the AssumptionCache, but you could really generate any amount of
these issues by having a fold create a pattern we recognize in
computeKnownBitsFromCmp and a having a prior node that
we will only fold w/ certain known bits.

I think the fix is that when we add an assume, we need to
also add all instructions that reference it in the AssumptionCache

@goldsteinn
Copy link
Contributor Author

@nikic and @dtcxzyw

@goldsteinn
Copy link
Contributor Author

Probably a non-issue? I see us hitting this in other places (for some reason thought this was something we enforced).

@EugeneZelenko EugeneZelenko added the invalid Resolved as invalid, i.e. not a bug label Mar 29, 2024
@EugeneZelenko EugeneZelenko closed this as not planned Won't fix, can't repro, duplicate, stale Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid Resolved as invalid, i.e. not a bug llvm:instcombine
Projects
None yet
Development

No branches or pull requests

2 participants