cmd/compile: assume use of variable if explicitly assigned to _

[dsnet]

Using Go1.7rc6

The new SSA compiler is too smart ;)

Consider the following benchmark:

func ReverseUint64(v uint64) uint64 {
    v = (v&0xaaaaaaaaaaaaaaaa)>>1 | (v&0x5555555555555555)<<1
    v = (v&0xcccccccccccccccc)>>2 | (v&0x3333333333333333)<<2
    v = (v&0xf0f0f0f0f0f0f0f0)>>4 | (v&0x0f0f0f0f0f0f0f0f)<<4
    v = (v&0xff00ff00ff00ff00)>>8 | (v&0x00ff00ff00ff00ff)<<8
    v = (v&0xffff0000ffff0000)>>16 | (v&0x0000ffff0000ffff)<<16
    v = (v&0xffffffff00000000)>>32 | (v&0x00000000ffffffff)<<32
    return v
}

func BenchmarkReverse64(b *testing.B) {
    u := uint64(0x123456789abcdef)
    for i := 0; i < b.N; i++ {
        ReverseUint64(u)
    }
}

On Go 1.6, this runs in 4.20ns/op, while on Go 1.7, this runs in 0.28ns/op. While this sounds wonderful, the problem is that the above benchmark is completely useless on Go 1.7.

The current compiler compiles the BenchmarkReverse64 as the following:

    0x0000 00000 (/tmp/sandbox508321108/main_test.go:21)    MOVQ    "".b+8(FP), AX
    0x0005 00005 (/tmp/sandbox508321108/main_test.go:21)    MOVQ    $0, CX
    0x0007 00007 (/tmp/sandbox508321108/main_test.go:21)    MOVQ    184(AX), DX
    0x000e 00014 (/tmp/sandbox508321108/main_test.go:21)    CMPQ    CX, DX
    0x0011 00017 (/tmp/sandbox508321108/main_test.go:21)    JGE $0, 34
    0x0013 00019 (/tmp/sandbox508321108/main_test.go:21)    INCQ    CX
    0x0016 00022 (/tmp/sandbox508321108/main_test.go:21)    MOVQ    184(AX), DX
    0x001d 00029 (/tmp/sandbox508321108/main_test.go:21)    CMPQ    CX, DX
    0x0020 00032 (/tmp/sandbox508321108/main_test.go:21)    JLT $0, 19

which completely eliminates all of the code worth benchmarking.

The optimizations that the current compiler does is reasonable, IMHO. So I tried adjusting my benchmark to the following:

func BenchmarkReverse64(b *testing.B) {
    u := uint64(0x123456789abcdef)
    var v uint64
    for i := 0; i < b.N; i++ {
        v += ReverseUint64(u)
    }
    _ = v
}

My goal was to try and use the result of ReverseUint64 to defeat the optimization. However, the optimizations still occurred and the benchmark measures pretty much nothing.

In order to finally get it to measure properly I did the following hack:

func BenchmarkReverse64(b *testing.B) {
    u := uint64(0x123456789abcdef)
    var v uint64
    for i := 0; i < b.N; i++ {
        v += ReverseUint64(u)
    }
    if &v == nil {
        b.Log(v) // Ensure that the compiler thinks that v is being used.
    }
}   

However, this is clunky and not very intuitive.

I propose that the assignment of a variable to _ by itself indicates that the variable is being used, and this prevents whatever optimization is screwing with benchmarks.

/CC @randall77 @dr2chase @cherrymui

[ALTree]

However, this is clunky and not very intuitive.

I agree, but do you really need that? Just assign to a package level variable:

var sink uint64

func BenchmarkReverse64(b *testing.B) {
    u := uint64(0x123456789abcdef)
    for i := 0; i < b.N; i++ {
        sink = ReverseUint64(u)
    }
}

BenchmarkReverse64-4    500000000            4.16 ns/op

[bradfitz]

Yeah, just use a package-level sink variable. I feel like I've seen this discussed before and there was reluctance to assign any magic to underscore.

[randall77]

We thought about using _ = ... for runtime.KeepAlive(...). It seemed too much magic. This is similar. Just use a package-level sink. See #15843, #15277, #13347.

[ianlancetaylor]

Benchmarking is hard, but the answer is not to change the language to make benchmarking easier. That just leads you down the garden path of measuring code that does not correspond to any real code.