aboutsummaryrefslogtreecommitdiff
path: root/src/cmd/asm
AgeCommit message (Collapse)Author
2017-06-13cmd/internal/obj/arm: fix MOVW to/from FPSRBen Shi
"MOVW FPSR, g" should be assembled to 0xeef1aa10, but actually 0xee30a110 (RFS). "MOVW g, FPSR" should be 0xeee1aa10, but actually 0xee20a110 (WFS). They should be updated to VFP forms, since the ARM back end doesn't support non-VFP floating points. The patch fixes them and adds more assembly encoding tests. fixes #20643 Change-Id: I3b29490337c6e8d891b400fcedc8b0a87b82b527 Reviewed-on: https://go-review.googlesource.com/45276 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-06-09cmd/internal/obj/arm: fix encoding of move register/immediate to CPSRBen Shi
"MOVW R1, CPSR" is assembled to 0xe129f001, which should be 0xe12cf001. "MOVW $255, CPSR" is assembled to 0xe329f0ff, which should be 0xe32cf0ff. This patch fixes them and adds more assembly encoding tests. fix #20626 Change-Id: Iefc945879ea774edf40438ce39f52c144e1501a1 Reviewed-on: https://go-review.googlesource.com/45170 Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-06-05cmd/internal/obj/arm: fix constant decompositionBen Shi
There are two issues in constant decomposition. 1. A typo in "func immrot2s" blocks "case 107" of []optab be triggered. 2. Though "ADD $0xffff, R0, R0" is decomposed to "ADD $0xff00, R0, R0" and "ADD $0x00ff, R0, R0" as expected, "ADD $0xffff, R0" still uses the constant pool, which should be the same as "ADD $0xffff, R0, R0". This patch fixes them and adds more instruction encoding tests. fix #20516 Change-Id: Icd7bdfa1946b29db15580dcb429111266f1384c6 Reviewed-on: https://go-review.googlesource.com/44335 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-05-25cmd/asm/internal/asm: fix a bug in ARM assembly encoding testBen Shi
It is expected to test assembly code for ARMv5, ARMv6 and ARMv7 in cmd/asm/internal/asm/endtoend_test.go. But actually the loop in "func TestARMEndToEnd(t *testing.T)" runs three times all for ARMv5. This patch fixes that bug and adds a new armv6.s which is only tested with GOARM=6. fixes #20465 Change-Id: I5dbf00809a47ace2c195335e2c9bdd768479aada Reviewed-on: https://go-review.googlesource.com/43930 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-05-25cmd/internal/obj/arm: fix illegal forms of ARM VFP instructionBen Shi
"ADDF F0, R1, F2" is silently accepted by the arm assembler and assembled to the same binary code of "ADDF F0, F1, F2". So does "CMPF F0, R1". "ABSF F0, F1, F2" is also silently accepted and assembled to a different instruction. This patch reports those illegal forms and adds test cases. fix #20464 Change-Id: I88b80dc29de24c6266ac7bf7bce1578c5adbc68c Reviewed-on: https://go-review.googlesource.com/43931 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-05-22cmd/internal/obj/arm: report invalid .S/.P/.W suffix in ARM instructionsBen Shi
Many instructions can not have a .S suffix, such as MULS, SWI, CLZ, CMP, STREX and others. And so do .P and .W suffixes. Even wrong assembly code is generated for some instructions with invalid suffixes. This patch tries to simplify .S/.W/.P checks. And a wrong assembly test for arm is added. fixes #20377 Change-Id: Iba1c99d9e6b7b16a749b4d93ca2102e17c5822fe Reviewed-on: https://go-review.googlesource.com/43561 Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-05-18cmd/internal/obj/arm: remove illegal form of the SWI instructionBen Shi
SWI only support "SWI $imm", but currently "SWI (Reg)" is also accepted. This patch fixes it. And more instruction tests are added to cmd/asm/internal/asm/testdata/arm.s fixes #20375 Change-Id: Id437d853924a403e41da9b6cbddd20d994b624ff Reviewed-on: https://go-review.googlesource.com/43552 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-05-16cmd/internal/obj/mips: add support of LLV, SCV, NOOP instructionsCherry Zhang
LLV and SCV are 64-bit load-linked and store-conditional. They were used in runtime as #define WORD. Change them to normal instruction form. NOOP is hardware no-op. It was written as WORD $0. Make a name for it for better disassembly output. Fixes #12561. Fixes #18238. Change-Id: I82c667ce756fa83ef37b034b641e8c4366335e83 Reviewed-on: https://go-review.googlesource.com/40297 Reviewed-by: Minux Ma <minux@golang.org> Run-TryBot: Minux Ma <minux@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-05-09cmd/internal/obj/arm64, cmd/compile: improve offset folding on ARM64Cherry Zhang
ARM64 assembler backend only accepts loads and stores with small or aligned offset. The compiler therefore can only fold small or aligned offsets into loads and stores. For locals and args, their offsets to SP are not known until very late, and the compiler makes conservative decision not folding some of them. However, in most cases, the offset is indeed small or aligned, and can be folded into load and store (but actually not). This CL adds support of loads and stores with large and unaligned offsets. When the offset doesn't fit into the instruction, it uses two instructions and (for very large offset) the constant pool. This way, the compiler doesn't need to be conservative, and can simply fold the offset. To make it work, the assembler's optab matching rules need to be changed. Before, MOVD accepts C_UAUTO32K which matches multiple of 8 between 0 and 32K, and also C_UAUTO16K, which may not be multiple of 8 and does not fit into MOVD instruction. The assembler errors in the latter case. This change makes it only matches multiple of 8 (or offsets within ±256, which also fits in instruction), and uses the large-or-unaligned-offset rule for things doesn't fit (without error). Other sized move rules are changed similarly. Class C_UAUTO64K and C_UOREG64K are removed, as they are never used. In shared library, load/store of global is rewritten to using GOT and temp register, which conflicts with the use of temp register for assembling large offset. So the folding is disabled for globals in shared library mode. Reduce cmd/go binary size by 2%. name old time/op new time/op delta BinaryTree17-8 8.67s ± 0% 8.61s ± 0% -0.60% (p=0.000 n=9+10) Fannkuch11-8 6.24s ± 0% 6.19s ± 0% -0.83% (p=0.000 n=10+9) FmtFprintfEmpty-8 116ns ± 0% 116ns ± 0% ~ (all equal) FmtFprintfString-8 196ns ± 0% 192ns ± 0% -1.89% (p=0.000 n=10+10) FmtFprintfInt-8 199ns ± 0% 198ns ± 0% -0.35% (p=0.001 n=9+10) FmtFprintfIntInt-8 294ns ± 0% 293ns ± 0% -0.34% (p=0.000 n=8+8) FmtFprintfPrefixedInt-8 318ns ± 1% 318ns ± 1% ~ (p=1.000 n=10+10) FmtFprintfFloat-8 537ns ± 0% 531ns ± 0% -1.17% (p=0.000 n=9+10) FmtManyArgs-8 1.19µs ± 1% 1.18µs ± 1% -1.41% (p=0.001 n=10+10) GobDecode-8 17.2ms ± 1% 17.3ms ± 2% ~ (p=0.165 n=10+10) GobEncode-8 14.7ms ± 1% 14.7ms ± 2% ~ (p=0.631 n=10+10) Gzip-8 837ms ± 0% 836ms ± 0% -0.14% (p=0.006 n=9+10) Gunzip-8 141ms ± 0% 139ms ± 0% -1.24% (p=0.000 n=9+10) HTTPClientServer-8 256µs ± 1% 253µs ± 1% -1.35% (p=0.000 n=10+10) JSONEncode-8 40.1ms ± 1% 41.3ms ± 1% +3.06% (p=0.000 n=10+9) JSONDecode-8 157ms ± 1% 156ms ± 1% -0.83% (p=0.001 n=9+8) Mandelbrot200-8 8.94ms ± 0% 8.94ms ± 0% +0.02% (p=0.000 n=9+9) GoParse-8 8.69ms ± 0% 8.54ms ± 1% -1.69% (p=0.000 n=8+10) RegexpMatchEasy0_32-8 227ns ± 1% 228ns ± 1% +0.48% (p=0.016 n=10+9) RegexpMatchEasy0_1K-8 1.92µs ± 0% 1.63µs ± 0% -15.08% (p=0.000 n=10+9) RegexpMatchEasy1_32-8 256ns ± 0% 251ns ± 0% -2.19% (p=0.000 n=10+9) RegexpMatchEasy1_1K-8 2.38µs ± 0% 2.09µs ± 0% -12.49% (p=0.000 n=10+9) RegexpMatchMedium_32-8 352ns ± 0% 354ns ± 0% +0.39% (p=0.002 n=10+9) RegexpMatchMedium_1K-8 106µs ± 0% 106µs ± 0% -0.05% (p=0.005 n=10+9) RegexpMatchHard_32-8 5.92µs ± 0% 5.89µs ± 0% -0.40% (p=0.000 n=9+8) RegexpMatchHard_1K-8 180µs ± 0% 179µs ± 0% -0.14% (p=0.000 n=10+9) Revcomp-8 1.20s ± 0% 1.13s ± 0% -6.29% (p=0.000 n=9+8) Template-8 159ms ± 1% 154ms ± 1% -3.14% (p=0.000 n=9+10) TimeParse-8 800ns ± 3% 769ns ± 1% -3.91% (p=0.000 n=10+10) TimeFormat-8 826ns ± 2% 817ns ± 2% -1.04% (p=0.050 n=10+10) [Geo mean] 145µs 143µs -1.79% Change-Id: I5fc42087cee9b54ea414f8ef6d6d020b80eb5985 Reviewed-on: https://go-review.googlesource.com/42172 Run-TryBot: Cherry Zhang <cherryyz@google.com> Reviewed-by: David Chase <drchase@google.com>
2017-05-07cmd/asm: enable MOVSD in the encoding end-to-end testDamien Lespiau
MOVSD is properly handled but its encoding test wasn't enabled. Enable it. For reference this was found with a little tool I wrote [1] to explore which instructions are missing or not tested in the go obj package and assembler: "which SSE2 instructions aren't tested? And don't list instructions which can take MMX operands" $ x86db-gogen list --extension SSE2 --not-tested --not-mmx CLFLUSH mem [m: np 0f ae /7] WILLAMETTE,SSE2 MOVSD xmmreg,xmmreg [rm: f2 0f 10 /r] WILLAMETTE,SSE2 MOVSD xmmreg,xmmreg [mr: f2 0f 11 /r] WILLAMETTE,SSE2 MOVSD mem64,xmmreg [mr: f2 0f 11 /r] WILLAMETTE,SSE2 MOVSD xmmreg,mem64 [rm: f2 0f 10 /r] WILLAMETTE,SSE2 (CLFLUSH was introduced with SSE2, but has its own CPUID bit) [1] https://github.com/dlespiau/x86db Change-Id: Ic3af3028cb8d4f02e53fdebb9b30fb311f4ee454 Reviewed-on: https://go-review.googlesource.com/42814 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-05-06cmd/asm: fix operand order of ARM's MULA instructionBen Shi
As discussion in issue #19141, the addend should be the third argument of MULA. This patch fixes it in both the front end and the back end of the assembler. And also tests are added to the encoding test. Fixes #19141 Change-Id: Idbc6f338b8fdfcad97a135f27a98c5b375b27d43 Reviewed-on: https://go-review.googlesource.com/42028 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-05-02cmd/{asm,compile}: avoid zeroAuto clobbering flags on s390xMichael Munday
This CL modifies how MOV[DWHB] instructions that store a constant to memory are assembled to avoid them clobbering the condition code (flags). It also modifies zeroAuto to use MOVD instructions instead of CLEAR (which is assembled as XC). MOV[DWHB]storeconst ops also no longer clobbers flags. Note: this CL modifies the assembler so that it can no longer handle immediates outside the range of an int16 or offsets from SB, which reflects what the machine instructions support. The compiler doesn't need this capability any more and I don't think this affects any existing assembly, but it is easy to workaround if it does. Fixes #20187. Change-Id: Ie54947ff38367bd6a19962bf1a6d0296a4accffb Reviewed-on: https://go-review.googlesource.com/42179 Reviewed-by: David Chase <drchase@google.com> Run-TryBot: David Chase <drchase@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-05-01cmd/internal/obj/x86: fix ANDPS encodingDamien Lespiau
ANDPS, like all others PS (Packed Single precision floats) instructions, need Ym: they don't use the 0x66 prefix. From the manual: NP 0F 54 /r ANDPS xmm1, xmm2/m128 NP meaning, quoting the manual: NP - Indicates the use of 66/F2/F3 prefixes (beyond those already part of the instructions opcode) are not allowed with the instruction. And indeed, the same instruction prefixed by 0x66 is ANDPD. Updates #14069 Change-Id: If312a6f1e77113ab8c0febe66bdb1b4171e41e0a Reviewed-on: https://go-review.googlesource.com/42090 Reviewed-by: Keith Randall <khr@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-05-01cmd/asm: enable CMPPS, CMPPD, CMPSS and CMPSD encoding testsDamien Lespiau
The generated test cases had their arguments reversed, putting them back in order makes those tests pass. CMPPS SRC, DEST, CC Change-Id: Ie15021edc533d5681a6a78d10d88b665e3de9017 Reviewed-on: https://go-review.googlesource.com/42097 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-04-27cmd/internal/obj/arm64: fix encoding of conditionWei Xiao
The current code treats condition as special register and write its raw data directly into instruction. The fix converts the raw data into correct condition encoding. Also fix the operand catogery of FCCMP. Add tests to cover all cases. Change-Id: Ib194041bd9017dd0edbc241564fe983082ac616b Reviewed-on: https://go-review.googlesource.com/41511 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-04-26cmd/internal/obj/x86: fix adcb r/mem8,reg8 encodingDamien Lespiau
Taken from the Intel Software Development Manual (of course, in the line below it's ADC DST, SRC; The opposite of the commit subject). 12 /r ADC r8, r/m8 We need 0x12 for the corresponding ytab line, not 0x10. {Ymb, Ynone, Yrb, Zm_r, 1}, Updates #14069 Change-Id: Id37cbd0c581c9988c2de355efa908956278e2189 Reviewed-on: https://go-review.googlesource.com/41857 Reviewed-by: Keith Randall <khr@golang.org>
2017-04-25cmd: fix the order that s390x operands are printed inMichael Munday
The assembler reordered the operands of some instructions to put the first operand into From3. Unfortunately this meant that when the instructions were printed the operands were in a different order than the assembler would expect as input. For example, 'MVC $8, (R1), (R2)' would be printed as 'MVC (R1), $8, (R2)'. Originally this was done to ensure that From contained the source memory operand. The current compiler no longer requires this and so this CL simply makes all instructions use the standard order for operands: From, Reg, From3 and finally To. Fixes #18295 Change-Id: Ib2b5ec29c647ca7a995eb03dc78f82d99618b092 Reviewed-on: https://go-review.googlesource.com/40299 Run-TryBot: Michael Munday <munday@ca.ibm.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-04-20cmd/internal/obj: eliminate LSym.VersionJosh Bleecher Snyder
There were only two versions, 0 and 1, and the only user of version 1 was the assembler, to indicate that a symbol was static. Rename LSym.Version to Static, and add it to LSym.Attributes. Simplify call-sites. Passes toolstash-check. Change-Id: Iabd39918f5019cce78f381d13f0481ae09f3871f Reviewed-on: https://go-review.googlesource.com/41201 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-04-20cmd/compile: add rotates to PPC64.rulesLynn Boger
This updates PPC64.rules to include rules to generate rotates for ADD, OR, XOR operators that combine two opposite shifts that sum to 32 or 64. To support this change opcodes for ROTL and ROTLW were added to be used like the rotldi and rotlwi extended mnemonics. This provides the following improvement in sha3: BenchmarkPermutationFunction-8 302.83 376.40 1.24x BenchmarkSha3_512_MTU-8 98.64 121.92 1.24x BenchmarkSha3_384_MTU-8 136.80 168.30 1.23x BenchmarkSha3_256_MTU-8 169.21 211.29 1.25x BenchmarkSha3_224_MTU-8 179.76 221.19 1.23x BenchmarkShake128_MTU-8 212.87 263.23 1.24x BenchmarkShake256_MTU-8 196.62 245.60 1.25x BenchmarkShake256_16x-8 163.57 194.37 1.19x BenchmarkShake256_1MiB-8 199.02 248.74 1.25x BenchmarkSha3_512_1MiB-8 106.55 133.13 1.25x Fixes #20030 Change-Id: I484c56f48395d32f53ff3ecb3ac6cb8191cfee44 Reviewed-on: https://go-review.googlesource.com/40992 Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com> Reviewed-by: Michael Munday <munday@ca.ibm.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-04-19cmd/internal/objabi: extract shared functionality from objMatthew Dempsky
Now only cmd/asm and cmd/compile depend on cmd/internal/obj. Changing the assembler backends no longer requires reinstalling cmd/link or cmd/addr2line. There's also now one canonical definition of the object file format in cmd/internal/objabi/doc.go, with a warning to update all three implementations. objabi is still something of a grab bag of unrelated code (e.g., flag and environment variable handling probably belong in a separate "tool" package), but this is still progress. Fixes #15165. Fixes #20026. Change-Id: Ic4b92fac7d0d35438e0d20c9579aad4085c5534c Reviewed-on: https://go-review.googlesource.com/40972 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2017-04-18cmd/internal/obj: un-embed FuncInfo field in LSymMatthew Dempsky
Automated refactoring using github.com/mdempsky/unbed (to rewrite s.Foo to s.FuncInfo.Foo) and then gorename (to rename the FuncInfo field to just Func). Passes toolstash-check -all. Change-Id: I802c07a1239e0efea058a91a87c5efe12170083a Reviewed-on: https://go-review.googlesource.com/40670 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2017-04-17cmd/asm: detect invalid DS form offsets for ppc64xLynn Boger
While debugging a recent regression it was discovered that the assembler for ppc64x was not always generating the correct instruction for DS form loads and stores. When an instruction is DS form then the offset must be a multiple of 4, and if it isn't then bits outside the offset field were being incorrectly set resulting in unexpected and incorrect instructions. This change adds a check to determine when the opcode is DS form and then verifies that the offset is a multiple of 4 before generating the instruction, otherwise logs an error. This also changes a few asm files that were using unaligned offsets for DS form loads and stores. In the runtime package these were instructions intended to cause a crash so using aligned or unaligned offsets doesn't change that behavior. Change-Id: Ie3a7e1e65dcc9933b54de7a46a054da8459cb56f Reviewed-on: https://go-review.googlesource.com/40476 Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
2017-04-17cmd/asm, cmd/internal/obj/s390x, math: add LGDR and LDGR instructionsMichael Munday
The instructions allow moves between floating point and general purpose registers without any conversion taking place. Change-Id: I82c6f3ad9c841a83783b5be80dcf5cd538ff49e6 Reviewed-on: https://go-review.googlesource.com/38777 Run-TryBot: Michael Munday <munday@ca.ibm.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-04-12cmd/compile: move Text.From.Sym initialization earlierJosh Bleecher Snyder
The initialization of an ATEXT Prog's From.Sym can race with the assemblers in a concurrent compiler. CL 40254 contains an initial, failed attempt to fix that race. This CL takes a different approach: Rather than expose an API to initialize the Prog, expose an API to initialize the Sym. The initialization of the Sym can then be moved earlier in the compiler, avoiding the race. The growth of gc.Func has negligible performance impact; see below. Passes toolstash -cmp. Updates #15756 name old alloc/op new alloc/op delta Template 38.8MB ± 0% 38.8MB ± 0% ~ (p=0.968 n=9+10) Unicode 29.8MB ± 0% 29.8MB ± 0% ~ (p=0.684 n=10+10) GoTypes 113MB ± 0% 113MB ± 0% ~ (p=0.912 n=10+10) SSA 1.25GB ± 0% 1.25GB ± 0% ~ (p=0.481 n=10+10) Flate 25.3MB ± 0% 25.3MB ± 0% ~ (p=0.105 n=10+10) GoParser 31.7MB ± 0% 31.8MB ± 0% +0.09% (p=0.016 n=8+10) Reflect 78.3MB ± 0% 78.2MB ± 0% ~ (p=0.190 n=10+10) Tar 26.5MB ± 0% 26.6MB ± 0% +0.13% (p=0.011 n=10+10) XML 42.4MB ± 0% 42.4MB ± 0% ~ (p=0.971 n=10+10) name old allocs/op new allocs/op delta Template 378k ± 1% 378k ± 0% ~ (p=0.315 n=10+9) Unicode 321k ± 1% 321k ± 0% ~ (p=0.436 n=10+10) GoTypes 1.14M ± 0% 1.14M ± 0% ~ (p=0.079 n=10+9) SSA 9.70M ± 0% 9.70M ± 0% -0.04% (p=0.035 n=10+10) Flate 233k ± 1% 234k ± 1% ~ (p=0.529 n=10+10) GoParser 315k ± 0% 316k ± 0% ~ (p=0.095 n=9+10) Reflect 980k ± 0% 980k ± 0% ~ (p=0.436 n=10+10) Tar 249k ± 1% 250k ± 0% ~ (p=0.280 n=10+10) XML 391k ± 1% 391k ± 1% ~ (p=0.481 n=10+10) Change-Id: I3c93033dddd2e1df8cc54a106a6e615d27859e71 Reviewed-on: https://go-review.googlesource.com/40496 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-04-12cmd/internal/obj: stop storing Text flags in From3Josh Bleecher Snyder
Prior to this CL, flags such as NOSPLIT on ATEXT Progs were stored in From3.Offset. Some but not all of those flags were also duplicated into From.Sym.Attribute. This CL migrates all of those flags into From.Sym.Attribute and stops creating a From3. A side-effect of this is that printing an ATEXT Prog can no longer simply dump From3.Offset. That's kind of good, since the raw flag value wasn't very informative anyway, but it did necessitate a bunch of updates to the cmd/asm tests. The reason I'm doing this work now is that avoiding storing flags in both From.Sym and From3.Offset simplifies some other changes to fix the data race first described in CL 40254. This CL almost passes toolstash-check -all. The only changes are in cases where the assembler has decided that a function's flags may be altered, e.g. to make a function with no calls in it NOSPLIT. Prior to this CL, that information was not printed. Sample before: "".Ctz64 t=1 size=63 args=0x10 locals=0x0 0x0000 00000 (/Users/josh/go/tip/src/runtime/internal/sys/intrinsics.go:35) TEXT "".Ctz64(SB), $0-16 0x0000 00000 (/Users/josh/go/tip/src/runtime/internal/sys/intrinsics.go:35) FUNCDATA $0, gclocals·f207267fbf96a0178e8758c6e3e0ce28(SB) Sample after: "".Ctz64 t=1 nosplit size=63 args=0x10 locals=0x0 0x0000 00000 (/Users/josh/go/tip/src/runtime/internal/sys/intrinsics.go:35) TEXT "".Ctz64(SB), NOSPLIT, $0-16 0x0000 00000 (/Users/josh/go/tip/src/runtime/internal/sys/intrinsics.go:35) FUNCDATA $0, gclocals·f207267fbf96a0178e8758c6e3e0ce28(SB) Observe the additional "nosplit" in the first line and the additional "NOSPLIT" in the second line. Updates #15756 Change-Id: I5c59bd8f3bdc7c780361f801d94a261f0aef3d13 Reviewed-on: https://go-review.googlesource.com/40495 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-04-11runtime: use hardware divider to improve performanceBen Shi
The hardware divider is an optional component of ARMv7. This patch detects whether it is available in runtime and use it or not. 1. The hardware divider is detected at startup and a flag is set/clear according to a perticular bit of runtime.hwcap. 2. Each call of runtime.udiv will check this flag and decide if use the hardware division instruction. A rough test shows the performance improves 40-50% for ARMv7. And the compatibility of ARMv5/v6 is not broken. fixes #19118 Change-Id: Ic586bc9659ebc169553ca2004d2bdb721df823ac Reviewed-on: https://go-review.googlesource.com/37496 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-04-11cmd/internal/obj: refactor ATEXT symbol initializationJosh Bleecher Snyder
This makes the core Flushplist loop clearer. We may also want to move the Sym initialization much earlier in the compiler (see discussion on CL 40254), for which this paves the way. While we're here, eliminate package log in favor of ctxt.Diag. Passes toolstash-check -all. Updates #15756 Change-Id: Ieaf848d196764a5aa82578b689af7bc6638c385a Reviewed-on: https://go-review.googlesource.com/40313 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
2017-04-07cmd/internal/obj: eagerly initialize assemblersJosh Bleecher Snyder
CL 38662 changed the x86 assembler to be eagerly initialized, for a concurrent backend. This CL puts in place a proper mechanism for doing so, and switches all architectures to use it. Passes toolstash-check -all. Updates #15756 Change-Id: Id2aa527d3a8259c95797d63a2f0d1123e3ca2a1c Reviewed-on: https://go-review.googlesource.com/39917 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-04-06cmd/internal/obj: remove LinklookupJosh Bleecher Snyder
It was simply a wrapper around Link.Lookup. Unwrap everything. CL prepared using eg with template: package p import "cmd/internal/obj" func before(ctxt *obj.Link, name string, version int) *obj.LSym { return obj.Linklookup(ctxt, name, version) } func after(ctxt *obj.Link, name string, version int) *obj.LSym { return ctxt.Lookup(name, version) } Then one comment in cmd/asm/internal/asm/parse.go was manually updated (and gofmt'ed!), and func Linklookup deleted. Passes toolstash-check (as a sanity measure). Change-Id: Icc4d56b0b2b5c8888d3184c1898c48359ea1e638 Reviewed-on: https://go-review.googlesource.com/39715 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-04-06cmd/internal/obj/arm64: fix encoding of AND MBCONCherry Zhang
When a constant is both MOVCON (can fit into a MOV instruction) and BITCON (can fit into a logical instruction), the assembler chooses to use the MOVCON encoding, which is actually longer for logical instructions. We add MBCON rules explicitly to make sure it uses the BITCON encoding. Updates #19857. Change-Id: Ib9881be363cbc491ac2a0792b36b87e74eff34a8 Reviewed-on: https://go-review.googlesource.com/39652 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
2017-04-06cmd/compile: add Prog cache to ProgsJosh Bleecher Snyder
The existing bulk/cached Prog allocator, Ctxt.NewProg, is not concurrency-safe. This CL moves Prog allocation to its clients, the compiler and the assembler. The assembler is so fast and generates so few Progs that it does not need optimization of Prog allocation. I could not generate measureable changes. And even if I could, the assembly is a miniscule portion of build times. The compiler already has a natural place to manage Prog allocation; this CL migrates the Prog cache there. It will be made concurrency-safe in a later CL by partitioning the Prog cache into chunks and assigning each chunk to a different goroutine to manage. This CL does cause a performance degradation when the compiler is invoked with the -S flag (to dump assembly). However, such usage is rare and almost always done manually. The one instance I know of in a test is TestAssembly in cmd/compile/internal/gc, and I did not detect a measurable performance impact there. Passes toolstash-check -all. Minor compiler performance impact. Updates #15756 Performance impact from just this CL: name old time/op new time/op delta Template 213ms ± 4% 213ms ± 4% ~ (p=0.571 n=49+49) Unicode 89.1ms ± 3% 89.4ms ± 3% ~ (p=0.388 n=47+48) GoTypes 581ms ± 2% 584ms ± 3% +0.56% (p=0.019 n=47+48) SSA 6.48s ± 2% 6.53s ± 2% +0.84% (p=0.000 n=47+49) Flate 128ms ± 4% 128ms ± 4% ~ (p=0.832 n=49+49) GoParser 152ms ± 3% 152ms ± 3% ~ (p=0.815 n=48+47) Reflect 371ms ± 4% 371ms ± 3% ~ (p=0.617 n=50+47) Tar 112ms ± 4% 112ms ± 3% ~ (p=0.724 n=49+49) XML 208ms ± 3% 208ms ± 4% ~ (p=0.678 n=49+50) [Geo mean] 284ms 285ms +0.18% name old user-ns/op new user-ns/op delta Template 251M ± 7% 252M ±11% ~ (p=0.704 n=49+50) Unicode 107M ± 7% 108M ± 5% +1.25% (p=0.036 n=50+49) GoTypes 738M ± 3% 740M ± 3% ~ (p=0.305 n=49+48) SSA 8.83G ± 2% 8.86G ± 4% ~ (p=0.098 n=47+50) Flate 146M ± 6% 147M ± 3% ~ (p=0.584 n=48+41) GoParser 178M ± 6% 179M ± 5% +0.93% (p=0.036 n=49+48) Reflect 441M ± 4% 446M ± 7% ~ (p=0.218 n=44+49) Tar 126M ± 5% 126M ± 5% ~ (p=0.766 n=48+49) XML 245M ± 5% 244M ± 4% ~ (p=0.359 n=50+50) [Geo mean] 341M 342M +0.51% Performance impact from this CL combined with its parent: name old time/op new time/op delta Template 213ms ± 3% 214ms ± 4% ~ (p=0.685 n=47+50) Unicode 89.8ms ± 6% 90.5ms ± 6% ~ (p=0.055 n=50+50) GoTypes 584ms ± 3% 585ms ± 2% ~ (p=0.710 n=49+47) SSA 6.50s ± 2% 6.53s ± 2% +0.39% (p=0.011 n=46+50) Flate 128ms ± 3% 128ms ± 4% ~ (p=0.855 n=47+49) GoParser 152ms ± 3% 152ms ± 3% ~ (p=0.666 n=49+49) Reflect 371ms ± 3% 372ms ± 3% ~ (p=0.298 n=48+48) Tar 112ms ± 5% 113ms ± 3% ~ (p=0.107 n=49+49) XML 208ms ± 3% 208ms ± 2% ~ (p=0.881 n=50+49) [Geo mean] 285ms 285ms +0.26% name old user-ns/op new user-ns/op delta Template 254M ± 9% 252M ± 8% ~ (p=0.290 n=49+50) Unicode 106M ± 6% 108M ± 7% +1.44% (p=0.034 n=50+50) GoTypes 741M ± 4% 743M ± 4% ~ (p=0.992 n=50+49) SSA 8.86G ± 2% 8.83G ± 3% ~ (p=0.158 n=47+49) Flate 147M ± 4% 148M ± 5% ~ (p=0.832 n=50+49) GoParser 179M ± 5% 178M ± 5% ~ (p=0.370 n=48+50) Reflect 441M ± 6% 445M ± 7% ~ (p=0.246 n=45+47) Tar 126M ± 6% 126M ± 6% ~ (p=0.815 n=49+50) XML 244M ± 3% 245M ± 4% ~ (p=0.190 n=50+50) [Geo mean] 342M 342M +0.17% Change-Id: I020f1c079d495fbe2e15ccb51e1ea2cc1b5a1855 Reviewed-on: https://go-review.googlesource.com/39634 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
2017-03-31cmd/asm/internal/arch: use generic obj.Rconv function everywhereDave Cheney
Rather than using arm64.Rconv directly in the archArm64 constructor use the generic obj.Rconv helper. This removes the only use of arm64.Rconv outside the arm64 package itself. Change-Id: I99e9e7156b52cd26dc134f610f764ec794264e2c Reviewed-on: https://go-review.googlesource.com/38756 Run-TryBot: Dave Cheney <dave@cheney.net> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2017-03-30cmd/internal/obj/arm: support more ARMv5/ARMv6/ARMv7 instructionsBen Shi
REV/REV16/REVSH were introduced in ARMv6, they offered more efficient byte reverse operatons. MMUL/MMULA/MMULS were introduced in ARMv6, they simplified a serial of mul->shift->add/sub operations into a single instruction. RBIT was introduced in ARMv7, it inversed a 32-bit word's bit order. MULS was introduced in ARMv7, it corresponded to MULA. MULBB/MULABB were introduced in ARMv5TE, they performed 16-bit multiplication (and accumulation). Change-Id: I6365b17b3c4eaf382a657c210bb0094b423b11b8 Reviewed-on: https://go-review.googlesource.com/35565 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-03-27cmd/asm: add support to shift operands on arm64wei xiao
Fixes: #18070 Also added a test in: cmd/asm/internal/asm/testdata/arm64.s Change-Id: Icc43ff7383cc06b8eaccabd9ff0aefa61c4ecb88 Reviewed-on: https://go-review.googlesource.com/33595 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-03-25cmd/internal/obj: eagerly initialize x86 assemblerJosh Bleecher Snyder
Prior to this CL, instinit was called as needed. This does not work well in a concurrent backend. Initialization is very cheap; do it on startup instead. Passes toolstash-check -all. No compiler performance impact. Updates #15756 Change-Id: Ifa5e82e8abf4504435e1b28766f5703a0555f42d Reviewed-on: https://go-review.googlesource.com/38662 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-03-24cmd/asm: fix TBZ/TBNZ instructions on arm64wei xiao
Fixes #18069 Also added a test in: cmd/asm/internal/asm/testdata/arm64.s Change-Id: Iee400bda4f30503ea3c1dc5bb8301568f19c92d1 Signed-off-by: Wei Xiao <wei.xiao@arm.com> Reviewed-on: https://go-review.googlesource.com/33594 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-03-23cmd/internal/obj/arm: Fix wrong assembly in the arm assemblerBen Shi
As #19357 reported, TST has 3 different sub forms TST $imme, Rx TST Ry << imme, Rx TST Ry << Rz, Rx just like CMP/CMN/TEQ has. But current arm assembler assembles all TST instructions wrongly. This patch fixes it and adds more tests. Fixes #19357 Change-Id: Iafedccfaab2cbb2631e7acf259837a782e2e8e2f Reviewed-on: https://go-review.googlesource.com/37662 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-03-20cmd/internal/obj: convert Debug* Link fields into boolsJosh Bleecher Snyder
Change-Id: I9ac274dbfe887675a7820d2f8f87b5887b1c9b0e Reviewed-on: https://go-review.googlesource.com/38383 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-03-01cmd/compile, cmd/asm: remove Link.PlistsHeschi Kreinick
Link.Plists never contained more than one Plist, and sometimes none. Passing around the Plist being worked on is straightforward and makes the data flow easier to follow. Change-Id: I79cb30cb2bd3d319fdbb1dfa5d35b27fcb748e5c Reviewed-on: https://go-review.googlesource.com/37169 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-02-10cmd/{asm,internal/obj/s390x}, math: remove emulated float instructionsMichael Munday
The s390x port was based on the ppc64 port and, because of the way the port was done, inherited some instructions from it. ppc64 supports 3-operand (4-operand for FMADD etc.) floating point instructions but s390x doesn't (the destination register is always an input) and so these were emulated. There is a bug in the emulation of FMADD whereby if the destination register is also a source for the multiplication it will be clobbered. This doesn't break any assembly code in the std lib but could affect future work. To fix this I have gone through the floating point instructions and removed all unnecessary 3-/4-operand emulation. The compiler doesn't need it and assembly writers don't need it, it's just a source of bugs. I've also deleted the FNMADD family of emulated instructions. They aren't used anywhere. Change-Id: Ic07cedcf141a6a3b43a0c84895460f6cfbf56c04 Reviewed-on: https://go-review.googlesource.com/33350 Run-TryBot: Michael Munday <munday@ca.ibm.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-02-09cmd/asm, cmd/internal/obj/ppc64: Add ISA 2.05, 2.06 and 2.07 instructions.Carlos Eduardo Seo
This change adds instructions from ISA 2.05, 2.06 and 2.07 that are frequently used in assembly optimizations for ppc64. It also fixes two problems: * the implementation of RLDICR[CC]/RLDICL[CC] did not consider all possible cases for the bit mask. * removed two non-existing instructions that were added by mistake in the VMX implementation (VORL/VANDL). Change-Id: Iaef4e5c6a5240c2156c6c0f28ad3bcd8780e9830 Reviewed-on: https://go-review.googlesource.com/36230 Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
2017-02-01cmd/asm, cmd/internal/obj/s390x: fix encoding of VREPI{H,F,G}Michael Munday
Also adds tests for all missing VRI-a instructions (which may be affected by this change). Fixes #18749. Change-Id: I48249dda626f32555da9ab58659e2e140de6504a Reviewed-on: https://go-review.googlesource.com/35561 Run-TryBot: Michael Munday <munday@ca.ibm.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
2017-01-09[dev.inline] cmd/internal/src: introduce compact source position representationRobert Griesemer
XPos is a compact (8 instead of 16 bytes on a 64bit machine) source position representation. There is a 1:1 correspondence between each XPos and each regular Pos, translated via a global table. In some sense this brings back the LineHist, though positions can track line and column information; there is a O(1) translation between the representations (no binary search), and the translation is factored out. The size increase with the prior change is brought down again and the compiler speed is in line with the master repo (measured on the same "quiet" machine as for prior change): name old time/op new time/op delta Template 256ms ± 1% 262ms ± 2% ~ (p=0.063 n=5+4) Unicode 132ms ± 1% 135ms ± 2% ~ (p=0.063 n=5+4) GoTypes 891ms ± 1% 871ms ± 1% -2.28% (p=0.016 n=5+4) Compiler 3.84s ± 2% 3.89s ± 2% ~ (p=0.413 n=5+4) MakeBash 47.1s ± 1% 46.2s ± 2% ~ (p=0.095 n=5+5) name old user-ns/op new user-ns/op delta Template 309M ± 1% 314M ± 2% ~ (p=0.111 n=5+4) Unicode 165M ± 1% 172M ± 9% ~ (p=0.151 n=5+5) GoTypes 1.14G ± 2% 1.12G ± 1% ~ (p=0.063 n=5+4) Compiler 5.00G ± 1% 4.96G ± 1% ~ (p=0.286 n=5+4) Change-Id: Icc570cc60ab014d8d9af6976f1f961ab8828cc47 Reviewed-on: https://go-review.googlesource.com/34506 Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-01-09[dev.inline] cmd/internal/src: replace src.Pos with syntax.PosRobert Griesemer
This replaces the src.Pos LineHist-based position tracking with the syntax.Pos implementation and updates all uses. The LineHist table is not used anymore - the respective code is still there but should be removed eventually. CL forthcoming. Passes toolstash -cmp when comparing to the master repo (with the exception of a couple of swapped assembly instructions, likely due to different instruction scheduling because the line-based sorting has changed; though this is won't affect correctness). The sizes of various important compiler data structures have increased significantly (see the various sizes_test.go files); this is probably the reason for an increase of compilation times (to be addressed). Here are the results of compilebench -count 5, run on a "quiet" machine (no apps running besides a terminal): name old time/op new time/op delta Template 256ms ± 1% 280ms ±15% +9.54% (p=0.008 n=5+5) Unicode 132ms ± 1% 132ms ± 1% ~ (p=0.690 n=5+5) GoTypes 891ms ± 1% 917ms ± 2% +2.88% (p=0.008 n=5+5) Compiler 3.84s ± 2% 3.99s ± 2% +3.95% (p=0.016 n=5+5) MakeBash 47.1s ± 1% 47.2s ± 2% ~ (p=0.841 n=5+5) name old user-ns/op new user-ns/op delta Template 309M ± 1% 326M ± 2% +5.18% (p=0.008 n=5+5) Unicode 165M ± 1% 168M ± 4% ~ (p=0.421 n=5+5) GoTypes 1.14G ± 2% 1.18G ± 1% +3.47% (p=0.008 n=5+5) Compiler 5.00G ± 1% 5.16G ± 1% +3.12% (p=0.008 n=5+5) Change-Id: I241c4246cdff627d7ecb95cac23060b38f9775ec Reviewed-on: https://go-review.googlesource.com/34273 Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2016-12-09[dev.inline] cmd/internal/obj: rename Prog.Lineno to Prog.PosDavid Lazar
Change-Id: I7585d85907869f5a286b36936dfd035f1e8e9906 Reviewed-on: https://go-review.googlesource.com/34197 Run-TryBot: David Lazar <lazard@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
2016-12-09[dev.inline] cmd/internal/obj: use src.Pos in obj.ProgDavid Lazar
This will let us use the src.Pos struct to thread inlining information through to obj. Change-Id: I96a16d3531167396988df66ae70f0b729049cc82 Reviewed-on: https://go-review.googlesource.com/34195 Run-TryBot: David Lazar <lazard@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org> Reviewed-by: Austin Clements <austin@google.com>
2016-11-28cmd/asm: fix parsing of the s390x instructions VSTE{G,F,H,B}Michael Munday
The element index needs to be placed in From3. Before this CL it was impossible to write a VSTE instruction that could be successfully parsed, so this won't affect existing assembly code. Fixes #18075. Change-Id: I5b71be4c6632b1d5a30820a529122f96fd1bc864 Reviewed-on: https://go-review.googlesource.com/33584 Run-TryBot: Michael Munday <munday@ca.ibm.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bill O'Farrell <billotosyr@gmail.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2016-11-17cmd/asm/internal/asm: fix copy/paste errors in commentMichael Munday
Change-Id: I0249b60e340710bea7b6671c9b7405c278b037bd Reviewed-on: https://go-review.googlesource.com/33351 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-11-08cmd/asm: add support for GOARCH=mips{,le}Vladimir Stefanovic
Change-Id: I6a5256a42f895bb93ac56764e91ade1861c00e04 Reviewed-on: https://go-review.googlesource.com/31476 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
2016-11-08cmd/internal/obj/mips: add support for GOARCH=mips{,le}Vladimir Stefanovic
Implements subset of MIPS32(r1) instruction set. Change-Id: Iba017350f6c2763de05d4d1bc2f123e8eb76d0ff Reviewed-on: https://go-review.googlesource.com/31475 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>