Skip to content

Missed optimization in math expression: squashing sqrt functions #34940

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 · 5 comments
Closed

Missed optimization in math expression: squashing sqrt functions #34940

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

Comments

@zamazan4ik
Copy link

zamazan4ik commented Dec 10, 2017

Bugzilla Link 35592
Resolution FIXED
Resolved on Apr 15, 2018 20:04
Version trunk
OS All
Blocks #34959
CC @hfinkel,@jryans,@RKSimon,@rotateright

Extended Description

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

#include <cmath>

double f(double a, double b, double c, double d)
{
    return sqrt(a)*sqrt(b)*sqrt(c)*sqrt(d);
}

generates this assembly:

f(double, double, double, double): # @f(double, double, double, double)
  vsqrtsd xmm0, xmm0, xmm0
  vsqrtsd xmm1, xmm1, xmm1
  vmulsd xmm0, xmm1, xmm0
  vsqrtsd xmm1, xmm2, xmm2
  vsqrtsd xmm2, xmm3, xmm3
  vmulsd xmm1, xmm1, xmm2
  vmulsd xmm0, xmm0, xmm1
  ret

GCC (trunk) with '--std=c++17 -O3 -march=native -ffast-math' flags generates more optimal code:

f(double, double, double, double):
        vmulsd  xmm0, xmm0, xmm1
        vmulsd  xmm2, xmm2, xmm3
        vmulsd  xmm2, xmm2, xmm0
        vsqrtsd xmm0, xmm0, xmm2
        ret
@hfinkel
Copy link
Collaborator

hfinkel commented Dec 10, 2017

It will certainly make sense to have sqrt(a)sqrt(b) -> sqrt(ab) in InstCombine.

@zamazan4ik
Copy link
Author

@hfinkel
Copy link
Collaborator

hfinkel commented Dec 10, 2017

See a lot of optimizations here:
https://github.com/gcc-mirror/gcc/blob/
07b69d3f1cd3dd8ebb0af1fbff95914daee477d2/gcc/match.pd

You might want to create a "meta bug" about all of these, and then link the individual bug reports to that as dependencies.

Also, there are other sources of these kinds of things that we should look at. For example, in https://github.com/sympy/sympy/blob/master/sympy/functions/elementary/trigonometric.py we have:

class cos(TrigonometricFunction):
  ...
    def eval(cls, arg):
  ...
        if isinstance(arg, acos):
            return arg.args[0]

        if isinstance(arg, atan):
            x = arg.args[0]
            return 1 / sqrt(1 + x**2)

        if isinstance(arg, atan2):
            y, x = arg.args
            return x / sqrt(x**2 + y**2)

        if isinstance(arg, asin):
            x = arg.args[0]
            return sqrt(1 - x ** 2)

        if isinstance(arg, acot):
            x = arg.args[0]
            return 1 / sqrt(1 + 1 / x**2)

        if isinstance(arg, acsc):
            x = arg.args[0]
            return sqrt(1 - 1 / x**2)

        if isinstance(arg, asec):
            x = arg.args[0]
            return 1 / x

@jryans
Copy link
Member

jryans commented Apr 16, 2018

It appears this was reviewed in https://reviews.llvm.org/D41322 and fixed in r321637.

@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