Escape analysis has a hard time with tree-like
structures (see #13493 and #14858).
This is unlikely to change.
As a result, when invoking a function that accepts
a **Node parameter, we usually allocate a *Node
on the heap. This happens a whole lot.
This CL changes functions from taking a **Node
to acting more like append: It both modifies
the input and returns a replacement for it.
Because of the cascading nature of escape analysis,
in order to get the benefits, I had to modify
almost all such functions. The remaining functions
are in racewalk and the backend. I would be happy
to update them as well in a separate CL.
This CL was created by manually updating the
function signatures and the directly impacted
bits of code. The callsites were then automatically
updated using a bespoke script:
https://gist.github.com/josharian/046b1be7aceae244de39
For ease of reviewing and future understanding,
this CL is also broken down into four CLs,
mailed separately, which show the manual
and the automated changes separately.
They are CLs 20990, 20991, 20992, and 20993.
name old text-bytes new text-bytes delta
HelloSize 578k ± 0% 578k ± 0% ~ (all samples are equal)
CmdGoSize 6.46M ± 0% 6.46M ± 0% ~ (all samples are equal)
name old data-bytes new data-bytes delta
HelloSize 128k ± 0% 128k ± 0% ~ (all samples are equal)
CmdGoSize 281k ± 0% 281k ± 0% ~ (all samples are equal)
name old exe-bytes new exe-bytes delta
HelloSize 921k ± 0% 921k ± 0% ~ (all samples are equal)
CmdGoSize 9.86M ± 0% 9.86M ± 0% ~ (all samples are equal)
Change-Id: I277d95bd56d51c166ef7f560647aeaa092f3f475
Reviewed-on: https://go-review.googlesource.com/20959 Reviewed-by: Dave Cheney <dave@cheney.net> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Wed, 16 Mar 2016 21:15:54 +0000 (14:15 -0700)]
debug/dwarf: add Reader.SeekPC and Data.Ranges
These new methods help find the compilation unit to pass to the
LineReader method in order to find the line information for a PC.
The Ranges method also helps identify the specific function for a PC,
needed to determine the function name.
This uses the .debug.ranges section if necessary, and changes the object
file format packages to pass in the section contents if available.
Change-Id: I5ebc3d27faaf1a126ffb17a1e6027efdf64af836
Reviewed-on: https://go-review.googlesource.com/20769 Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Keith Randall [Mon, 21 Mar 2016 23:18:45 +0000 (16:18 -0700)]
cmd/compile: allow naming of subexpressions
Allow names to be used for subexpressions of match rules.
For example:
(OpA x:(OpB y)) -> ..use x here to refer to the OpB value..
This gets rid of the .Args[0].Args[0]... way of naming we
used to use.
While we're here, give all subexpression matches names instead
of recomputing them with .Args[i] sequences each time they
are referenced. Makes the generated rule code a bit smaller.
Robert Griesemer [Fri, 18 Mar 2016 18:16:35 +0000 (11:16 -0700)]
math/big: fix rounding to smallest denormal for Float.Float32/64
Converting a big.Float value x to a float32/64 value did not correctly
round x up to the smallest denormal float32/64 if x was smaller than the
smallest denormal float32/64, but larger than 0.5 of a smallest denormal
float32/64.
Handle this case explicitly and simplify some code in the turn.
For #14651.
Change-Id: I025e24bf8f0e671581a7de0abf7c1cd7e6403a6c
Reviewed-on: https://go-review.googlesource.com/20816
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com>
Keith Randall [Mon, 7 Mar 2016 00:58:30 +0000 (16:58 -0800)]
runtime: use MOVSB instead of MOVSQ for unaligned moves
MOVSB is quite a bit faster for unaligned moves.
Possibly we should use MOVSB all of the time, but Intel folks
say it might be a bit faster to use MOVSQ on some processors
(but not any I have access to at the moment).
benchmark old ns/op new ns/op delta
BenchmarkMemmove4096-8 93.9 93.2 -0.75%
BenchmarkMemmoveUnalignedDst4096-8 256 151 -41.02%
BenchmarkMemmoveUnalignedSrc4096-8 175 90.5 -48.29%
Fixes #14630
Change-Id: I568e6d6590eb3615e6a699fb474020596be665ff
Reviewed-on: https://go-review.googlesource.com/20293 Reviewed-by: Ian Lance Taylor <iant@golang.org>
cmd/internal/obj: change linkgetline from C to Go func style
Passes toolstash -cmp.
Change-Id: I8725dee490778be9c1fd31990a6b27df9713c3c9
Reviewed-on: https://go-review.googlesource.com/20957 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Matthew Dempsky [Sun, 20 Mar 2016 20:55:42 +0000 (13:55 -0700)]
cmd/compile: change Mp{int,flt} functions into methods
Also give them more idiomatic Go names. Adding godocs is outside the
scope of this CL. (Besides, the method names almost all directly
parallel an underlying math/big.Int or math/big.Float method.)
CL prepared mechanically with sed (for rewriting mpint.go/mpfloat.go)
and gofmt (for rewriting call sites).
Passes toolstash -cmp.
Change-Id: Id76f4aee476ba740f48db33162463e7978c2083d
Reviewed-on: https://go-review.googlesource.com/20909
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Michael Munday [Fri, 18 Mar 2016 21:14:23 +0000 (17:14 -0400)]
cmd/go: add s390x support
Minimum architecture of z196 required so that GCC can assemble
gcc_s390x.S in runtime/cgo.
Change-Id: I603ed2edd39f826fb8193740ece5bd11d18c3dc5
Reviewed-on: https://go-review.googlesource.com/20876 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Michael Munday [Mon, 21 Mar 2016 03:00:40 +0000 (23:00 -0400)]
cmd/dist: enable -shared and external linking tests on s390x
Change-Id: Iedd01ef3a9d2831cb55c53b7a1984e7e932f4249
Reviewed-on: https://go-review.googlesource.com/20932 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Michael Munday [Sun, 20 Mar 2016 23:25:58 +0000 (19:25 -0400)]
test/fixedbugs: add s390x case to issue11656
An instruction consisting of all 0s causes an illegal instruction
signal on s390x. Since 0s are the default in this test this CL just
makes it explicit.
can never be true: if dup != nil, then s.Name is ".dup" (and this is not new:
the same broken logic is present in 1.4, at least). Delete the whole block.
Dave Cheney [Mon, 21 Mar 2016 03:19:57 +0000 (14:19 +1100)]
cmd/internal/obj: move Nocache helper to arm back end
The obj.Nocache helper was only used by the arm back end, move it there.
Change-Id: I5c9faf995499991ead1f3d8c8ffc3b6af7346876
Reviewed-on: https://go-review.googlesource.com/20868 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Matthew Dempsky [Sun, 20 Mar 2016 21:54:13 +0000 (14:54 -0700)]
math/big: cleanup documentation for Format methods
'b' is a standard verb for floating point values. The runes like '+'
and '#' are called "flags" by package fmt's documentation. The flag
'-' controls left/right justification, not anything related to signs.
Richard Miller [Sun, 20 Mar 2016 19:17:36 +0000 (19:17 +0000)]
net/http: adaptive wait time in PersistConnLeak tests
In tests TransportPersistConnLeak and TransportPersistConnLeakShortBody,
there's a fixed wait time (100ms and 400ms respectively) to allow
goroutines to exit after CloseIdleConnections is called. This
is sometimes too short on a slow host running many simultaneous
tests.
This CL replaces the fixed sleep in each test with a sequence of
shorter sleeps, testing the number of remaining goroutines until
it reaches the threshold or an overall time limit of 500ms expires.
This prevents some failures in the plan9_arm builder, while reducing
the test time on faster machines.
Ian Lance Taylor [Sun, 20 Mar 2016 17:25:46 +0000 (10:25 -0700)]
cmd/compile: fix varexpr handling of ODOT
For a long time varexpr has handled ODOT incorrectly: it has always
returned false. Before https://golang.org/cl/20890 this has been
because an ODOT had a Right field with an ONAME with no Class, for which
varexpr returns false. CL 20890 preserved the behavior of varexpr for
ODOT, so that the change would pass toolstash -cmp.
This CL fixes varexpr so that ODOT can return true in some cases. This
breaks toolstash -cmp. While the changed compiler allocates temporary
variables in a different order, I have not been able to find any
examples where the generated code is different, other than using
different stack offsets and, in some cases, registers. It seems that
other parts of the compiler will force the ODOT into a temporary anyhow.
Still, this change is clearly correct, and is a minor compiler cleanup.
Change-Id: I71506877aa3c13966bb03c281aa16271ee7fe80a
Reviewed-on: https://go-review.googlesource.com/20907
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Ian Lance Taylor [Sat, 19 Mar 2016 01:12:14 +0000 (18:12 -0700)]
cmd/compile: don't penalize ODOT and friends when inlining
Historically ODOT and friends have been considered to cost an extra
budget point when deciding whether they should be inlined, because they
had an ONAME node that represented the name to the right of the dot.
This doesn't really make sense, as in general that symbol does not add
any extra instructions; it just affects the offset of the load or store
instruction. And the ONAME node is gone now. So, remove the extra
cost.
This does not pass toolstash -cmp, as it changes inlining decisions.
For example, mspan.init in runtime/mheap.go is now considered to be an
inlining candidate.
Change-Id: I5ad27f08c66fd5daa4c8472dd0795df989183f5e
Reviewed-on: https://go-review.googlesource.com/20891 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Matthew Dempsky [Sun, 20 Mar 2016 00:51:17 +0000 (17:51 -0700)]
cmd/compile: ignore receiver name when checking duplicate methods
In golang.org/cl/20602, I changed the semantics of Eqtype to stop
checking the receiver parameters for type equality, and pushed this
responsibility to addmethod (the only Eqtype caller that cared).
However, I accidentally made the check stricter by making it start
requiring that receiver names were identical.
In general, this is a non-problem because the receiver names in export
data will always match the original source. But running
GO_GCFLAGS=-newexport ./all.bash at one point tries to load both old
and new format export data for package sync, which reveals the
problem. (See golang.org/issue/14877 for details.)
Easy fix: just check the receiver type for type equality in addmethod,
instead of the entire receiver parameter list.
Martin Möhrmann [Sat, 19 Mar 2016 12:18:43 +0000 (13:18 +0100)]
fmt: remove depth argument from handleMethods and printArg
handleMethods can format Error() and String() directly as its known
these return strings that can be directly printed using fmtString.
Remove the obsolete depth argument from handleMethods.
Remove the depth argument from printArg since it is only ever
called with depth set to 0. Recursion for formatting complex
arguments is handled only by printValue which keeps track of depth.
Change-Id: I4c4be588751de12ed999e7561a51bc168eb9eb2d
Reviewed-on: https://go-review.googlesource.com/20911
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Ian Lance Taylor [Fri, 18 Mar 2016 23:52:30 +0000 (16:52 -0700)]
cmd/compile: change ODOT and friends to use Sym, not Right
The Node type ODOT and its variants all represent a selector, with a
simple name to the right of the dot. Before this change this was
represented by using an ONAME Node in the Right field. This ONAME node
served no useful purpose. This CL changes these Node types to store the
symbol in the Sym field instead, thus not requiring allocating a Node
for each selector.
When compiling x/tools/go/types this CL eliminates nearly 5000 calls to
newname and reduces the total number of Nodes allocated by about 6.6%.
It seems to cut compilation time by 1 to 2 percent.
Getting this right was somewhat subtle, and I added two dubious changes
to produce the exact same output as before. One is to ishairy in
inl.go: the ONAME node increased the cost of ODOT and friends by 1, and
I retained that, although really ODOT is not more expensive than any
other node. The other is to varexpr in walk.go: because the ONAME in
the Right field of an ODOT has no class, varexpr would always return
false for an ODOT, although in fact for some ODOT's it seemingly ought
to return true; I added an && false for now. I will send separate CLs,
that will break toolstash -cmp, to clean these up.
Todd Neal [Wed, 16 Mar 2016 23:44:17 +0000 (18:44 -0500)]
cmd/compile: allow inlining of functions that declare a const
Consider functions with an ODCLCONST for inlining and modify exprfmt to
ignore those nodes when exporting. Don't add symbols to the export list
if there is no definition. This occurs when OLITERAL symbols are looked
up via Pkglookup for non-exported symbols.
Martin Möhrmann [Fri, 18 Mar 2016 15:56:47 +0000 (16:56 +0100)]
fmt: simplify handling of reporting flags to formatters
Remove rewriting of flags before calling formatters.
Change Flag method to directly take plusV and sharpV flags
into account when reporting if plus or sharp flag is set.
Change-Id: Ic3423881ad89e5a5f9fff5ab59e842062394ef6d
Reviewed-on: https://go-review.googlesource.com/20859
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Marcel van Lohuizen [Fri, 29 Jan 2016 15:55:35 +0000 (16:55 +0100)]
testing: hoisted chunks of code to prepare for Run method
testing.go:
- run method will evolve into the Run method.
- added level field in common
benchmark.go:
- benchContext will be central to distinguish handling of benchmarks
between normal Run methods and ones called from within Benchmark
function.
- expandCPU will evolve into the processing hook for Run methods
called within normal processing.
- runBench will evolve into the Run method.
Marcel van Lohuizen [Tue, 19 Jan 2016 21:43:52 +0000 (22:43 +0100)]
testing: prepare for the introduction of Run methods
The biggest change is that each test is now responsible for managing
the starting and stopping of its parallel subtests.
The "Main" test could be run as a tRunner as well. This shows that
the introduction of subtests is merely a generalization of and
consistent with the current semantics.
Christopher Nelson [Tue, 15 Mar 2016 13:14:22 +0000 (09:14 -0400)]
cmd/go: fix TestShadowingLogic fails when GOROOT path has spaces
Improve the test by also translating " " to "_".
Fixes #14671.
Change-Id: Ie5997934b93c7663d7b8432244fad47bb5d3ffbe
Reviewed-on: https://go-review.googlesource.com/20714 Reviewed-by: Ian Lance Taylor <iant@golang.org>
David Chase [Thu, 17 Mar 2016 18:12:12 +0000 (14:12 -0400)]
cmd/compile: get gcflags to bootstrap; ssa debug opts for "all"
This is intended to help debug compiler problems that pop
up in the bootstrap phase of make.bash. GO_GCFLAGS does not
normally apply there. Options-for-all phases is intended
to allow crude tracing (and full timing) by turning on timing
for all phases, not just one.
Phase names can also be specified using a regular expression,
for example
BOOT_GO_GCFLAGS=-d='ssa/~^.*scc$/off' \
GO_GCFLAGS='-d=ssa/~^.*scc$/off' ./make.bash
I just added this because it was the fastest way to get
me to a place where I could easily debug the compiler.
Change-Id: I0781f3e7c19651ae7452fa25c2d54c9a245ef62d
Reviewed-on: https://go-review.googlesource.com/20775 Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Mikio Hara [Wed, 16 Mar 2016 20:33:13 +0000 (05:33 +0900)]
net: make unexposed methods start with lowercase letters
This change makes unexposed methods start with lowercase letters for
avoiding unnecessary confusion because the net package uses many
embedding structures and intrefaces for controlling exposure of APIs.
Note that this change leaves DNS-related methods as they are.
Change-Id: I253758d1659175c5d0af6b2efcd30ce83f46543d
Reviewed-on: https://go-review.googlesource.com/20784
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Alexandru Moșoi [Sun, 13 Mar 2016 21:12:03 +0000 (22:12 +0100)]
encoding/binary: remove bound checks from conversions.
* This the simplest solution I could came up with
that doesn't required changing the compiler.
* The bound checks become constants now
so they are removed during opt phase.
Matthew Dempsky [Thu, 17 Mar 2016 08:47:16 +0000 (01:47 -0700)]
cmd/compile: get rid of Type's {This,In,Out}tuple fields
Boolean expressions involving t.Thistuple were converted to use
t.Recv(), because it's a bit clearer and will hopefully reveal cases
where we could remove redundant calls to t.Recv() (in followup CLs).
The other cases were all converted to use t.Recvs().NumFields(),
t.Params().NumFields(), or t.Results().NumFields().