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: redundant loop generated for comparing/hashing arrays of size 1. #14723

Open
brtzsnr opened this Issue Mar 8, 2016 · 3 comments

Comments

Projects
None yet
3 participants
@brtzsnr
Contributor

brtzsnr commented Mar 8, 2016

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
    go version devel +f81cfc6 Tue Mar 8 23:54:09 2016 +0100 linux/amd64
    (local compile)
  2. What operating system and processor architecture are you using (go env)?
    GOARCH="amd64"
    GOHOSTARCH="amd64"
  3. What did you do?
    go tool objdump vet| less
  4. What did you expect to see?
    No loop. There are many instances of this in the standard library.
  5. What did you see instead?
    There is clearly a 1 trip loop in the following assembly code;
TEXT type..hash.[1]go/types.operand(SB) /.../go/src/go/types/api.go
        api.go:1        0x524af0        64488b0c25f8ffffff      FS MOVQ FS:0xfffffff8, CX
        api.go:1        0x524af9        483b6110                CMPQ 0x10(CX), SP
        api.go:1        0x524afd        7658                    JBE 0x524b57
        api.go:1        0x524aff        4883ec20                SUBQ $0x20, SP
        api.go:1        0x524b03        31c0                    XORL AX, AX
        api.go:1        0x524b05        488b4c2430              MOVQ 0x30(SP), CX
        api.go:1        0x524b0a        4889442418              MOVQ AX, 0x18(SP)
        api.go:1        0x524b0f        4883f801                CMPQ $0x1, AX   
        api.go:1        0x524b13        7d38                    JGE 0x524b4d
        api.go:1        0x524b15        488b542428              MOVQ 0x28(SP), DX   //  LOOP START
        api.go:1        0x524b1a        8402                    TESTL AL, 0(DX)
        api.go:1        0x524b1c        4889c3                  MOVQ AX, BX
        api.go:1        0x524b1f        48c1e306                SHLQ $0x6, BX
        api.go:1        0x524b23        4801d3                  ADDQ DX, BX
        api.go:1        0x524b26        48891c24                MOVQ BX, 0(SP)
        api.go:1        0x524b2a        48894c2408              MOVQ CX, 0x8(SP)
        api.go:1        0x524b2f        e8ccf1ffff              CALL type..hash.go/types.operand(SB)
        api.go:1        0x524b34        488b4c2410              MOVQ 0x10(SP), CX
        api.go:1        0x524b39        488b542418              MOVQ 0x18(SP), DX
        api.go:1        0x524b3e        488d4201                LEAQ 0x1(DX), AX
        api.go:1        0x524b42        4889442418              MOVQ AX, 0x18(SP)
        api.go:1        0x524b47        4883f801                CMPQ $0x1, AX
        api.go:1        0x524b4b        7cc8                    JL 0x524b15          // LOOP END
        api.go:1        0x524b4d        48894c2438              MOVQ CX, 0x38(SP)
        api.go:1        0x524b52        4883c420                ADDQ $0x20, SP
        api.go:1        0x524b56        c3                      RET
        api.go:1        0x524b57        e8b4d5f4ff              CALL runtime.morestack_noctxt(SB)
        api.go:1        0x524b5c        eb92                    JMP type..hash.[1]go/types.operand(SB)
        api.go:1        0x524b5e        cc                      INT $0x3
        api.go:1        0x524b5f        cc                      INT $0x3

@brtzsnr brtzsnr changed the title from cmd/compile: redundant loop generated of comparing/hashing arrays of size 1. to cmd/compile: redundant loop generated for comparing/hashing arrays of size 1. Mar 8, 2016

@josharian

This comment has been minimized.

Contributor

josharian commented Mar 9, 2016

I'll fix this one. I thought I already had. :(

@josharian josharian self-assigned this Mar 9, 2016

@josharian

This comment has been minimized.

Contributor

josharian commented Mar 9, 2016

Heads up--this fix will take a little while, as I'm going to do it somewhat indirectly.

@bradfitz bradfitz added the Performance label Mar 9, 2016

@bradfitz bradfitz added this to the Go1.7 milestone Mar 9, 2016

@josharian

This comment has been minimized.

Contributor

josharian commented May 3, 2016

This code is basically never executed. The right thing to do is not to generate these algs. Moving to unplanned.

@josharian josharian modified the milestones: Unplanned, Go1.7 May 3, 2016

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