From: Austin Clements Date: Wed, 15 Nov 2017 22:43:05 +0000 (-0800) Subject: runtime: remove another TODO X-Git-Tag: go1.10beta1~217 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=bf9ad7080d0a22acf502a60d8bc6ebbc4f5340ef;p=gostls13.git runtime: remove another TODO I experimented with having the compiler spill the two registers that are clobbered by the write barrier fast path, but it slightly slows down compilebench, which is a good write barrier benchmark: name old time/op new time/op delta Template 175ms ± 0% 176ms ± 1% ~ (p=0.393 n=10+10) Unicode 83.6ms ± 1% 85.1ms ± 2% +1.79% (p=0.000 n=9+10) GoTypes 585ms ± 0% 588ms ± 1% ~ (p=0.173 n=8+10) Compiler 2.78s ± 1% 2.81s ± 2% +0.81% (p=0.023 n=10+10) SSA 7.11s ± 1% 7.15s ± 1% +0.59% (p=0.029 n=10+10) Flate 115ms ± 1% 116ms ± 2% ~ (p=0.853 n=10+10) GoParser 144ms ± 2% 145ms ± 2% ~ (p=1.000 n=10+10) Reflect 389ms ± 1% 390ms ± 1% ~ (p=0.481 n=10+10) Tar 185ms ± 2% 185ms ± 2% ~ (p=0.529 n=10+10) XML 205ms ± 0% 207ms ± 2% ~ (p=0.065 n=9+10) Since this didn't pan out, remove the TODO. Change-Id: I2186942c6d1ba10585a5da03cd7c1d26ce906273 Reviewed-on: https://go-review.googlesource.com/78034 Reviewed-by: Brad Fitzpatrick Reviewed-by: Rick Hudson --- diff --git a/src/runtime/asm_amd64.s b/src/runtime/asm_amd64.s index 7e13458b0b..576a61ca6c 100644 --- a/src/runtime/asm_amd64.s +++ b/src/runtime/asm_amd64.s @@ -2380,11 +2380,8 @@ TEXT runtime·addmoduledata(SB),NOSPLIT,$0-0 // It clobbers FLAGS. It does not clobber any general-purpose registers, // but may clobber others (e.g., SSE registers). TEXT runtime·gcWriteBarrier(SB),NOSPLIT,$120 - // Save the registers clobbered by the fast path. - // - // TODO: Teach the register allocator that this clobbers some registers - // so we don't always have to save them? Use regs it's least likely to - // care about. + // Save the registers clobbered by the fast path. This is slightly + // faster than having the caller spill these. MOVQ R14, 104(SP) MOVQ R13, 112(SP) // TODO: Consider passing g.m.p in as an argument so they can be shared