Skip to content

Missed optimization in math expression: sqrt(pow(a, 2.0)) == |a| #34949

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
zamazan4ik opened this issue Dec 10, 2017 · 8 comments
Closed

Missed optimization in math expression: sqrt(pow(a, 2.0)) == |a| #34949

zamazan4ik opened this issue Dec 10, 2017 · 8 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla

Comments

@zamazan4ik
Copy link

zamazan4ik commented Dec 10, 2017

Bugzilla Link 35601
Resolution FIXED
Resolved on Dec 10, 2017 09:38
Version trunk
OS All
Blocks #34959
CC @hfinkel,@rotateright

Extended Description

clang(trunk) with '--std=c++17 -O3 -march=native -ffast-math' flags for this code:

#include <cmath>

double test(double a, double b, double c, double d)
{
    return sqrt(pow(a, 2.0));
}

generates this assembly:

test(double, double, double, double): # @test(double, double, double, double)
  vmulsd xmm0, xmm0, xmm0
  vsqrtsd xmm0, xmm0, xmm0
  ret

gcc(trunk) with '--std=c++17 -O3 -march=native -ffast-math' flags generates this:

test(double, double, double, double):
        vandpd  xmm0, xmm0, XMMWORD PTR .LC0[rip]
        ret
.LC0:
        .long   4294967295
        .long   2147483647
        .long   0
        .long   0
@zamazan4ik
Copy link
Author

assigned to @rotateright

@hfinkel
Copy link
Collaborator

hfinkel commented Dec 10, 2017

Could be in InstCombine.

@rotateright
Copy link
Contributor

I thought we already had this one via:
// We're looking for a repeated factor in a multiplication tree,
// so we can do this fold: sqrt(x * x) -> fabs(x);

Let me see how it escaped.

@rotateright
Copy link
Contributor

rotateright commented Dec 10, 2017

The fmul lost its 'fast' FMF:

define double @test(double) local_unnamed_addr #0 {
  %2 = fmul double %0, %0
  %3 = tail call fast double @llvm.sqrt.f64(double %2)
  ret double %3
}

@zamazan4ik
Copy link
Author

@rotateright
Copy link
Contributor

Should be fixed after:
https://reviews.llvm.org/rL320309
https://reviews.llvm.org/rL320310

Note that there's a sibling bug for pow(x, -1.0) that is still alive (I marked it with a 'FIXME' comment in the source).

Also, we should be getting these folds with vector types too, but I suspect we're missing a bunch of those cases.

@rotateright
Copy link
Contributor

Note that there's a sibling bug for pow(x, -1.0) that is still alive (I
marked it with a 'FIXME' comment in the source).

https://reviews.llvm.org/rL320312

I don't think that's a factor in any of the related bugs so far, but feel free to double-check. Thanks for filing these!

@RKSimon
Copy link
Collaborator

RKSimon commented Nov 27, 2021

mentioned in issue #34959

@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

4 participants