Shows up occassionally, especially after p = p[:8:len(p)]
Updates #14905
Change-Id: Iab35ef2eac57817e6a10c6aaeeb84709e8021641
Reviewed-on: https://go-review.googlesource.com/21025
Run-TryBot: Alexandru Moșoi <alexandru@mosoi.ro>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The matcher is responsible for sanitizing and uniquing the
test and benchmark names and thus needs to be included before the
API can be exposed.
Matching currently uses the regexp to only match the top-level
tests/benchmarks.
Support for subtest matching is for another CL.
Change-Id: I7c8464068faef7ebc179b03a7fe3d01122cc4f0b
Reviewed-on: https://go-review.googlesource.com/18897
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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.
Passes toolstash -cmp.
name old time/op new time/op delta
Template 335ms ± 5% 324ms ± 5% -3.35% (p=0.000 n=23+24)
Unicode 176ms ± 9% 165ms ± 6% -6.12% (p=0.000 n=23+24)
GoTypes 1.10s ± 4% 1.07s ± 2% -2.77% (p=0.000 n=24+24)
Compiler 5.31s ± 3% 5.15s ± 3% -2.95% (p=0.000 n=24+24)
MakeBash 41.6s ± 1% 41.7s ± 2% ~ (p=0.586 n=23+23)
name old alloc/op new alloc/op delta
Template 63.3MB ± 0% 62.4MB ± 0% -1.36% (p=0.000 n=25+23)
Unicode 42.4MB ± 0% 41.6MB ± 0% -1.99% (p=0.000 n=24+25)
GoTypes 220MB ± 0% 217MB ± 0% -1.11% (p=0.000 n=25+25)
Compiler 994MB ± 0% 973MB ± 0% -2.08% (p=0.000 n=24+25)
name old allocs/op new allocs/op delta
Template 681k ± 0% 574k ± 0% -15.71% (p=0.000 n=24+25)
Unicode 518k ± 0% 413k ± 0% -20.34% (p=0.000 n=25+24)
GoTypes 2.08M ± 0% 1.78M ± 0% -14.62% (p=0.000 n=25+25)
Compiler 9.26M ± 0% 7.64M ± 0% -17.48% (p=0.000 n=25+25)
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>
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>
Adds a new R_PCRELDBL relocation for 2-byte aligned relative
relocations on s390x. Should be removed once #14218 is
implemented.
Change-Id: I79dd2d8e746ba8cbc26c570faccfdd691e8161e8
Reviewed-on: https://go-review.googlesource.com/20941
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Should let the s390x builder progress a little further.
Change-Id: I5eab5f384b0b039f8e246ba69ecfb24de08625d2
Reviewed-on: https://go-review.googlesource.com/20965
Reviewed-by: Minux Ma <minux@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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.
Change-Id: Ie42139f6f208933b75bd2ae8bd34e95419bc0e4e
Reviewed-on: https://go-review.googlesource.com/20997
Run-TryBot: Todd Neal <todd@tneal.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Todd Neal <todd@tneal.org>
Don't write back parts of a slicing operation if they
are unchanged from the source of the slice. For example:
x.s = x.s[0:5] // don't write back pointer or cap
x.s = x.s[:5] // don't write back pointer or cap
x.s = x.s[:5:7] // don't write back pointer
There is more to be done here, for example:
x.s = x.s[:len(x.s):7] // don't write back ptr or len
This CL can't handle that one yet.
Fixes#14855
Change-Id: Id1e1a4fa7f3076dc1a76924a7f1cd791b81909bb
Reviewed-on: https://go-review.googlesource.com/20954
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
For the following example, but there are a few more in the stdlib:
func histogram(b []byte, h *[256]int32) {
for _, t := range b {
h[t]++
}
}
Change-Id: I56615f341ae52e02ef34025588dc6d1c52122295
Reviewed-on: https://go-review.googlesource.com/20924
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Alexandru Moșoi <alexandru@mosoi.ro>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Allow inlining of functions with switch statements as long as they don't
contain a break or type switch.
Fixes#13071
Change-Id: I057be351ea4584def1a744ee87eafa5df47a7f6d
Reviewed-on: https://go-review.googlesource.com/20824
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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>
The inserted early bound checks cause the slice
to expand beyond the original length of the slice.
Change-Id: Ib38891605f4a9a12d3b9e2071a5f77640b083d2d
Reviewed-on: https://go-review.googlesource.com/20981
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
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>
Left over from CL 20931.
Change-Id: I3b8dd9ef748bcbf70b5118da28135aaa1e5ba3a8
Reviewed-on: https://go-review.googlesource.com/20955
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Minux Ma <minux@golang.org>
Constant comparisons against 0 are reasonably common.
Special-case and avoid allocating a new zero value each time.
Change-Id: I6c526c8ab30ef7f0fef59110133c764b7b90ba05
Reviewed-on: https://go-review.googlesource.com/20956
Reviewed-by: Alan Donovan <adonovan@google.com>
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>
Fixed bug that slipped probably slipped in after rebasing and
explain why it failed on nacl/netbsd/plan9, which set default
maxparallelism to 1.
Change-Id: I4d59682fb2843d138b320334189f53fcdda5b2f6
Reviewed-on: https://go-review.googlesource.com/20980
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The Linux ABI takes arguments in a different order on s390x.
Change-Id: Ic9cfcc22a5ea3d8ef77d4dd0b915fc266ff3e5f7
Reviewed-on: https://go-review.googlesource.com/20960
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
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>
It was just a funny way of saying len(Ctxt.Allsym) by now.
Change-Id: Iff75e73c9f7ec4ba26cfef479bbd05d7dcd172f5
Reviewed-on: https://go-review.googlesource.com/20973
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Nothing cares about it.
I did this after looking at the memprof output, but it helps performance a bit:
name old s/op new s/op delta
LinkCmdGo 0.44 ± 3% 0.43 ± 3% -2.20% (p=0.000 n=94+90)
LinkJuju 3.98 ± 5% 3.94 ± 5% -1.19% (p=0.000 n=100+91)
As well as MaxRSS (i.e. what /usr/bin/time -f '%M' prints):
name old MaxRSS new MaxRSS delta
LinkCmdGo 130k ± 0% 120k ± 3% -7.79% (p=0.000 n=79+90)
LinkJuju 862k ± 6% 827k ± 8% -4.01% (p=0.000 n=100+99)
Change-Id: I6306b7b3369576a688659e2ecdb0815b4152ae96
Reviewed-on: https://go-review.googlesource.com/20972
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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.
Change-Id: Id6e060eed1a588f4b10a4e4861709fcd19b434ac
Reviewed-on: https://go-review.googlesource.com/20962
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Debugasm can never be set in cmd/link, so delete it and the code it enables.
Change-Id: If828db0b09f1a9e512dc660ac2750657a769094c
Reviewed-on: https://go-review.googlesource.com/20971
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This expression in readsym:
dup != nil && len(dup.P) > 0 && strings.HasPrefix(s.Name, "gclocals·")
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.
Change-Id: I33b14d9a82b292116d6fd79d22b38e3842501317
Reviewed-on: https://go-review.googlesource.com/20970
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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>
Add a special helper for its one external use.
This is in preparation for an upcoming CL.
Passes toolstash -cmp / buildall.
Change-Id: I9d3463792afe220cc4bc89269bdecf0279abd281
Reviewed-on: https://go-review.googlesource.com/20933
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This CL addresses a long standing CL by rsc by pushing the use of
Link.Windows down to its two users.
Link.Window was always initalised with the value of runtime.GOOS so
this does not affect cross compilation.
Change-Id: Ibbae068f8b5aad06336909691f094384caf12352
Reviewed-on: https://go-review.googlesource.com/20869
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
'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.
Change-Id: Ia9cf81b002df373f274ce635fe09b5bd0066aa1c
Reviewed-on: https://go-review.googlesource.com/20930
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Another object file change, gives a reasonable improvement:
name old s/op new s/op delta
LinkCmdGo 0.46 ± 3% 0.44 ± 9% -3.34% (p=0.000 n=98+82)
LinkJuju 4.09 ± 4% 3.92 ± 5% -4.30% (p=0.000 n=98+99)
I guess the data section could be mmap-ed instead of read, I haven't tried
that.
Change-Id: I959eee470a05526ab1579e3f5d3ede41c16c954f
Reviewed-on: https://go-review.googlesource.com/20928
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
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.
Fixes#14887
Change-Id: Ia5c871062df139e2667cdfb2ce8283e135435318
Reviewed-on: https://go-review.googlesource.com/20922
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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>
