• Josh Bleecher Snyder's avatar
    cmd/compile: re-enable in-place append optimization · 03e216f3
    Josh Bleecher Snyder authored
    CL 21891 was too clever in its attempts to avoid spills.
    Storing newlen too early caused uses of append in the runtime
    itself to receive an inconsistent view of a slice,
    leading to corruption.
    
    This CL makes the generate code much more similar to
    the old backend. It spills more than before,
    but those spills have been contained to the grow path.
    It recalculates newlen unnecessarily on the fast path,
    but that's measurably cheaper than spilling it.
    
    CL 21891 caused runtime failures in 6 of 2000 runs
    of net/http and crypto/x509 in my test setup.
    This CL has gone 6000 runs without a failure.
    
    
    Benchmarks going from master to this CL:
    
    name                         old time/op  new time/op  delta
    AppendInPlace/NoGrow/Byte-8   439ns ± 2%   436ns ± 2%  -0.72%  (p=0.001 n=28+27)
    AppendInPlace/NoGrow/1Ptr-8   901ns ± 0%   856ns ± 0%  -4.95%  (p=0.000 n=26+29)
    AppendInPlace/NoGrow/2Ptr-8  2.15µs ± 1%  1.95µs ± 0%  -9.07%  (p=0.000 n=28+30)
    AppendInPlace/NoGrow/3Ptr-8  2.66µs ± 0%  2.45µs ± 0%  -7.93%  (p=0.000 n=29+26)
    AppendInPlace/NoGrow/4Ptr-8  3.24µs ± 1%  3.02µs ± 1%  -6.75%  (p=0.000 n=28+30)
    AppendInPlace/Grow/Byte-8     269ns ± 1%   271ns ± 1%  +0.84%  (p=0.000 n=30+29)
    AppendInPlace/Grow/1Ptr-8     275ns ± 1%   280ns ± 1%  +1.75%  (p=0.000 n=30+30)
    AppendInPlace/Grow/2Ptr-8     384ns ± 0%   391ns ± 0%  +1.94%  (p=0.000 n=27+30)
    AppendInPlace/Grow/3Ptr-8     455ns ± 0%   462ns ± 0%  +1.43%  (p=0.000 n=29+29)
    AppendInPlace/Grow/4Ptr-8     478ns ± 0%   479ns ± 0%  +0.23%  (p=0.000 n=30+27)
    
    
    However, for the large no-grow cases, there is still more work to be done.
    Going from this CL to the non-SSA backend:
    
    name                         old time/op  new time/op  delta
    AppendInPlace/NoGrow/Byte-8   436ns ± 2%   436ns ± 2%     ~     (p=0.967 n=27+29)
    AppendInPlace/NoGrow/1Ptr-8   856ns ± 0%   884ns ± 0%   +3.28%  (p=0.000 n=29+26)
    AppendInPlace/NoGrow/2Ptr-8  1.95µs ± 0%  1.56µs ± 0%  -20.28%  (p=0.000 n=30+29)
    AppendInPlace/NoGrow/3Ptr-8  2.45µs ± 0%  1.89µs ± 0%  -22.88%  (p=0.000 n=26+28)
    AppendInPlace/NoGrow/4Ptr-8  3.02µs ± 1%  2.56µs ± 1%  -15.35%  (p=0.000 n=30+28)
    AppendInPlace/Grow/Byte-8     271ns ± 1%   283ns ± 1%   +4.56%  (p=0.000 n=29+29)
    AppendInPlace/Grow/1Ptr-8     280ns ± 1%   288ns ± 1%   +2.99%  (p=0.000 n=30+30)
    AppendInPlace/Grow/2Ptr-8     391ns ± 0%   409ns ± 0%   +4.66%  (p=0.000 n=30+29)
    AppendInPlace/Grow/3Ptr-8     462ns ± 0%   481ns ± 0%   +4.13%  (p=0.000 n=29+30)
    AppendInPlace/Grow/4Ptr-8     479ns ± 0%   502ns ± 0%   +4.81%  (p=0.000 n=27+26)
    
    
    New generated code:
    
    var x []byte
    
    func a() {
    	x = append(x, 1)
    }
    
    
    "".a t=1 size=208 args=0x0 locals=0x48
    	0x0000 00000 (a.go:5)	TEXT	"".a(SB), $72-0
    	0x0000 00000 (a.go:5)	MOVQ	(TLS), CX
    	0x0009 00009 (a.go:5)	CMPQ	SP, 16(CX)
    	0x000d 00013 (a.go:5)	JLS	190
    	0x0013 00019 (a.go:5)	SUBQ	$72, SP
    	0x0017 00023 (a.go:5)	FUNCDATA	$0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
    	0x0017 00023 (a.go:5)	FUNCDATA	$1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
    	0x0017 00023 (a.go:6)	MOVQ	"".x+16(SB), CX
    	0x001e 00030 (a.go:6)	MOVQ	"".x+8(SB), DX
    	0x0025 00037 (a.go:6)	MOVQ	"".x(SB), BX
    	0x002c 00044 (a.go:6)	LEAQ	1(DX), BP
    	0x0030 00048 (a.go:6)	CMPQ	BP, CX
    	0x0033 00051 (a.go:6)	JGT	$0, 73
    	0x0035 00053 (a.go:6)	LEAQ	1(DX), AX
    	0x0039 00057 (a.go:6)	MOVQ	AX, "".x+8(SB)
    	0x0040 00064 (a.go:6)	MOVB	$1, (BX)(DX*1)
    	0x0044 00068 (a.go:7)	ADDQ	$72, SP
    	0x0048 00072 (a.go:7)	RET
    	0x0049 00073 (a.go:6)	LEAQ	type.[]uint8(SB), AX
    	0x0050 00080 (a.go:6)	MOVQ	AX, (SP)
    	0x0054 00084 (a.go:6)	MOVQ	BX, 8(SP)
    	0x0059 00089 (a.go:6)	MOVQ	DX, 16(SP)
    	0x005e 00094 (a.go:6)	MOVQ	CX, 24(SP)
    	0x0063 00099 (a.go:6)	MOVQ	BP, 32(SP)
    	0x0068 00104 (a.go:6)	PCDATA	$0, $0
    	0x0068 00104 (a.go:6)	CALL	runtime.growslice(SB)
    	0x006d 00109 (a.go:6)	MOVQ	40(SP), CX
    	0x0072 00114 (a.go:6)	MOVQ	48(SP), DX
    	0x0077 00119 (a.go:6)	MOVQ	DX, "".autotmp_0+64(SP)
    	0x007c 00124 (a.go:6)	MOVQ	56(SP), BX
    	0x0081 00129 (a.go:6)	MOVQ	BX, "".x+16(SB)
    	0x0088 00136 (a.go:6)	MOVL	runtime.writeBarrier(SB), AX
    	0x008e 00142 (a.go:6)	TESTB	AL, AL
    	0x0090 00144 (a.go:6)	JNE	$0, 162
    	0x0092 00146 (a.go:6)	MOVQ	CX, "".x(SB)
    	0x0099 00153 (a.go:6)	MOVQ	"".x(SB), BX
    	0x00a0 00160 (a.go:6)	JMP	53
    	0x00a2 00162 (a.go:6)	LEAQ	"".x(SB), BX
    	0x00a9 00169 (a.go:6)	MOVQ	BX, (SP)
    	0x00ad 00173 (a.go:6)	MOVQ	CX, 8(SP)
    	0x00b2 00178 (a.go:6)	PCDATA	$0, $0
    	0x00b2 00178 (a.go:6)	CALL	runtime.writebarrierptr(SB)
    	0x00b7 00183 (a.go:6)	MOVQ	"".autotmp_0+64(SP), DX
    	0x00bc 00188 (a.go:6)	JMP	153
    	0x00be 00190 (a.go:6)	NOP
    	0x00be 00190 (a.go:5)	CALL	runtime.morestack_noctxt(SB)
    	0x00c3 00195 (a.go:5)	JMP	0
    
    
    Fixes #14969 again
    
    Change-Id: Ia50463b1f506011aad0718a4fef1d4738e43c32d
    Reviewed-on: https://go-review.googlesource.com/22197
    Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
    TryBot-Result: Gobot Gobot <gobot@golang.org>
    Reviewed-by: 's avatarKeith Randall <khr@golang.org>
    03e216f3
ssa.go 133 KB