math: Poor performance for math.Sqrt on ppc64le/ppc64

[laboger]

The implementation of the Sqrt function is written in Go and performs poorly on ppc64le & ppc64. For other platforms there are asm implementations.

I have a change to use the asm sqrt instruction to improve the performance on Power. I will post that change shortly.

[laboger]

https://go-review.googlesource.com/#/c/19515/

Improved some sqrt testcases by 10X or more.

Further improvement if this was inlined so that the load of the argument and store of the return value could be avoided.

[gopherbot]

CL https://golang.org/cl/19515 mentions this issue.

[mundaym]

There is an easy to miss hack in cmd/compile/internal/gc/walk.go which inlines the square root function on platforms where it is a single instruction:

if n.Left.Op == ONAME && n.Left.Sym.Name == "Sqrt" && n.Left.Sym.Pkg.Path == "math" {
    switch Thearch.Thechar {
        case '5', '6', '7':
            n.Op = OSQRT
            n.Left = n.List.N
            n.List = nil
            break opswitch
        }
}

It sounds like '9' should be added to the case statement.

[laboger]

Thanks! I'll try it out.

On 02/19/2016 10:04 AM, Michael Munday wrote:

There is an easy to miss hack in cmd/compile/internal/gc/walk.go which
inlines the square root function on platforms where it is a single
instruction:

if n.Left.Op ==ONAME && n.Left.Sym.Name =="Sqrt" && n.Left.Sym.Pkg.Path =="math" {
switch Thearch.Thechar {
case '5','6','7':
n.Op = OSQRT
n.Left = n.List.N
n.List =nil
break opswitch
}
}

It sounds like |'9'| should be added to the case statement.

—
Reply to this email directly or view it on GitHub
#14349 (comment).

[minux]

You also need to handle gc.OSQRT in optoas in the ppc64 backend.

[laboger]

I had to change a few files to get the inlining to work for sqrt but I think it is right now. Are these additional files something I can add to my original CL or should I create a new one?

[bradfitz]

Add to your existing CL.