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

Incorrect transformation of fabs with nnan flag #47304

Closed
aqjune opened this issue Oct 24, 2020 · 2 comments
Closed

Incorrect transformation of fabs with nnan flag #47304

aqjune opened this issue Oct 24, 2020 · 2 comments
Labels
bugzilla Issues migrated from bugzilla

Comments

@aqjune
Copy link
Contributor

aqjune commented Oct 24, 2020

Bugzilla Link 47960
Resolution FIXED
Resolved on Jul 25, 2021 07:46
Version trunk
OS All
CC @LebedevRI,@RKSimon,@nunoplopes,@regehr,@rotateright
Fixed by commit(s) 7bd3612

Extended Description

// llvm/test/Transforms/InstCombine/fabs.ll

define double @​select_fcmp_nnan_ole_zero(double %x) {
%0:
  %lezero = fcmp ole double %x, 0.000000
  %negx = fsub nnan double 0.000000, %x
  %fabs = select i1 %lezero, double %negx, double %x
  ret double %fabs
}
=>
define double @​select_fcmp_nnan_ole_zero(double %x) {
%0:
  %1 = fabs nnan double %x
  ret double %1
}
Transformation doesn't verify!
ERROR: Target is more poisonous than source

Example:
double %x = NaN

Source:
i1 %lezero = #x0 (0)
double %negx = poison
double %fabs = NaN

Target:
double %1 = poison
Source value: NaN
Target value: poison

After the transformation, nnan flag should be dropped because %x can be NaN.

@rotateright
Copy link
Contributor

@rotateright
Copy link
Contributor

This has potential to cause perf regressions:
https://reviews.llvm.org/rG7bd361200a7b
...but hopefully that makes the transform sound for all existing regression tests.

We discussed follow-ups to improve the transform in the review.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

2 participants