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

cmd/compile: unnecessary branching #15031

Open
randall77 opened this Issue Mar 30, 2016 · 2 comments

Comments

Projects
None yet
3 participants
@randall77
Contributor

randall77 commented Mar 30, 2016

type T struct {
    x, y, z int
}
func f(x interface{}) int {
    t, ok := x.(*T)
    if ok && t != nil {
        return 7
    }
    return 3
}

The SSA compiler for this function generates some extra branching logic that seems unnecessary.

    0x0000 00000 (bin.go:12)    LEAQ    type.*"".T(SB), AX
    0x0007 00007 (bin.go:8) MOVQ    "".x+8(FP), CX
    0x000c 00012 (bin.go:8) CMPQ    AX, CX
    0x000f 00015 (bin.go:8) JNE $0, 49
    0x0011 00017 (bin.go:7) MOVQ    "".x+16(FP), AX
    0x0016 00022 (bin.go:9) JNE 29
    0x0018 00024 (bin.go:9) TESTQ   AX, AX
    0x001b 00027 (bin.go:9) JNE $0, 39
    0x001d 00029 (bin.go:12)    MOVQ    $3, "".~r1+24(FP)
    0x0026 00038 (bin.go:12)    RET
    0x0027 00039 (bin.go:10)    MOVQ    $7, "".~r1+24(FP)
    0x0030 00048 (bin.go:10)    RET
    0x0031 00049 (bin.go:8) MOVQ    $0, AX
    0x0038 00056 (bin.go:9) JMP 22

The 4th line there could be JNE directly to line 29. Instead it goes to 49/56, rejoins the main flow at 22, and then goes to 29.
Something wrong with the shortcircuit pass, maybe?

@bradfitz

This comment has been minimized.

Member

bradfitz commented Mar 30, 2016

Related: the ok value can be ignored entirely. If t is non-nil, that implies that ok is true anyway:

func f(x interface{}) int {
    t, _ := x.(*T)
    if t != nil {
        return 7
    }
    return 3
}
@josharian

This comment has been minimized.

Contributor

josharian commented Mar 31, 2016

Related: #12628

@bradfitz bradfitz added the Performance label Apr 7, 2016

@bradfitz bradfitz added this to the Unplanned milestone Apr 7, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment