The liveness computation of parameters generally was never
correct, but forcing all parameters to be live throughout the
function covered up that problem. The new SSA back end is
too clever: even though it currently keeps the parameter values live
throughout the function, it may find optimizations that mean
the current values are not written back to the original parameter
stack slots immediately or ever (for example if a parameter is set
to nil, SSA constant propagation may replace all later uses of the
parameter with a constant nil, eliminating the need to write the nil
value back to the stack slot), so the liveness code must now
track the actual operations on the stack slots, exposing these
problems.
One small problem in the handling of arguments is that nodarg
can return ONAME PPARAM nodes with adjusted offsets, so that
there are actually multiple *Node pointers for the same parameter
in the instruction stream. This might be possible to correct, but
not in this CL. For now, we fix this by using n.Orig instead of n
when considering PPARAM and PPARAMOUT nodes.
The major problem in the handling of arguments is general
confusion in the liveness code about the meaning of PPARAM|PHEAP
and PPARAMOUT|PHEAP nodes, especially as contrasted with PAUTO|PHEAP.
The difference between these two is that when a local variable "moves"
to the heap, it's really just allocated there to start with; in contrast,
when an argument moves to the heap, the actual data has to be copied
there from the stack at the beginning of the function, and when a
result "moves" to the heap the value in the heap has to be copied
back to the stack when the function returns
This general confusion is also present in the SSA back end.
The PHEAP bit worked decently when I first introduced it 7 years ago (!)
in 391425ae. The back end did nothing sophisticated, and in particular
there was no analysis at all: no escape analysis, no liveness analysis,
and certainly no SSA back end. But the complications caused in the
various downstream consumers suggest that this should be a detail
kept mainly in the front end.
This CL therefore eliminates both the PHEAP bit and even the idea of
"heap variables" from the back ends.
First, it replaces the PPARAM|PHEAP, PPARAMOUT|PHEAP, and PAUTO|PHEAP
variable classes with the single PAUTOHEAP, a pseudo-class indicating
a variable maintained on the heap and available by indirecting a
local variable kept on the stack (a plain PAUTO).
Second, walkexpr replaces all references to PAUTOHEAP variables
with indirections of the corresponding PAUTO variable.
The back ends and the liveness code now just see plain indirected
variables. This may actually produce better code, but the real goal
here is to eliminate these little-used and somewhat suspect code
paths in the back end analyses.
The OPARAM node type goes away too.
A followup CL will do the same to PPARAMREF. I'm not sure that
the back ends (SSA in particular) are handling those right either,
and with the framework established in this CL that change is trivial
and the result clearly more correct.
Fixes#15747.
Change-Id: I2770b1ce3cbc93981bfc7166be66a9da12013d74
Reviewed-on: https://go-review.googlesource.com/23393
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The problem was introduced by the recent filepath.Join change.
Fixes#14949
Change-Id: I7ee52f210e12bbb1369e308e584ddb2c7766e095
Reviewed-on: https://go-review.googlesource.com/23240
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The cgo tool generates compiler errors to find out what kind of name it
is using. Turning on optimization can confuse that process by producing
new unexpected messages.
Fixes#14669.
Change-Id: Idc8e35fd259711ecc9638566b691c11d17140325
Reviewed-on: https://go-review.googlesource.com/23231
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Add a test which compiles a function and checks the
generated assembly to make sure certain patterns are present.
This test allows us to do white box tests of the compiler
to make sure optimizations don't regress.
Added a few simple tests for now. More to come.
Change-Id: I4ab5ce5d95b9e04e7d0d9328ffae47b8d1f95e74
Reviewed-on: https://go-review.googlesource.com/23403
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Decoding a JSON message does not touch unspecified or null fields;
always use a new underlying struct to prevent old field values from
sticking around.
Fixes: #14640
Change-Id: Ica78c208ce104e2cdee1d4e92bf58596ea5587c8
Reviewed-on: https://go-review.googlesource.com/23483
Reviewed-by: Andrew Gerrand <adg@golang.org>
When rules are generated with -log, log rule application to a file.
The file is opened in append mode so multiple calls to the compiler
union their logs.
Change-Id: Ib35c7c85bf58e5909ea9231043f8cbaa6bf278b7
Reviewed-on: https://go-review.googlesource.com/23406
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
domorder has some non-obvious useful properties
that we’re relying on in cse.
Document them and provide an argument that they hold.
While we’re here, do some minor renaming.
The argument is a re-working of a private email
exchange with Todd Neal and David Chase.
Change-Id: Ie154e0521bde642f5f11e67fc542c5eb938258be
Reviewed-on: https://go-review.googlesource.com/23449
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
I'm glad my CL fixed the library use case inside Google.
It fixes neither of the two tests here.
Change-Id: Ica91722dced8955a0a8ba3aad3d288816b46564e
Reviewed-on: https://go-review.googlesource.com/23482
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
This has a minor performance cost, but far less than is being gained by SSA.
As an experiment, enable it during the Go 1.7 beta.
Having frame pointers on by default makes Linux's perf, Intel VTune,
and other profilers much more useful, because it lets them gather a
stack trace efficiently on profiling events.
(It doesn't help us that much, since when we walk the stack we usually
need to look up PC-specific information as well.)
Fixes#15840.
Change-Id: I4efd38412a0de4a9c87b1b6e5d11c301e63f1a2a
Reviewed-on: https://go-review.googlesource.com/23451
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It is timing out on the dashboard.
(We enabled it as an experiment to see if it was still broken. Looks that way.)
Change-Id: I425b7e54a2ab95b623ab7a15554b4173078f75e2
Reviewed-on: https://go-review.googlesource.com/23480
Reviewed-by: Russ Cox <rsc@golang.org>
The irregular calling convention for defers currently incorrectly
manages the BP if frame pointers are enabled. Specifically, jmpdefer
manipulates the SP as if its own caller, deferreturn, had returned.
However, it does not manipulate the BP to match. As a result, when a
BP-based traceback happens during a deferred function call, it unwinds
to the function that performed the defer and then thinks that function
called itself in an infinite regress.
Fix this by making jmpdefer manipulate the BP as if deferreturn had
actually returned.
Fixes#12968.
Updates #15840.
Change-Id: Ic9cc7c863baeaf977883ed0c25a7e80e592cf066
Reviewed-on: https://go-review.googlesource.com/23457
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The offsets computed by the DWARF expressions for local variables
currently don't account for the extra stack slot used by the frame
pointer when GOEXPERIMENT=framepointer is enabled.
Fix this by adding the extra stack slot to the offset.
This fixes TestGdbPython with GOEXPERIMENT=framepointer.
Updates #15840.
Change-Id: I1b2ebb2750cd22266f4a89ec8d9e8bfa05fabd19
Reviewed-on: https://go-review.googlesource.com/23458
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
A few other architectures have already defined a NOFRAME flag.
Use it to disable frame pointer code on a few very low-level functions
that must behave like Windows code.
Makes the failing os/signal test pass on a Windows gomote.
Change-Id: I982365f2c59a0aa302b4428c970846c61027cf3e
Reviewed-on: https://go-review.googlesource.com/23456
Reviewed-by: Austin Clements <austin@google.com>
AVX2 variant reads next blocks while calculating current block.
Avoid reading past the end of data, by switching back to original,
for last blocks.
Fixes#15617.
Change-Id: I04fa2d83f1b47995117c77b4a3d403a7dff594d4
Reviewed-on: https://go-review.googlesource.com/23138
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I have been running this patch inside Google against Go 1.6 for the last month.
The new tests will probably break the builders but let's see
exactly how they break.
Change-Id: Ia65cf7d3faecffeeb4b06e9b80875c0e57d86d9e
Reviewed-on: https://go-review.googlesource.com/23452
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The importer had several bugs with respect to labels and gotos:
- it didn't create a new ONAME node for label names (label dcl,
goto, continue, and break)
- it overwrote the symbol for gotos with the dclstack
- it didn't set the dclstack for labels
In the process changed export format slightly to always assume
a label name for labels and gotos, and never assume a label for
fallthroughs.
For fallthroughs and switch cases, now also set Xoffset like in
the parser. (Not setting it, i.e., using 0 was ok since this is
only used for verifying correct use of fallthroughs, which was
checked already. But it's an extra level of verification of the
import.)
Fixes#15838.
Change-Id: I3637f6314b8651c918df0c8cd70cd858c92bd483
Reviewed-on: https://go-review.googlesource.com/23445
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Acquire and release the TSAN synchronization point when calling malloc,
just as we do when calling any other C function. If we don't do this,
TSAN will report false positive errors about races calling malloc and
free.
We used to have a special code path for malloc and free, going through
the runtime functions cmalloc and cfree. The special code path for cfree
was no longer used even before this CL. This CL stops using the special
code path for malloc, because there is no place along that path where we
could conditionally insert the TSAN synchronization. This CL removes
the support for the special code path for both functions.
Instead, cgo now automatically generates the malloc function as though
it were referenced as C.malloc. We need to automatically generate it
even if C.malloc is not called, even if malloc and size_t are not
declared, to support cgo-provided functions like C.CString.
Change-Id: I829854ec0787a80f33fa0a8a0dc2ee1d617830e2
Reviewed-on: https://go-review.googlesource.com/23260
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This makes GOEXPERIMENT=framepointer, GOOS=darwin, and buildmode=carchive coexist.
Change-Id: I9f6fb2f0f06f27df683e5b51f2fa55cd21872453
Reviewed-on: https://go-review.googlesource.com/23454
Reviewed-by: Austin Clements <austin@google.com>
Currently scanstack obtains its own gcWork from the P for the duration
of the stack scan and then, if called during mark termination,
disposes the gcWork.
However, this means that the number of workbufs allocated will be at
least the number of stacks scanned during mark termination, which may
be very high (especially during a STW GC). This happens because, in
steady state, each scanstack will obtain a fresh workbuf (either from
the empty list or by allocating it), fill it with the scan results,
and then dispose it to the full list. Nothing is consuming from the
full list during this (and hence nothing is recycling them to the
empty list), so the length of the full list by the time mark
termination starts draining it is at least the number of stacks
scanned.
Fix this by pushing the gcWork acquisition up the stack to either the
gcDrain that calls markroot that calls scanstack (which batches across
many stack scans and is the path taken during STW GC) or to newstack
(which is still a single scanstack call, but this is roughly bounded
by the number of Ps).
This fix reduces the workbuf allocation for the test program from
issue #15319 from 213 MB (roughly 2KB * 1e5 goroutines) to 10 MB.
Fixes#15319.
Note that there's potentially a similar issue in write barriers during
mark 2. Fixing that will be more difficult since there's no broader
non-preemptible context, but it should also be less of a problem since
the full list is being drained during mark 2.
Some overall improvements in the go1 benchmarks, plus the usual noise.
No significant change in the garbage benchmark (time/op or GC memory).
name old time/op new time/op delta
BinaryTree17-12 2.54s ± 1% 2.51s ± 1% -1.09% (p=0.000 n=20+19)
Fannkuch11-12 2.12s ± 0% 2.17s ± 0% +2.18% (p=0.000 n=19+18)
FmtFprintfEmpty-12 45.1ns ± 1% 45.2ns ± 0% ~ (p=0.078 n=19+18)
FmtFprintfString-12 127ns ± 0% 128ns ± 0% +1.08% (p=0.000 n=19+16)
FmtFprintfInt-12 125ns ± 0% 122ns ± 1% -2.71% (p=0.000 n=14+18)
FmtFprintfIntInt-12 196ns ± 0% 190ns ± 1% -2.91% (p=0.000 n=12+20)
FmtFprintfPrefixedInt-12 196ns ± 0% 194ns ± 1% -0.94% (p=0.000 n=13+18)
FmtFprintfFloat-12 253ns ± 1% 251ns ± 1% -0.86% (p=0.000 n=19+20)
FmtManyArgs-12 807ns ± 1% 784ns ± 1% -2.85% (p=0.000 n=20+20)
GobDecode-12 7.13ms ± 1% 7.12ms ± 1% ~ (p=0.351 n=19+20)
GobEncode-12 5.89ms ± 0% 5.95ms ± 0% +0.94% (p=0.000 n=19+19)
Gzip-12 219ms ± 1% 221ms ± 1% +1.35% (p=0.000 n=18+20)
Gunzip-12 37.5ms ± 1% 37.4ms ± 0% ~ (p=0.057 n=20+19)
HTTPClientServer-12 81.4µs ± 4% 81.9µs ± 3% ~ (p=0.118 n=17+18)
JSONEncode-12 15.7ms ± 1% 15.8ms ± 1% +0.73% (p=0.000 n=17+18)
JSONDecode-12 57.9ms ± 1% 57.2ms ± 1% -1.34% (p=0.000 n=19+19)
Mandelbrot200-12 4.12ms ± 1% 4.10ms ± 0% -0.33% (p=0.000 n=19+17)
GoParse-12 3.22ms ± 2% 3.25ms ± 1% +0.72% (p=0.000 n=18+20)
RegexpMatchEasy0_32-12 70.6ns ± 1% 71.1ns ± 2% +0.63% (p=0.005 n=19+20)
RegexpMatchEasy0_1K-12 240ns ± 0% 239ns ± 1% -0.59% (p=0.000 n=19+20)
RegexpMatchEasy1_32-12 71.3ns ± 1% 71.3ns ± 1% ~ (p=0.844 n=17+17)
RegexpMatchEasy1_1K-12 384ns ± 2% 371ns ± 1% -3.45% (p=0.000 n=19+20)
RegexpMatchMedium_32-12 109ns ± 1% 108ns ± 2% -0.48% (p=0.029 n=19+19)
RegexpMatchMedium_1K-12 34.3µs ± 1% 34.5µs ± 2% ~ (p=0.160 n=18+20)
RegexpMatchHard_32-12 1.79µs ± 9% 1.72µs ± 2% -3.83% (p=0.000 n=19+19)
RegexpMatchHard_1K-12 53.3µs ± 4% 51.8µs ± 1% -2.82% (p=0.000 n=19+20)
Revcomp-12 386ms ± 0% 388ms ± 0% +0.72% (p=0.000 n=17+20)
Template-12 62.9ms ± 1% 62.5ms ± 1% -0.57% (p=0.010 n=18+19)
TimeParse-12 325ns ± 0% 331ns ± 0% +1.84% (p=0.000 n=18+19)
TimeFormat-12 338ns ± 0% 343ns ± 0% +1.34% (p=0.000 n=18+20)
[Geo mean] 52.7µs 52.5µs -0.42%
Change-Id: Ib2d34736c4ae2ec329605b0fbc44636038d8d018
Reviewed-on: https://go-review.googlesource.com/23391
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Also mark it go:systemstack and explain why.
Change-Id: I88baf22741c04012ba2588d8e03dd3801d19b5c0
Reviewed-on: https://go-review.googlesource.com/23390
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
shortens code and gives an example of the use of Run.
Change-Id: I75ffaf762218a589274b4b62e19022e31e805d1b
Reviewed-on: https://go-review.googlesource.com/23424
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Names for Append?Bytes are slightly changed in addition to adding a slash.
Change-Id: I0291aa29c693f9040fd01368eaad9766259677df
Reviewed-on: https://go-review.googlesource.com/23426
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This causes the large files to be loaded only once per benchmark.
This CL also serves as an example use case of sub(tests|-benchmarks).
This CL ensures that names are identical to the original
except for an added slashes. Things could be
simplified further if this restriction were dropped.
Change-Id: I45e303e158e3152e33d0d751adfef784713bf997
Reviewed-on: https://go-review.googlesource.com/23420
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Names of sub-benchmarks are preserved, short of the additional slash.
Change-Id: I9b3f82964f9a44b0d28724413320afd091ed3106
Reviewed-on: https://go-review.googlesource.com/23425
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is reverting golang.org/cl/19622 and introducing "<input>"
as filename if no filename is specified.
Fixes#15813.
Change-Id: Iafc74b789fa33f48ee639c42d4aebc6f06435f95
Reviewed-on: https://go-review.googlesource.com/23402
Reviewed-by: Russ Cox <rsc@golang.org>
Covers a bunch of constant-folding rules in generic.rules that aren't
being covered currently.
Increases coverage in generic.rules from 65% to 72%.
Change-Id: I7bf58809faf22e97070183b42e6dd7d3f35bf5f9
Reviewed-on: https://go-review.googlesource.com/23407
Run-TryBot: Todd Neal <todd@tneal.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Todd Neal <todd@tneal.org>
Also remove some of the now unnecessary corner case handling and
tests I've been adding recently for unexported method data.
For #15673
Change-Id: Ie0c7b03f2370bbe8508cdc5be765028f08000bd7
Reviewed-on: https://go-review.googlesource.com/23410
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 23400 introduced a check to make sure the gold linker is used
on ARM host links. The check itself works, but the error checking
logic was reversed; fix it.
I manually verified that the check now correctly rejects host links
on my RPi2 running an ancient rasbian without the gold linker
installed.
Updates #15696
Change-Id: I927832620f0a60e91a71fdedf8cbd2550247b666
Reviewed-on: https://go-review.googlesource.com/23421
Run-TryBot: Elias Naur <elias.naur@gmail.com>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Other GOARCHs already handle their callee-saved FP registers, but
arm was missing. Without this change, code using Cgo and floating
point code might fail in mysterious and hard to debug ways.
There are no floating point registers when GOARM=5, so skip the
registers when runtime.goarm < 6.
darwin/arm doesn't support GOARM=5, so the check is left out of
rt0_darwin_arm.s.
Fixes#14876
Change-Id: I6bcb90a76df3664d8ba1f33123a74b1eb2c9f8b2
Reviewed-on: https://go-review.googlesource.com/23140
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
The intent of this comment is to reduce the number of issues opened
against the package to add support for new kinds of CSV formats, such as
issues #3150, #8458, #12372, #12755.
Change-Id: I452c0b748e4ca9ebde3e6cea188bf7774372148e
Reviewed-on: https://go-review.googlesource.com/23401
Reviewed-by: Andrew Gerrand <adg@golang.org>
When gentraceback starts on a system stack in sigprof, it is
configured to jump to the user stack when it reaches the end of the
system stack. Currently this updates the current frame's FP, but not
its SP. This is okay on non-LR machines (x86) because frame.sp is only
used to find defers, which the bottom-most frame of the user stack
will never have.
However, on LR machines, we use frame.sp to find the saved LR. We then
use to resolve the function of the next frame, which is used to
resolved the size of the next frame. Since we're not updating frame.sp
on a stack jump, we read the saved LR from the system stack instead of
the user stack and wind up resolving the wrong function and hence the
wrong frame size for the next frame.
This has had remarkably few ill effects (though the resulting profiles
must be wrong). We noticed it because of a bad interaction with stack
barriers. Specifically, once we get the next frame size wrong, we also
get the location of its LR wrong. If we happen to get a stack slot
that contains a stale stack barrier LR (for a stack barrier we already
hit) and hasn't been overwritten with something else as we re-grew the
stack, gentraceback will fail with a "found next stack barrier at ..."
error, pointing at the slot that it thinks is an LR, but isn't.
Fixes#15138.
Updates #15313 (might fix it).
Change-Id: I13cfa322b44c0c2f23ac2b3d03e12631e4a6406b
Reviewed-on: https://go-review.googlesource.com/23291
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Document the fact that the default Source uses only
the bottom 31 bits of the given seed.
Fixes#15788
Change-Id: If20d1ec44a55c793a4a0a388f84b9392c2102bd1
Reviewed-on: https://go-review.googlesource.com/23352
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Document the correct use of the testdata directory
where test writers might be expecting to find it.
It seems that alldocs.go was out of date, so it
has picked up some other changes with this commit.
Fixes#14715.
Change-Id: I0a22676bb7a64b2a61b56495f7ea38db889d8b37
Reviewed-on: https://go-review.googlesource.com/23353
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It's not clear we want to enshrine an io interface in which Size cannot
return an error. Because this requires more thought before committing
to the API, remove from Go 1.7.
Fixes#15818.
Change-Id: Ic4138ffb0e033030145a12d33f78078350a8381f
Reviewed-on: https://go-review.googlesource.com/23392
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
Mostly complete but a few TODOs remain for future CLs.
For #15810.
Change-Id: I81ee19d1088d192cf709a5f7e6b7bcc44ad892ac
Reviewed-on: https://go-review.googlesource.com/23379
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
