Skip to content

[InstSimplify] FP-related miscompile #58046

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

Jump to bottom
Closed
nikic opened this issue Sep 28, 2022 · 6 comments · Fixed by llvm/llvm-project-release-prs#174
Closed

[InstSimplify] FP-related miscompile #58046

nikic opened this issue Sep 28, 2022 · 6 comments · Fixed by llvm/llvm-project-release-prs#174
Assignees
@nikic
Labels
llvm:optimizations miscompilation release:backport release:merged
Milestone
LLVM 15.0.2 Release

Comments

@nikic
Copy link
Contributor

nikic commented Sep 28, 2022 •
edited by VoltrexKeyva
Loading

From rust-lang/rust#102402:

define i1 @src(i64 %0) {
  %i2 = uitofp i64 %0 to float
  %i3 = fpext float %i2 to double
  %i4 = fmul double -0.000000e+00, %i3
  %i5 = fdiv double 1.000000e+00, %i4
  %i6 = fcmp oeq double %i5, 0xFFF0000000000000
  ret i1 %i6
}
; RUN: opt -S -instsimplify
define i1 @src(i64 %0) {
  ret i1 false
}

Counter-proof: https://alive2.llvm.org/ce/z/6Rx_Cb
@nikic nikic mentioned this issue Sep 28, 2022
Closed
@nikic
Copy link
Contributor Author

nikic commented Sep 28, 2022

I believe this is a bug in CannotBeOrderedLessThanZero. For fdiv, this probably needs to do a SignBitOnly check, because it can turn a negative zero into a negative non-zero.

@nikic
Copy link
Contributor Author

nikic commented Sep 29, 2022

Candidate patch: https://reviews.llvm.org/D134876

@nikic nikic self-assigned this Sep 29, 2022
@nikic nikic added this to the LLVM 15.0.2 Release milestone Sep 29, 2022
@nikic nikic moved this from Needs Triage to Needs Fix in LLVM Release Status Sep 29, 2022
@nikic nikic moved this to Needs Triage in LLVM Release Status Sep 29, 2022
@nikic nikic closed this as completed in aa25c92 Sep 29, 2022
@nikic nikic moved this from Needs Fix to Needs Pull Request in LLVM Release Status Sep 29, 2022
@nikic nikic reopened this Sep 29, 2022
@nikic
Copy link
Contributor Author

nikic commented Sep 29, 2022

/cherry-pick ea32658 aa25c92

@llvmbot
Copy link
Member

llvmbot commented Sep 29, 2022

Failed to cherry-pick: aa25c92

https://github.com/llvm/llvm-project/actions/runs/3152450212

Please manually backport the fix and push it to your github fork. Once this is done, please add a comment like this:

/branch <user>/<repo>/<branch>

@nikic
Copy link
Contributor Author

nikic commented Sep 29, 2022

/branch nikic/llvm-project/backport-fdiv-fix

nikic added a commit to nikic/llvm-project that referenced this issue Sep 29, 2022
@nikic
[ValueTracking] Fix CannotBeOrderedLessThanZero() for fdiv (PR58046)
7ba4428 
When checking the RHS of fdiv, we should set the SignBitOnly flag,
because a negative zero can become -Inf, which is ordered less
than zero.

Fixes llvm#58046.

Differential Revision: https://reviews.llvm.org/D134876
@llvmbot llvmbot mentioned this issue Sep 29, 2022
Merged
@llvmbot
Copy link
Member

llvmbot commented Sep 29, 2022

/pull-request llvm/llvm-project-release-prs#174

@tru tru moved this from Needs Pull Request to Needs Merge in LLVM Release Status Sep 30, 2022
tru pushed a commit to llvm/llvm-project-release-prs that referenced this issue Sep 30, 2022
@nikic @tru
[ValueTracking] Fix CannotBeOrderedLessThanZero() for fdiv (PR58046)
77ff99c 
When checking the RHS of fdiv, we should set the SignBitOnly flag,
because a negative zero can become -Inf, which is ordered less
than zero.

Fixes llvm/llvm-project#58046.

Differential Revision: https://reviews.llvm.org/D134876
@tru tru moved this from Needs Merge to Done in LLVM Release Status Sep 30, 2022
rotateright added a commit that referenced this issue Oct 4, 2022
@rotateright
[InstSimplify] try harder to fold fmul with 0.0 operand
0a1210e 
https://alive2.llvm.org/ce/z/oShzr3

This was noted as a missing fold in D134876 (with additional
examples based on issue #58046).

I'm assuming that fmul with a zero operand is rare enough
that the use of ValueTracking will not noticeably increase
compile-time.

This adjusts a PowerPC codegen test that was added with D88388
because it would get folded away and no longer provide coverage
for the bug fix.
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue May 22, 2024
@nikic @veselypeta
[ValueTracking] Fix CannotBeOrderedLessThanZero() for fdiv (PR58046)
42fcda0 
When checking the RHS of fdiv, we should set the SignBitOnly flag,
because a negative zero can become -Inf, which is ordered less
than zero.

Fixes llvm/llvm-project#58046.

Differential Revision: https://reviews.llvm.org/D134876
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nikic nikic

Labels
llvm:optimizations miscompilation release:backport release:merged
Projects
LLVM Release Status
Archived in project
Milestone
LLVM 15.0.2 Release
Development

Successfully merging a pull request may close this issue.

PR for llvm/llvm-project#58046 llvm/llvm-project-release-prs
3 participants
@nikic @EugeneZelenko @llvmbot