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

[CVP] Failure to recognise trunc nsw from assumptions #130100

Closed
RKSimon opened this issue Mar 6, 2025 · 6 comments · Fixed by #130504
Closed

[CVP] Failure to recognise trunc nsw from assumptions #130100

RKSimon opened this issue Mar 6, 2025 · 6 comments · Fixed by #130504
Labels
good first issue https://github.com/llvm/llvm-project/contribute llvm:transforms missed-optimization

Comments

@RKSimon
Copy link
Collaborator

RKSimon commented Mar 6, 2025

Pulled out of #130088

----------------------------------------
define i8 @src(i16 noundef %x) {
#0:
  %add = add i16 noundef %x, 128
  %or.cond.i = icmp ult i16 %add, 256
  assume i1 %or.cond.i
  %conv1 = trunc i16 noundef %x to i8
  ret i8 %conv1
}
=>
define i8 @tgt(i16 noundef %x) {
#0:
  %conv1 = trunc nsw i16 noundef %x to i8
  ret i8 %conv1
}
Transformation seems to be correct!

https://alive2.llvm.org/ce/z/2qhZy4

@dtcxzyw dtcxzyw added good first issue https://github.com/llvm/llvm-project/contribute llvm:optimizations missed-optimization and removed new issue labels Mar 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 6, 2025

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

  1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
  2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
  3. Fix the issue locally.
  4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
  5. Create a Git commit.
  6. Run git clang-format HEAD~1 to format your changes.
  7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

@llvmbot
Copy link
Member

llvmbot commented Mar 6, 2025

@llvm/issue-subscribers-good-first-issue

Author: Simon Pilgrim (RKSimon)

Pulled out of #130088 ```ll ---------------------------------------- define i8 @src(i16 noundef %x) { #0: %add = add i16 noundef %x, 128 %or.cond.i = icmp ult i16 %add, 256 assume i1 %or.cond.i %conv1 = trunc i16 noundef %x to i8 ret i8 %conv1 } => define i8 @tgt(i16 noundef %x) { #0: %conv1 = trunc nsw i16 noundef %x to i8 ret i8 %conv1 } Transformation seems to be correct! ``` https://alive2.llvm.org/ce/z/2qhZy4

@dtcxzyw dtcxzyw changed the title Failure to recognise trunc nsw from assumptions [CVP] Failure to recognise trunc nsw from assumptions Mar 6, 2025
@bababuck
Copy link
Contributor

bababuck commented Mar 6, 2025

@dtcxzyw Can iI give this a go if no one else is already working on it?

@ArunaHulakoti
Copy link

@llvmbot Could you please provide more details on the expected behavior?

@dtcxzyw dtcxzyw linked a pull request Mar 10, 2025 that will close this issue
@dtcxzyw
Copy link
Member

dtcxzyw commented Mar 10, 2025

@dtcxzyw Can iI give this a go if no one else is already working on it?

Sorry to tell you that this issue is covered by #130504.

@veera-sivarajan
Copy link
Contributor

@bababuck sorry, I didn't know you were assigned to this.

frederik-h pushed a commit to frederik-h/llvm-project that referenced this issue Mar 18, 2025
Proof: https://alive2.llvm.org/ce/z/U-G7yV

Helps: rust-lang/rust#72646 and
rust-lang/rust#122734

  Rust compiler's current output: https://godbolt.org/z/7E3fET6Md

IPSCCP can do this transform but it does not help the motivating issue
since it runs only once early in the optimization pipeline.

Reimplementing this in CVP folds the motivating issue into a simple
`icmp eq` instruction.
  
  Fixes llvm#130100
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue https://github.com/llvm/llvm-project/contribute llvm:transforms missed-optimization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants