This change is a non-minimal fix for #32917, but incidentally fixes
several other bugs and makes the error messages much more ergonomic.
Updates #32917
Updates #27122
Updates #28459
Updates #29280
Updates #30590
Updates #37214
Updates #36173
Updates #36587Fixes#36008Fixes#30992
Change-Id: Iedb26d2e0963697c130df5d0f72e7f83ec2dcf06
Reviewed-on: https://go-review.googlesource.com/c/go/+/185345
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This change consolidates predicates currently scattered throughout
various parts of the package and module loader into methods on the
search.Match type.
That not only makes them more concise, but also encourages
consistency, both in the code and in reasoning about the kinds of
patterns that need to be handled. (For example, the IsLocal predicate
was previously two different calls, either of which could be easily
forgotten at a given call site.)
Factored out from CL 185344 and CL 185345.
Updates #32917
Change-Id: Ifa450ffaf6101f673e0ed69ced001a487d6f9335
Reviewed-on: https://go-review.googlesource.com/c/go/+/221458
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Previously, we would either invoke base.Fatalf (which is too aggressive),
or log.Print (which is too passive).
Updates #32917
Change-Id: I5475e873e76948de7df65dca08bc0ce67a7fc827
Reviewed-on: https://go-review.googlesource.com/c/go/+/185344
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
AmbiguousImportErrors will now be formatted like other ImportPathErrors:
this means that now the ambiguously imported package won't be printed
twice. Whereas the error message looked like the following:
can't load package: package example.com/m/importy: ambiguous import: found package example.com/m/importy in multiple directories:
$WORK/importy
$WORK/vendor/example.com/m/importy
It now looks like this:
can't load package: ambiguous import: found package example.com/m/importy in multiple directories:
$WORK/importy
$WORK/vendor/example.com/m/importy
Change-Id: I52a2074a6b3f5eb7d78d331d0852b7ea6b3735e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/221457
Run-TryBot: Michael Matloob <matloob@golang.org>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The code was incorrectly using a string conversion of a numeric port
to display the port number.
No test because as far as I can tell this code is only executed if
there is some error in a /net file.
Updates #32479
Change-Id: I0b8deebbf3c0b7cb1e1eee0fd059505f3f4c1623
Reviewed-on: https://go-review.googlesource.com/c/go/+/221377
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The goal here is improved AuxInt printing in ssa.html.
Instead of displaying an inscrutable encoded integer,
it displays something like
v25 (28) = UBFX <int> [lsb=4,width=8] v52
which is much nicer for debugging.
Change-Id: I40713ff7f4a857c4557486cdf73c2dff137511ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/221420
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
SignExt32to64 can be implemented with a single ADDIW instruction, rather than
the two shifts that are in use currently.
Change-Id: Ie1bbaef4018f1ba5162773fc64fa5a887457cfc9
Reviewed-on: https://go-review.googlesource.com/c/go/+/220922
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Apply CL 40930 to src/cmd/gofmt/internal.go to bring
it into sync with src/go/format/internal.go.
Also revert '\n' back to "\n\n" in one of the comments,
because the previous text was more accurate.
Gofmt replaces the "; " part of "package p; func _() {"
input with two newline characters, not one.
Updates #11844
Change-Id: I6bb8155a931b793311991d3cd8e006a2931b167a
Reviewed-on: https://go-review.googlesource.com/c/go/+/221497
Reviewed-by: Robert Griesemer <gri@golang.org>
These detect opportunities to convert a rule to use an ellipsis,
and provide better error messages when something goes wrong.
This change was used to generate all the preceding changes
converting rules to use ellipses. This change is at the end of those
changes rather than the beginning in order to avoid log spam during rule
generation (say during a git bisection).
The preceding changes collectively shrink the cmd/compile binary by ~2.2%.
Part of this detection is also warning when the presence of an
unmentioned aux or auxint could cause conversion to an ellipsis
rule to change the sematics of the rule.
For example:
(Div64 x y) -> (DIV x y)
looks like a promising rule for an ellipsis. However, Div64 has an auxint,
and (on most platforms) DIV does not. An ellipsis rule would keep the
auxint intact, rather than zeroing it, which can infere with CSE.
So this change flags this rule as doing implicit zeroing;
it should be replaced by
(Div64 [a] x y) -> (DIV x y)
which makes it clear that the auxint is being zeroed.
This detection is not foolproof, but it currently has no false positives.
If false positives arise in the future, we will need to gate the output.
Change-Id: Ie21f284579e5d6e75aa304d0deb024d41ede528b
Reviewed-on: https://go-review.googlesource.com/c/go/+/217014
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
For example, 'go version -m' happily gives you Go's own version, even
though the -m flag only makes sense when grabbing the version of a
binary on disk.
Similarly, if any of the directly named files can't be found, the tool
would succeed. That's acceptable if an error is encountered while
walking a large directory, but not when locating a path directly given
by the user.
These added test cases run even in short mode, as 'go build' is not
needed for them.
Change-Id: I7bb40b72853799e31d9f86cc5e999c8d57813eef
Reviewed-on: https://go-review.googlesource.com/c/go/+/221397
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Plus a bonus optimization I noticed while working on this.
There are no functions (besides the rewrite rules) whose text size
increases as a result of this change.
Updates #21439
The following per-package text size stats were generated by parsing the
output of compiling with -S and summing the function size reported on the
STEXT line. This gives a far more accurate picture of the impact
on generated code than merely looking at the object file size changes
or the resulting binary size changes. The latter are below, for reference.
file before after Δ %
runtime.s 477257 476417 -840 -0.176%
math.s 35985 35976 -9 -0.025%
vendor/golang.org/x/net/dns/dnsmessage.s 87314 87232 -82 -0.094%
debug/dwarf.s 108444 108432 -12 -0.011%
regexp.s 64535 64467 -68 -0.105%
internal/xcoff.s 23175 22945 -230 -0.992%
cmd/vendor/golang.org/x/arch/arm/armasm.s 45263 45260 -3 -0.007%
cmd/vendor/golang.org/x/arch/arm64/arm64asm.s 118140 118135 -5 -0.004%
cmd/internal/obj/arm64.s 151502 151498 -4 -0.003%
cmd/compile/internal/ssa.s 6061483 6063120 +1637 +0.027%
total 9321728 9322112 +384 +0.004%
file before after Δ %
go 15188916 15184820 -4096 -0.027%
addr2line 4315984 4311888 -4096 -0.095%
cgo 4836088 4831992 -4096 -0.085%
compile 24506008 24493720 -12288 -0.050%
doc 4680952 4676856 -4096 -0.088%
link 6605336 6601240 -4096 -0.062%
pprof 14776756 14772660 -4096 -0.028%
total 135250956 135214092 -36864 -0.027%
Change-Id: I1243a098a08db452f7d1eb0998e241c9b199e2b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/213058
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The out method was renamed to Output in CL 70391 for #17628 and #21888.
Fixes#37514
Change-Id: I99be47b5030ccbbf10a056df9fcc3c97cb99b015
Reviewed-on: https://go-review.googlesource.com/c/go/+/221383
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Change-Id: I7e4827b3428b48c67060789a528586a8907ca3db
Reviewed-on: https://go-review.googlesource.com/c/go/+/221418
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Use ${:} instead of : so to be Windows-friendly.
Change-Id: I5e660af49b0a2ac2da868583cbcb55cf8bf89b68
Reviewed-on: https://go-review.googlesource.com/c/go/+/221461
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This rolls forward the change golang.org/cl/214431, which was reverted
in golang.org/cl/220217. The cl was broken because
TestVersionControlErrorMessageIncludesCorrectDirectory, which is going
to be removed in golang.org/cl/214429 hadn't been submitted yet.
Original change description:
Part of converting all tests to script framework to improve
test parallelism.
Updates #36320
Updates #17751
Change-Id: I87b3f9acb8575fbcbd58d454b5f9bac4923429b3
Reviewed-on: https://go-review.googlesource.com/c/go/+/220178
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This is a bit complex. There's a driver program to run go with modifications
to the GOPATH used to test Windows.
Also remove the cd method on testgoData, because this was the last function
that used it.
Part of converting all tests to script framework to improve
test parallelism.
Updates #36320
Updates #17751
Change-Id: I3e8e27f37fd3701bd36b6365b128dd73b69181c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/214578
Reviewed-by: Jay Conrod <jayconrod@google.com>
I think this test needs to be split up eventually. It's one of
the longest tests.
Part of converting all tests to script framework to improve
test parallelism.
Updates #36320
Updates #17751
Change-Id: If2168fac040d78fd0ec3dcbdef2affd2a8f48f6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/214158
Reviewed-by: Jay Conrod <jayconrod@google.com>
Part of converting all tests to script framework to improve
test parallelism.
Updates #36320
Updates #17751
Change-Id: Ib386838081abad8bc6b01c1f0a4656553d0b6ff3
Reviewed-on: https://go-review.googlesource.com/c/go/+/214579
Reviewed-by: Jay Conrod <jayconrod@google.com>
Part of converting all tests to script framework to improve
test parallelism.
Updates #36320
Updates #17751
Change-Id: I601e0fcee32b8c5bf2107b520d1dfbe12a19ad3f
Reviewed-on: https://go-review.googlesource.com/c/go/+/213223
Reviewed-by: Jay Conrod <jayconrod@google.com>
It looks like TestGoBuildGOPATHOrderBroken has been fixed so I've converted
that too, without the skip.
Part of converting all tests to script framework to improve
test parallelism.
Updates #36320
Updates #17751
Change-Id: I7ee77f22fb888811c175bcdc5eb814c80fbec420
Reviewed-on: https://go-review.googlesource.com/c/go/+/214432
Reviewed-by: Jay Conrod <jayconrod@google.com>
Part of converting all tests to script framework to improve
test parallelism.
Updates #36320
Updates #17751
Change-Id: I394844da1ffc0dcde7f5862c41ed8efa7c5ca088
Reviewed-on: https://go-review.googlesource.com/c/go/+/214429
Reviewed-by: Jay Conrod <jayconrod@google.com>
Part of converting all tests to script framework to improve
test parallelism.
Updates #36320
Updates #17751
Change-Id: I9a99aa5d37300c83a2f95fb906949cb4c1d5356f
Reviewed-on: https://go-review.googlesource.com/c/go/+/214426
Reviewed-by: Jay Conrod <jayconrod@google.com>
Part of converting all tests to script framework to improve
test parallelism.
Updates #36320
Updates #17751
Change-Id: If1e591f28d6399a07b37ed7f4a1419bf7cd915eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/214425
Reviewed-by: Jay Conrod <jayconrod@google.com>
The name of the test is too long to fit on the first line. It's
TestGoTestBuildsAnXtestContainingOnlyNonRunnableExamples.
Part of converting all tests to script framework to improve
test parallelism.
Updates #36320
Updates #17751
Change-Id: I490748271b10a85cbe1d34f9dbecb86ccf0101a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/214423
Reviewed-by: Jay Conrod <jayconrod@google.com>
Part of converting all tests to script framework to improve
test parallelism.
Updates #36320
Updates #17751
Change-Id: I1020feaa4ddb40ff52c46728bc4973cea4c7b066
Reviewed-on: https://go-review.googlesource.com/c/go/+/214391
Reviewed-by: Jay Conrod <jayconrod@google.com>
Instead of writing AuxInt during prove and then zeroing it during lower,
just don't write it in the first place.
Passes toolstash-check -all.
Change-Id: Iea4b555029a9d69332e835536f9cf3a42b8223db
Reviewed-on: https://go-review.googlesource.com/c/go/+/220682
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
We were assigning a cost of 2 for intrinsics:
One when we recognized an intrinsic,
and one for the OCALLFUNC node.
I believe that the intent was that intrinsics should
cost 1, since they are typically an arithmetic op,
and because they tend to occur in performance-sensitive code.
(Not that any of this is particularly principled right now.)
Stop charging when we recognize an intrinsic;
let the OCALLFUNC node cover the cost.
This has a negligible impact on std+cmd.
Change-Id: Ie6ae5c18da3ae7e40aec425aed95c6999c2831a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/221357
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Also, avoid string(i) where i has type int.
Updates #32479
Change-Id: If3c6edc8523860082726e034ef9e887b5f7fabd1
Reviewed-on: https://go-review.googlesource.com/c/go/+/221382
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL changes some unit test functions, making sure that these tests (and goroutines spawned during test) won't block.
Since they are just test functions, I use one CL to fix them all. I hope this won't cause trouble to reviewers and can save time for us.
There are three main categories of incorrect logic fixed by this CL:
1. Use testing.Fatal()/Fatalf() in spawned goroutines, which is forbidden by Go's document.
2. Channels are used in such a way that, when errors or timeout happen, the test will be blocked and never return.
3. Channels are used in such a way that, when errors or timeout happen, the test can return but some spawned goroutines will be leaked, occupying resource until all other tests return and the process is killed.
Change-Id: I3df931ec380794a0cf1404e632c1dd57c65d63e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/219380
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If we see a racy use of timers, as in concurrent calls to Timer.Reset,
do the operations in an unpredictable order, rather than crashing.
Fixes#37400
Change-Id: Idbac295df2dfd551b6d762909d5040fc532c1b34
Reviewed-on: https://go-review.googlesource.com/c/go/+/221077
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Now that android/arm64 supports internal linking PIE, enable the
test. While here, I realized that some cgo tests are also not
enabled on android/arm64. Enable them as well. Let's see if it
works.
Change-Id: Ibf186fe402ebf0bbec82873fd56d0eb752b48180
Reviewed-on: https://go-review.googlesource.com/c/go/+/221099
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Elias Naur <mail@eliasnaur.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It was enabled in CL 207877, but then accidentally disabled in
CL 207299 due to a bad rebase. Reenable.
Change-Id: I147bf724a4263d4aae54576a36cc7e1cad5e8a2c
Reviewed-on: https://go-review.googlesource.com/c/go/+/221098
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Elias Naur <mail@eliasnaur.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The SSA backend has rules to read the contents of readonly Lsyms.
However, this rule was failing to trigger for many readonly Lsyms.
This is because the readonly attribute that was set on the Node.Name
was not propagated to its Lsym until the dump globals phase, after SSA runs.
To work around this phase ordering problem, introduce Node.SetReadonly,
which sets Node.Name.Readonly and also configures the Lsym
enough that SSA can use it.
This change also fixes a latent problem in the rewrite rule function,
namely that reads past the end of lsym.P were treated as entirely zero,
instead of merely requiring padding with trailing zeros.
This change also adds an amd64 rule needed to fully optimize
the results of this change. It would be better not to need this,
but the zero extension that should handle this for us
gets optimized away too soon (see #36897 for a similar problem).
I have not investigated whether other platforms also need new
rules to take full advantage of the new optimizations.
Compiled code for (interface{})(true) on amd64 goes from:
LEAQ type.bool(SB), AX
MOVBLZX ""..stmp_0(SB), BX
LEAQ runtime.staticbytes(SB), CX
ADDQ CX, BX
to
LEAQ type.bool(SB), AX
LEAQ runtime.staticbytes+1(SB), BX
Prior to this change, the readonly symbol rewrite rules
fired a total of 884 times during make.bash.
Afterwards they fire 1807 times.
file before after Δ %
cgo 4827832 4823736 -4096 -0.085%
compile 24907768 24895656 -12112 -0.049%
fix 3376952 3368760 -8192 -0.243%
pprof 14751700 14747604 -4096 -0.028%
total 120343528 120315032 -28496 -0.024%
Change-Id: I59ea52138276c37840f69e30fb109fd376d579ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/220499
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
There are often many values to clobber.
Allow passing them all in at once.
The goal is increased rule readability.
As a bonus, it shrinks cmd/compile by ~97k, almost half a percent.
Package SSA requires 1.2% less memory to compile.
The single-line changes were make via regex,
and the remaining multi-line clobbers were manually combined.
Passes toolstash-check -all.
Change-Id: Ib310e9265d3616211f8192c9040b4c8933824d19
Reviewed-on: https://go-review.googlesource.com/c/go/+/220691
Reviewed-by: Michael Munday <mike.munday@ibm.com>
According to https://golang.org/wiki/NetBSD, NetBSD 7.0 is supported as
of Go 1.3 (with Go 1.5 recommended). NetBSD 6.0 was last supported in Go
1.9.7. Thus, bump the minimal ABI version to NetBSD 7.0
Suggested by Benny Siegert in CL 212461.
Change-Id: I11dd14c6b835ac9fc156880b2551f71893c7f267
Reviewed-on: https://go-review.googlesource.com/c/go/+/220428
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
These rules fire in particular when comparing to a constant
string of length two. They should trigger even more after CL 220499.
file before after Δ %
compile 20639976 20635880 -4096 -0.020%
total 116003456 115999360 -4096 -0.004%
Change-Id: I21c1c02cf32d710d7a4eb12efab00f02796ccb84
Reviewed-on: https://go-review.googlesource.com/c/go/+/220694
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Slicemask can be performed with three immediate instructions, rather than the
six currently in use.
Change-Id: I3f8ca2d5affd1403db8fa79b356f248e6e9332c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/220923
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Use SUBW to perform a 32-bit subtraction, rather than zero extending from
32 to 64 bits. This reduces Eq32 and Neq32 to two instructions, rather than
the four instructions required previously.
Change-Id: Ib2798324881e9db842c864e91a0c1b1e48c4b67b
Reviewed-on: https://go-review.googlesource.com/c/go/+/220921
Reviewed-by: Cherry Zhang <cherryyz@google.com>
