Deleting the string merging pass makes the linker 30-35% faster
but makes jujud (using the github.com/davecheney/benchjuju snapshot) 2.5% larger.
Two optimizations bring the space overhead down to 0.6%.
First, change the default alignment for string data to 1 byte.
(It was previously defaulting to larger amounts, usually pointer width.)
Second, write out the type string for T (usually a bigger expression) as "*T"[1:],
so that the type strings for T and *T share storage.
Combined, these obtain the bulk of the benefit of string merging
at essentially no cost. The remaining benefit from string merging
is not worth the excessive cost, so delete it.
As penance for making the jujud binary 0.6% larger,
the next CL in this sequence trims the reflect functype
information enough to make the jujud binary overall 0.75% smaller
(that is, that CL has a net -1.35% effect).
For #6853.
Fixes#14648.
Change-Id: I3fdd74c85410930c36bb66160ca4174ed540fc6e
Reviewed-on: https://go-review.googlesource.com/20334
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
cmd/link is clearly the way forward.
The original rationale for cmd/newlink was that it would be a clean Go reimplementation.
But when push came to shove, cmd/link got converted from C instead,
and all the work on build modes and the like is in cmd/link now.
Cleaning up cmd/link is likely a much better plan.
This directory is something to delete from releases and the
testdata is something that breaks every time the .6 format changes.
Fix both problems by just deleting it outright.
Change-Id: Ib00fecda258ba685f1752725971182af9d4459eb
Reviewed-on: https://go-review.googlesource.com/20380
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also, add more failure output to debug why linux/mips64le and
linux/ppc64 are failing. They should be working. I suspect their
builder test envs are missing something.
Change-Id: I97273fe72c4e3009db400394636d0da1ef147485
Reviewed-on: https://go-review.googlesource.com/20358
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This adds "slicing by 8" optimization to Castagnoli tables which will
speed up CRC32 calculation on systems without asssembler,
which are all but AMD64.
In my tests, it is faster to use "slicing by 8" for sizes all down to
16 bytes, so the switchover point has been adjusted.
There are no benchmarks for small sizes, so I have added one for 40 bytes,
as well as one for bigger sizes (32KB).
Castagnoli, No assembler, 40 Byte payload: (before, after)
BenchmarkCastagnoli40B-4 10000000 161 ns/op 246.94 MB/s
BenchmarkCastagnoli40B-4 20000000 100 ns/op 398.01 MB/s
Castagnoli, No assembler, 32KB payload: (before, after)
BenchmarkCastagnoli32KB-4 10000 115426 ns/op 283.89 MB/s
BenchmarkCastagnoli32KB-4 30000 45171 ns/op 725.41 MB/s
IEEE, No assembler, 1KB payload: (before, after)
BenchmarkCrc1KB-4 500000 3604 ns/op 284.10 MB/s
BenchmarkCrc1KB-4 1000000 1463 ns/op 699.79 MB/s
Compared:
benchmark old ns/op new ns/op delta
BenchmarkCastagnoli40B-4 161 100 -37.89%
BenchmarkCastagnoli32KB-4 115426 45171 -60.87%
BenchmarkCrc1KB-4 3604 1463 -59.41%
benchmark old MB/s new MB/s speedup
BenchmarkCastagnoli40B-4 246.94 398.01 1.61x
BenchmarkCastagnoli32KB-4 283.89 725.41 2.56x
BenchmarkCrc1KB-4 284.10 699.79 2.46x
Change-Id: I303e4ec84e8d4dafd057d64c0e43deb2b498e968
Reviewed-on: https://go-review.googlesource.com/19335
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Without SSA:
$ go build -a -gcflags='-S -ssa=0' runtime 2>&1 | grep 'TEXT.*""\.init(SB)'
0x0000 00000 ($GOROOT/src/runtime/write_err.go:14) TEXT "".init(SB), $88-0
With SSA, before this CL:
$ go build -a -gcflags='-S -ssa=1' runtime 2>&1 | grep 'TEXT.*""\.init(SB)'
0x0000 00000 ($GOROOT/src/runtime/traceback.go:608) TEXT "".init(SB), $152-0
With SSA, after this CL:
$ go build -a -gcflags='-S -ssa=1' runtime 2>&1 | grep 'TEXT.*""\.init(SB)'
0x0000 00000 ($GOROOT/src/runtime/write_err.go:14) TEXT "".init(SB), $152-0
Change-Id: Ida3541e03a1af6ffc753ee5c3abeb653459edbf6
Reviewed-on: https://go-review.googlesource.com/20321
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The existing implementation deals with absolute relocations in __TEXT
for darwin/amd64 in build-mode c-shared, but it ignores c-archive.
This results in issues when trying to use a c-archive in an iOS
app on the 64-bit simulator. This patch adds c-archive to the
handling of this issue.
Fixes#14217
Change-Id: I2e4d5193caa531171ad22fd0cd420a8bfb4646a6
Reviewed-on: https://go-review.googlesource.com/19206
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This adds a heap-based proper priority queue to the
scheduler which made a relatively easy to test quite a few
heuristics that "ought to work well". For go tools
themselves (which may not be representative) the heuristic
that works best is (1) in line-number-order, then (2) from
more to fewer args, then (3) in variable ID order. Trying
to improve this with information about use at end of
blocks turned out to be fruitless -- all of my naive
attempts at using that information turned out worse than
ignoring it. I can confirm that the stores-early heuristic
tends to help; removing it makes the results slightly worse.
My metric is code size reduction, which I take to mean fewer
spills from register allocation. It's not uniform.
Here's the endpoints for "vet" from one set of pretty-good
heuristics (this is representative at least).
-2208 time.parse 13472 15680 -14.081633%
-1514 runtime.pclntab 1002058 1003572 -0.150861%
-352 time.Time.AppendFormat 9952 10304 -3.416149%
-112 runtime.runGCProg 1984 2096 -5.343511%
-64 regexp/syntax.(*parser).factor 7264 7328 -0.873362%
-44 go.string.alldata 238630 238674 -0.018435%
48 math/big.(*Float).round 1376 1328 3.614458%
48 text/tabwriter.(*Writer).writeLines 1232 1184 4.054054%
48 math/big.shr 832 784 6.122449%
88 go.func.* 75174 75086 0.117199%
96 time.Date 1968 1872 5.128205%
Overall there appears to be an 0.1% decrease in text size.
No timings yet, and given the distribution of size reductions
it might make sense to wait on those.
addr2line text (code) = -4392 bytes (-0.156273%)
api text (code) = -5502 bytes (-0.147644%)
asm text (code) = -5254 bytes (-0.187810%)
cgo text (code) = -4886 bytes (-0.148846%)
compile text (code) = -1577 bytes (-0.019346%) * changed
cover text (code) = -5236 bytes (-0.137992%)
dist text (code) = -5015 bytes (-0.167829%)
doc text (code) = -5180 bytes (-0.182121%)
fix text (code) = -5000 bytes (-0.215148%)
link text (code) = -5092 bytes (-0.152712%)
newlink text (code) = -5204 bytes (-0.196986%)
nm text (code) = -4398 bytes (-0.156018%)
objdump text (code) = -4582 bytes (-0.155046%)
pack text (code) = -4503 bytes (-0.294287%)
pprof text (code) = -6314 bytes (-0.085177%)
trace text (code) = -5856 bytes (-0.097818%)
vet text (code) = -5696 bytes (-0.117334%)
yacc text (code) = -4971 bytes (-0.213817%)
This leaves me sorely tempted to look into a "real" scheduler
to try to do a better job, but I think it might make more
sense to look into getting loop information into the
register allocator instead.
Fixes#14577.
Change-Id: I5238b83284ce76dea1eb94084a8cd47277db6827
Reviewed-on: https://go-review.googlesource.com/20240
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
When the pointer offset is non-zero in the small loads, we need to add the offset
when converting to the larger load.
Fixes#14694
Change-Id: I5ba8bcb3b9ce26c7fae0c4951500b9ef0fed54cd
Reviewed-on: https://go-review.googlesource.com/20333
Reviewed-by: Keith Randall <khr@golang.org>
Currently, package obj reserves a range of 1<<12 opcodes for each
target architecture. E.g., mips64 has [6<<12, 7<<12).
However, because mips.ABEQ and mips.ALAST are both within that range,
the expression mips.ABEQ+mips.ALAST in turn falls (far) outside that
range around 12<<12, meaning it could theoretically collide with
another arch's opcodes.
More practically, it's a problem because 12<<12 overflows an int16,
which hampers fixing #14692. (We could also just switch to uint16 to
avoid the overflow, but that still leaves the first problem.)
As a workaround, use Michael Hudson-Doyle's solution from
https://golang.org/cl/20182 and use negative values for these variant
instructions.
Passes toolstash -cmp for GOARCH=arm and GOARCH=mips64.
Updates #14692.
Change-Id: Iad797d10652360109fa4db19d4d1edb6529fc2c0
Reviewed-on: https://go-review.googlesource.com/20345
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is needed now for subrepos to be able to conditionally use
API symbols found only after Go 1.6.
Change-Id: Ie7d9301332aa1739b585d93f8025424ae72a2430
Reviewed-on: https://go-review.googlesource.com/20344
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
TimeoutHandler was starting the Timer when the handler was created,
instead of when serving a request. It also was sharing it between
multiple requests, which is incorrect, as the requests might start
at different times.
Store the timeout duration and create the Timer when ServeHTTP is
called. Different requests will have different timers.
The testing plumbing was simplified to store the channel used to
control when timeout happens. It overrides the regular timer.
Fixes#14568.
Change-Id: I4bd51a83f412396f208682d3ae5e382db5f8dc81
Reviewed-on: https://go-review.googlesource.com/20046
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Compile time is about the same. Getting rid of the nodeSeq interfaces,
particularly nodeSeqIterate, should produce some improvements.
Passes toolstash -cmp.
Update #14473.
Change-Id: I678abafdd9129c6cccb0ec980511932eaed496a0
Reviewed-on: https://go-review.googlesource.com/20343
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It is only necessary in a few places, and this inlining will
simplify the transition away from NodeLists.
Passes toolstash -cmp.
Change-Id: I4ee9b4bf56ffa04df23e20a0a83b302d36b33510
Reviewed-on: https://go-review.googlesource.com/20290
Reviewed-by: Ian Lance Taylor <iant@golang.org>
-test flag is a testing only flag that enables all vet checks. It was needed
because there was no way to run all vet checks in a single command
invocation. However it is possible to do this now by combining -all and -shadow
flags.
Also a recently added -tests flag is similarly named, having both -test and
-tests can be confusing.
Change-Id: Ie5bacbe0bef5c8409eeace46f16141fa4e782c32
Reviewed-on: https://go-review.googlesource.com/20006
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Opt for replacements that avoid any assumptions
about the representations in use.
Passes toolstash -cmp.
Change-Id: Ia858a33abcae344e03fc1862fc9b0e192fde80c1
Reviewed-on: https://go-review.googlesource.com/20279
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fix some test output while we're here.
Change-Id: I265cedc222e078eff120f268b92451e12b0400b2
Reviewed-on: https://go-review.googlesource.com/20294
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fixes#14693
Change-Id: Id0a6a80b4c37c0b0f1c2755667b7233ed8964e40
Reviewed-on: https://go-review.googlesource.com/20342
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently work.finalizersDone is reset only at the beginning of
gcStart. As a result, it will be set when checkmark runs, so checkmark
will skip scanning finalizers. Hence, if there are any bugs that cause
the regular scan of finalizers to miss pointers, checkmark will also
miss them and fail to detect the missed pointer.
Fix this by resetting finalizersDone in gcResetMarkState. This way it
gets reset before any full mark, which is exactly what we want.
Change-Id: I4ddb5eba5b3b97e52aaf3e08fd9aa692bda32b20
Reviewed-on: https://go-review.googlesource.com/20332
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In increment and decrement statements, explicit check that the type
of operand is numeric. This avoids a related but less clear error
about converting "1" to be emitted.
So, when checking
package main
func main() {
var x bool
x++
}
instead of emitting the error
prog.go:5:2: cannot convert 1 (untyped int constant) to bool
emits
prog.go:5:2: invalid operation: x++ (non-numeric type bool).
Updates #12525.
Change-Id: I00aa6bd0bb23267a2fe10ea3f5a0b20bbf3552bc
Reviewed-on: https://go-review.googlesource.com/20244
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Move initproginfo and initvariants to ppc64.Main to avoid checking that
the tables are initialised every time.
Change-Id: I95ff4146a7abc18c42a20bfad716cc80ea8367e5
Reviewed-on: https://go-review.googlesource.com/20286
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fixes#14626
Change-Id: I91c40407dc35355e5c5046f24111a126f99260d9
Reviewed-on: https://go-review.googlesource.com/20192
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
As part of local testing with a large group member list, I discovered
that the lookup functions don't resize their buffer if they receive
ERANGE. I fixed this as a side-effect of this CL.
Thanks to @andrenth for the original CL.
Fixes#2617
Change-Id: Ie6aae2fe0a89eae5cce85786869a8acaa665ffe9
Reviewed-on: https://go-review.googlesource.com/19235
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The io.Reader contract makes no promises about how a Reader should
behave after it returns its first error. Usually the errors are
sticky, but they don't have to be. A regression in zlib.Reader (bug
accidentally relied on sticky errors.
Minimal fix: wrap the user's provided Reader in a Reader which
guarantees stickiness. The minimal fix is less scary than touching
the multipart state machine.
Fixes#14676
Change-Id: I8dd8814b13ae5530824ae0e68529f788974264a5
Reviewed-on: https://go-review.googlesource.com/20297
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Ensure that all errors (including io.EOF) are persistent across method
calls on zlib.Reader. Furthermore, ensure that these persistent errors
are properly cleared when Reset is called.
Fixes#14675
Change-Id: I15a20c7e25dc38219e7e0ff255d1ba775a86bb47
Reviewed-on: https://go-review.googlesource.com/20292
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Found by temporarily flipping fields from *NodeList to Nodes and fixing
all the compilation errors. This CL does not actually change any
fields.
Passes toolstash -cmp.
Update #14473.
Change-Id: Ib98fa37e8752f96358224c973a743618a6a0e736
Reviewed-on: https://go-review.googlesource.com/20320
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Eliminates type conversions in a bunch of Oconv(int(n.Op), ...) calls.
Notably, this identified a misuse of Oconv in amd64/gsubr.go to try to
print an assembly instruction op instead of a compiler node op.
Change-Id: I93b5aa49fe14a5eaf868b05426d3b8cd8ab52bc5
Reviewed-on: https://go-review.googlesource.com/20298
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Implementation more or less follows plan9_386 version.
Revised 7 March to correct a bug in runtime.seek and
tidy whitespace for 8-column tabs.
Change-Id: I2e921558b5816502e8aafe330530c5a48a6c7537
Reviewed-on: https://go-review.googlesource.com/18966
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Plan 9 trap/signal handling differs on ARM from other architectures
because ARM has a link register. Also trap message syntax varies
between different architectures (historical accident?).
Revised 7 March to clarify a comment.
Change-Id: Ib6485f82857a2f9a0d6b2c375cf0aaa230b83656
Reviewed-on: https://go-review.googlesource.com/18969
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In increment and decrement statements, explicit check that the type
of operand is numeric earlier. This avoids a related but less clear
error about converting "1" to be emitted.
So, when compiling
package main
func main() {
var x bool
x++
}
instead of emitting two errors
prog.go:5: cannot convert 1 to type bool
prog.go:5: invalid operation: x++ (non-numeric type bool)
just emits the second error.
Fixes#12525.
Change-Id: I6e81330703765bef0d6eb6c57098c1336af7c799
Reviewed-on: https://go-review.googlesource.com/20245
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This increases the number of matches in make.bash
from 853 to 984.
Change-Id: I12697697a50ecd86d49698200144a4c80dd3e5a4
Reviewed-on: https://go-review.googlesource.com/20274
Reviewed-by: Todd Neal <todd@tneal.org>
We used to start background mark workers and assists at different
times, so we needed to keep track of these separately. They're now set
to exactly the same time, so clean things up by merging them in to one
value, markStartTime.
Change-Id: I17c9843c3ed2d6f07b4c8cd0b2c438fc6de23b53
Reviewed-on: https://go-review.googlesource.com/20143
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
OffPtr allocates less and is easier to optimize.
With this change, the OffPtr collapsing opt
rule matches increase from 160k to 263k,
and the Load-after-Store opt rule matches
increase from 217 to 853.
Change-Id: I763426a3196900f22a367f7f6d8e8047b279653d
Reviewed-on: https://go-review.googlesource.com/20273
Run-TryBot: Todd Neal <todd@tneal.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Todd Neal <todd@tneal.org>
Reviewed-by: Keith Randall <khr@golang.org>
This triggers an astonishing 160k times
during make.bash. The second biggest
generic rewrite triggers 100k times.
However, this is really just moving
rewrites that were happening at the
architecture level to the generic level.
Change-Id: Ife06fe5234f31433328460cb2e0741c071deda41
Reviewed-on: https://go-review.googlesource.com/20235
Run-TryBot: Todd Neal <todd@tneal.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Todd Neal <todd@tneal.org>
This only deals with the loads themselves. The bounds checks
are a separate issue. Also doesn't handle stores, those are
harder because we need to make sure intermediate memory states
aren't observed (which is hard to do with rewrite rules).
Use one byte shorter instructions for zero-extending loads.
Update #14267
Change-Id: I40af25ab5208488151ba7db32bf96081878fa7d9
Reviewed-on: https://go-review.googlesource.com/20218
Reviewed-by: Alexandru Moșoi <alexandru@mosoi.ro>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
They were only used for rtype.ptrToThis which David Crawshaw removed a couple
of weeks ago. Removes two traversals of Ctxt.Allsym from the linker but it
doesn't seem to make much difference to performance.
Change-Id: I5c305e0180186f643221d57822d301de4aa18827
Reviewed-on: https://go-review.googlesource.com/20287
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The new code is a bit less efficient,
but it does not involve altering the structure
of any linked lists.
This will make it easier to replace NodeLists
with Node slices.
We can return to a more efficient algorithm
when NodeLists have been replaced.
Passes toolstash -cmp.
Change-Id: I0bb5ee75e7c0646e6d37fe558c8f0548729d8aa1
Reviewed-on: https://go-review.googlesource.com/20277
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
