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

runtime, cmd/compile: sqrt support on arm #15542

Closed
josharian opened this issue May 4, 2016 · 4 comments
Closed

runtime, cmd/compile: sqrt support on arm #15542

josharian opened this issue May 4, 2016 · 4 comments

Comments

@josharian
Copy link
Contributor

According to cmd/compile/internal/gc/walk.go, we assume that ARM has hardware sqrt support:

        if n.Left.Op == ONAME && n.Left.Sym.Name == "Sqrt" && n.Left.Sym.Pkg.Path == "math" {
            if Thearch.LinkArch.InFamily(sys.AMD64, sys.ARM, sys.ARM64, sys.PPC64, sys.S390X) {
                n.Op = OSQRT
                n.Left = n.List.First()
                n.List.Set(nil)
                break opswitch
            }
        }

But we have a runtime sqrt routine (runtime/sqrt.go), used only in runtime/softfloat_arm.go, which suggests that we are prepared for ARM not to have hardware support.

It seems like either:

(1) we can delete the runtime sqrt routine and use the hardware instruction instead by adding a check to walk.go that recognizes runtime.sqrt in addition to math.Sqrt
(2) we should have an extra ARM-related check somehow in walk.go
(3) I'm missing something

Which is it? :)

cc @davecheney @minux @cherrymui

@minux
Copy link
Member

minux commented May 4, 2016 via email

@josharian
Copy link
Contributor Author

It looks to me like we have an ARM5 bug then.

package x

import "math"

func root2() float64 {
    return math.Sqrt(2)
}

Compiling with GOARM=5 yields:

"".root2 t=1 size=72 args=0x8 locals=0x0
    0x0000 00000 (/Volumes/Data/src/go.tip/src/sqrt.go:5)   TEXT    "".root2(SB), $0-8
    0x0000 00000 (/Volumes/Data/src/go.tip/src/sqrt.go:5)   MOVW    8(g), R1
    0x0004 00004 (/Volumes/Data/src/go.tip/src/sqrt.go:5)   CMP R1, R13
    0x0008 00008 (/Volumes/Data/src/go.tip/src/sqrt.go:5)   BLS 48
    0x000c 00012 (/Volumes/Data/src/go.tip/src/sqrt.go:5)   MOVW.W  R14, -4(R13)
    0x0010 00016 (/Volumes/Data/src/go.tip/src/sqrt.go:5)   FUNCDATA    $0, gclocals·23e8278e2b69a3a75fa59b23c49ed6ad(SB)
    0x0010 00016 (/Volumes/Data/src/go.tip/src/sqrt.go:5)   FUNCDATA    $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
    0x0010 00016 (/Volumes/Data/src/go.tip/src/sqrt.go:5)   CALL    _sfloat(SB)
    0x0014 00020 (/Volumes/Data/src/go.tip/src/sqrt.go:5)   MOVD    $f64.0000000000000000(SB), F0
    0x001c 00028 (/Volumes/Data/src/go.tip/src/sqrt.go:6)   MOVD    $f64.4000000000000000(SB), F0
    0x0024 00036 (/Volumes/Data/src/go.tip/src/sqrt.go:6)   SQRTD   F0, F0
    0x0028 00040 (/Volumes/Data/src/go.tip/src/sqrt.go:6)   MOVD    F0, "".~r0(FP)
    0x002c 00044 (/Volumes/Data/src/go.tip/src/sqrt.go:6)   MOVW.P  4(R13), R15
    0x0030 00048 (/Volumes/Data/src/go.tip/src/sqrt.go:6)   NOP
    0x0030 00048 (/Volumes/Data/src/go.tip/src/sqrt.go:6)   MOVW    R14, R3
    0x0034 00052 (/Volumes/Data/src/go.tip/src/sqrt.go:6)   CALL    runtime.morestack_noctxt(SB)
    0x0038 00056 (/Volumes/Data/src/go.tip/src/sqrt.go:6)   JMP 0
    0x003c 00060 (/Volumes/Data/src/go.tip/src/sqrt.go:6)   JMP 0(PC)
    0x0040 00064 (/Volumes/Data/src/go.tip/src/sqrt.go:6)   WORD    $f64.0000000000000000(SB)
    0x0044 00068 (/Volumes/Data/src/go.tip/src/sqrt.go:6)   WORD    $f64.4000000000000000(SB)

Note that call to SQRTD.

I don't have an arm5 device to test on. Anyone else want to do the honors?

@minux
Copy link
Member

minux commented May 4, 2016 via email

@josharian
Copy link
Contributor Author

Whoah. Snazz.

@golang golang locked and limited conversation to collaborators May 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants