1
0
mirror of https://github.com/golang/go synced 2024-11-19 21:54:40 -07:00
Commit Graph

2824 Commits

Author SHA1 Message Date
Cherry Zhang
f33f20ef1f test: fix and re-enable nosplit.go
The test was skipped because it did not work on AMD64 with
frame pointer enabled, and accidentally skipped on other
architectures. Now frame pointer is the default on AMD64.
Update the test to work with frame pointer. Now the test
is skipped only when frame pointer is NOT enabled on AMD64.

Fixes #18317.

Change-Id: I724cb6874e562f16e67ce5f389a1d032a2003115
Reviewed-on: https://go-review.googlesource.com/68610
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2017-10-31 19:12:11 +00:00
Russ Cox
84dc501d20 test/run: use go tool compile + link instead of go run when possible
This cuts 6 seconds off all.bash with the new go command.
Not a ton, but also an easy 6 seconds to grab.

The -tags=use_go_run in the misc/cgo tests is just some
go command flag that will make run.go use go run,
but without making everything look stale.
(Those tests have relative imports,
so go tool compile+link is not enough.)

Change-Id: I43bf4bb661d3adde2b2d4aad5e8f64b97bc69ba9
Reviewed-on: https://go-review.googlesource.com/73994
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-31 13:21:05 +00:00
Russ Cox
7dea509703 cmd/go: switch to entirely content-based staleness determination
This CL changes the go command to base all its rebuilding decisions
on the content of the files being processed and not their file system
modification times. It also eliminates the special handling of release
toolchains, which were previously considered always up-to-date
because modification time order could not be trusted when unpacking
a pre-built release.

The go command previously tracked "build IDs" as a backup to
modification times, to catch changes not reflected in modification times.
For example, if you remove one .go file in a package with multiple .go
files, there is no modification time remaining in the system that indicates
that the installed package is out of date. The old build ID was the hash
of a list of file names and a few other factors, expected to change if
those factors changed.

This CL moves to using this kind of build ID as the only way to
detect staleness, making sure that the build ID hash includes all
possible factors that need to influence the rebuild decision.

One such factor is the compiler flags. As of this CL, if you run

	go build -gcflags -N cmd/gofmt

you will get a gofmt where every package is built with -N,
regardless of what may or may not be installed already.

Another such factor is the linker flags. As of this CL, if you run

	go install myprog
	go install -ldflags=-s myprog

the second go install will now correctly build a new myprog with
the updated linker flags. (Previously the installed myprog appeared
up-to-date, because the ldflags were not included in the build ID.)

Because we have more precise information we can also validate whether
the target of a "go test -c" operation is already the right binary and
therefore can avoid a rebuild.

This CL sets us up for having a more general build artifact cache,
maybe even a step toward not having a pkg directory with .a files,
but this CL does not take that step. For now the result of go install
is the same as it ever was; we just do a better job of what needs to
be installed.

This CL does slow down builds a small amount by reading all the
dependent source files in full. (The go command already read the
beginning of every dependent source file to discover build tags
and imports.) On my MacBook Pro, before this CL all.bash takes
3m58s, while after this CL and a few optimizations stacked above it
all.bash takes 4m28s. Given that CL 73850 cut 1m43s off the all.bash
time earlier today, we can afford adding 30s back for now.
More optimizations are planned that should make the go command
more efficient than it was even before this CL.

Fixes #15799.
Fixes #18369.
Fixes #19340.
Fixes #21477.

Change-Id: I10d7ca0e31ca3f58aabb9b1f11e2e3d9d18f0bc9
Reviewed-on: https://go-review.googlesource.com/73212
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2017-10-31 13:19:38 +00:00
Russ Cox
2beb173e98 all: respect $GO_GCFLAGS during run.bash
If the go install doesn't use the same flags as the main build
it can overwrite the installed standard library, leading to
flakiness and slow future tests.

Force uses of 'go install' etc to propagate $GO_GCFLAGS
or disable them entirely, to avoid problems.

As I understand it, the main place this happens is the ssacheck builder.
If there are other uses that need to run some of the now-disabled
tests we can reenable fixed tests in followup CLs.

Change-Id: Ib860a253539f402f8a96a3c00ec34f0bbf137c9a
Reviewed-on: https://go-review.googlesource.com/74470
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2017-10-31 13:19:15 +00:00
David Chase
b4c3fe7b04 cmd/compile: adjust expectations of test for issue 18902
The test for #18902 reads the assembly stream to be sure
that the line number does not change too often (this is an
indication that debugging the code will be unpleasant and
that the compiler is probably getting line numbers "wrong").

It checks that it is getting "enough" input, but the
compiler has gotten enough better since the test was written
that it now fails for lack of enough input.  The old
threshould was 200 instructions, the new one is 150 (the
minimum observed input is on arm64 with 184 instructions).

Fixes #22494.

Change-Id: Ibba7e9ff4ab6a7be369e5dd5859d150b7db94653
Reviewed-on: https://go-review.googlesource.com/74357
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
2017-10-30 20:27:09 +00:00
Keith Randall
0153a4130d cmd/compile: fix runtime.KeepAlive
KeepAlive needs to introduce a use of the spill of the
value it is keeping alive.  Without that, we don't guarantee
that the spill dominates the KeepAlive.

This bug was probably introduced with the code to move spills
down to the dominator of the restores, instead of always spilling
just after the value itself (CL 34822).

Fixes #22458.

Change-Id: I94955a21960448ffdacc4df775fe1213967b1d4c
Reviewed-on: https://go-review.googlesource.com/74210
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
2017-10-30 19:55:02 +00:00
Austin Clements
7e343134d3 cmd/compile: compiler support for buffered write barrier
This CL implements the compiler support for calling the buffered write
barrier added by the previous CL.

Since the buffered write barrier is only implemented on amd64 right
now, this still supports the old, eager write barrier as well. There's
little overhead to supporting both and this way a few tests in
test/fixedbugs that expect to have liveness maps at write barrier
calls can easily opt-in to the old, eager barrier.

This significantly improves the performance of the write barrier:

name             old time/op  new time/op  delta
WriteBarrier-12  73.5ns ±20%  19.2ns ±27%  -73.90%  (p=0.000 n=19+18)

It also reduces the size of binaries because the write barrier call is
more compact:

name        old object-bytes  new object-bytes  delta
Template           398k ± 0%         393k ± 0%  -1.14%  (p=0.008 n=5+5)
Unicode            208k ± 0%         206k ± 0%  -1.00%  (p=0.008 n=5+5)
GoTypes           1.18M ± 0%        1.15M ± 0%  -2.00%  (p=0.008 n=5+5)
Compiler          4.05M ± 0%        3.88M ± 0%  -4.26%  (p=0.008 n=5+5)
SSA               8.25M ± 0%        8.11M ± 0%  -1.59%  (p=0.008 n=5+5)
Flate              228k ± 0%         224k ± 0%  -1.83%  (p=0.008 n=5+5)
GoParser           295k ± 0%         284k ± 0%  -3.62%  (p=0.008 n=5+5)
Reflect           1.00M ± 0%        0.99M ± 0%  -0.70%  (p=0.008 n=5+5)
Tar                339k ± 0%         333k ± 0%  -1.67%  (p=0.008 n=5+5)
XML                404k ± 0%         395k ± 0%  -2.10%  (p=0.008 n=5+5)
[Geo mean]         704k              690k       -2.00%

name        old exe-bytes     new exe-bytes     delta
HelloSize         1.05M ± 0%        1.04M ± 0%  -1.55%  (p=0.008 n=5+5)

https://perf.golang.org/search?q=upload:20171027.1

(Amusingly, this also reduces compiler allocations by 0.75%, which,
combined with the better write barrier, speeds up the compiler overall
by 2.10%. See the perf link.)

It slightly improves the performance of most of the go1 benchmarks and
improves the performance of the x/benchmarks:

name                      old time/op    new time/op    delta
BinaryTree17-12              2.40s ± 1%     2.47s ± 1%  +2.69%  (p=0.000 n=19+19)
Fannkuch11-12                2.95s ± 0%     2.95s ± 0%  +0.21%  (p=0.000 n=20+19)
FmtFprintfEmpty-12          41.8ns ± 4%    41.4ns ± 2%  -1.03%  (p=0.014 n=20+20)
FmtFprintfString-12         68.7ns ± 2%    67.5ns ± 1%  -1.75%  (p=0.000 n=20+17)
FmtFprintfInt-12            79.0ns ± 3%    77.1ns ± 1%  -2.40%  (p=0.000 n=19+17)
FmtFprintfIntInt-12          127ns ± 1%     123ns ± 3%  -3.42%  (p=0.000 n=20+20)
FmtFprintfPrefixedInt-12     152ns ± 1%     150ns ± 1%  -1.02%  (p=0.000 n=18+17)
FmtFprintfFloat-12           211ns ± 1%     209ns ± 0%  -0.99%  (p=0.000 n=20+16)
FmtManyArgs-12               500ns ± 0%     496ns ± 0%  -0.73%  (p=0.000 n=17+20)
GobDecode-12                6.44ms ± 1%    6.53ms ± 0%  +1.28%  (p=0.000 n=20+19)
GobEncode-12                5.46ms ± 0%    5.46ms ± 1%    ~     (p=0.550 n=19+20)
Gzip-12                      220ms ± 1%     216ms ± 0%  -1.75%  (p=0.000 n=19+19)
Gunzip-12                   38.8ms ± 0%    38.6ms ± 0%  -0.30%  (p=0.000 n=18+19)
HTTPClientServer-12         79.0µs ± 1%    78.2µs ± 1%  -1.01%  (p=0.000 n=20+20)
JSONEncode-12               11.9ms ± 0%    11.9ms ± 0%  -0.29%  (p=0.000 n=20+19)
JSONDecode-12               52.6ms ± 0%    52.2ms ± 0%  -0.68%  (p=0.000 n=19+20)
Mandelbrot200-12            3.69ms ± 0%    3.68ms ± 0%  -0.36%  (p=0.000 n=20+20)
GoParse-12                  3.13ms ± 1%    3.18ms ± 1%  +1.67%  (p=0.000 n=19+20)
RegexpMatchEasy0_32-12      73.2ns ± 1%    72.3ns ± 1%  -1.19%  (p=0.000 n=19+18)
RegexpMatchEasy0_1K-12       241ns ± 0%     239ns ± 0%  -0.83%  (p=0.000 n=17+16)
RegexpMatchEasy1_32-12      68.6ns ± 1%    69.0ns ± 1%  +0.47%  (p=0.015 n=18+16)
RegexpMatchEasy1_1K-12       364ns ± 0%     361ns ± 0%  -0.67%  (p=0.000 n=16+17)
RegexpMatchMedium_32-12      104ns ± 1%     103ns ± 1%  -0.79%  (p=0.001 n=20+15)
RegexpMatchMedium_1K-12     33.8µs ± 3%    34.0µs ± 2%    ~     (p=0.267 n=20+19)
RegexpMatchHard_32-12       1.64µs ± 1%    1.62µs ± 2%  -1.25%  (p=0.000 n=19+18)
RegexpMatchHard_1K-12       49.2µs ± 0%    48.7µs ± 1%  -0.93%  (p=0.000 n=19+18)
Revcomp-12                   391ms ± 5%     396ms ± 7%    ~     (p=0.154 n=19+19)
Template-12                 63.1ms ± 0%    59.5ms ± 0%  -5.76%  (p=0.000 n=18+19)
TimeParse-12                 307ns ± 0%     306ns ± 0%  -0.39%  (p=0.000 n=19+17)
TimeFormat-12                325ns ± 0%     323ns ± 0%  -0.50%  (p=0.000 n=19+19)
[Geo mean]                  47.3µs         46.9µs       -0.67%

https://perf.golang.org/search?q=upload:20171026.1

name                       old time/op  new time/op  delta
Garbage/benchmem-MB=64-12  2.25ms ± 1%  2.20ms ± 1%  -2.31%  (p=0.000 n=18+18)
HTTP-12                    12.6µs ± 0%  12.6µs ± 0%  -0.72%  (p=0.000 n=18+17)
JSON-12                    11.0ms ± 0%  11.0ms ± 1%  -0.68%  (p=0.000 n=17+19)

https://perf.golang.org/search?q=upload:20171026.2

Updates #14951.
Updates #22460.

Change-Id: Id4c0932890a1d41020071bec73b8522b1367d3e7
Reviewed-on: https://go-review.googlesource.com/73712
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-10-30 18:12:46 +00:00
Austin Clements
b78b54ff83 cmd/compile: elide write barriers for copy of notinheap pointers
Currently copy and append for types containing only scalars and
notinheap pointers still get compiled to have write barriers, even
though those write barriers are unnecessary. Fix these to use
HasHeapPointer instead of just Haspointer so that they elide write
barriers when possible.

This fixes the unnecessary write barrier in runtime.recordspan when it
grows the h.allspans slice. This is important because recordspan gets
called (*very* indirectly) from (*gcWork).tryGet, which is
go:nowritebarrierrec. Unfortunately, the compiler's analysis has no
hope of seeing this because it goes through the indirect call
fixalloc.first, but I saw it happen.

Change-Id: Ieba3abc555a45f573705eab780debcfe5c4f5dd1
Reviewed-on: https://go-review.googlesource.com/73413
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-29 20:21:49 +00:00
Austin Clements
316e3036a7 cmd/compile: make HasHeapPointer recursive
Currently (*Type).HasHeapPointer only ignores pointers go:notinheap
types if the type itself is a pointer to a go:notinheap type. However,
if it's some other type that contains pointers where all of those
pointers are go:notinheap, it will conservatively return true. As a
result, we'll use write barriers where they aren't needed, for example
calling typedmemmove instead of just memmove on structs that contain
only go:notinheap pointers.

Fix this by making HasHeapPointer walk the whole type looking for
pointers that aren't marked go:notinheap.

Change-Id: Ib8c6abf6f7a20f34969d1d402c5498e0b990be59
Reviewed-on: https://go-review.googlesource.com/73412
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-29 20:21:47 +00:00
Austin Clements
5a4b6bce37 cmd/compile: improve coverage of nowritebarrierrec check
The current go:nowritebarrierrec checker has two problems that limit
its coverage:

1. It doesn't understand that systemstack calls its argument, which
means there are several cases where we fail to detect prohibited write
barriers.

2. It only observes calls in the AST, so calls constructed during
lowering by SSA aren't followed.

This CL completely rewrites this checker to address these issues.

The current checker runs entirely after walk and uses visitBottomUp,
which introduces several problems for checking across systemstack.
First, visitBottomUp itself doesn't understand systemstack calls, so
the callee may be ordered after the caller, causing the checker to
fail to propagate constraints. Second, many systemstack calls are
passed a closure, which is quite difficult to resolve back to the
function definition after transformclosure and walk have run. Third,
visitBottomUp works exclusively on the AST, so it can't observe calls
created by SSA.

To address these problems, this commit splits the check into two
phases and rewrites it to use a call graph generated during SSA
lowering. The first phase runs before transformclosure/walk and simply
records systemstack arguments when they're easy to get. Then, it
modifies genssa to record static call edges at the point where we're
lowering to Progs (which is the latest point at which position
information is conveniently available). Finally, the second phase runs
after all functions have been lowered and uses a direct BFS walk of
the call graph (combining systemstack calls with static calls) to find
prohibited write barriers and construct nice error messages.

Fixes #22384.
For #22460.

Change-Id: I39668f7f2366ab3c1ab1a71eaf25484d25349540
Reviewed-on: https://go-review.googlesource.com/72773
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-29 19:36:44 +00:00
Ian Lance Taylor
6222997047 test: add type alias test that caused gccgo to crash
Change-Id: I3b388e4ac05ace5b7768ade03df2bee5bcc26ba8
Reviewed-on: https://go-review.googlesource.com/73790
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
2017-10-26 22:52:53 +00:00
griesemer
c74712b3b2 test: add test cases for method expressions with literal receiver types
For #9060.

Change-Id: Ibd0f047083f3c98cec96c655a3e2024df8e1d2a0
Reviewed-on: https://go-review.googlesource.com/73554
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-26 03:47:37 +00:00
Keith Randall
40649e6979 cmd/compile: make sure not to use SP as an index register
...because that's an illegal addressing mode.

I double-checked handling of this code, and 387 is the only
place where this check is missing.

Fixes #22429

Change-Id: I2284fe729ea86251c6af2f04076ddf7a5e66367c
Reviewed-on: https://go-review.googlesource.com/73551
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-26 02:45:05 +00:00
Matthew Dempsky
efa9efe8e4 cmd/compile: silence unnecessary unsafe error
If n.Type==nil after typechecking, then we should have already
reported a more useful error somewhere else. Just return 0 in
evalunsafe without trying to do anything else that's likely to cause
problems.

Also, further split out issue7525.go into more test files, because
cmd/compile reports at most one typechecking loop per compilation
unit.

Fixes #22351.

Change-Id: I3ebf505f72c48fcbfef5ec915606224406026597
Reviewed-on: https://go-review.googlesource.com/72251
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-10-24 02:28:02 +00:00
Dhananjay Nakrani
4c8e8fc301 cmd/compile: fix segfault in race instrumentation
Fixes #13265.

Change-Id: I792eb4ee26bef8a56e279e23f9802cb39019e0d0
Reviewed-on: https://go-review.googlesource.com/34929
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-10-18 16:46:18 +00:00
griesemer
0b2cb89196 cmd/compile/internal/syntax: better recovery after missing closing parentheses
Fine-tune skipping of tokens after missing closing parentheses in lists.

Fixes #22164.

Change-Id: I575d86e21048cd40340a2c08399e8b0deec337cf
Reviewed-on: https://go-review.googlesource.com/71250
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-17 01:04:56 +00:00
Martin Möhrmann
743117a85e cmd/compile: simplify slice/array range loops for some element sizes
In range loops over slices and arrays besides a variable to track the
index an extra variable containing the address of the current element
is used. To compute a pointer to the next element the elements size is
added to the address.

On 386 and amd64 an element of size 1, 2, 4 or 8 bytes can by copied
from an array using a MOV instruction with suitable addressing mode
that uses the start address of the array, the index of the element and
element size as scaling factor. Thereby, for arrays and slices with
suitable element size we can avoid keeping and incrementing an extra
variable to compute the next elements address.

Shrinks cmd/go by 4 kilobytes.

AMD64:
name                   old time/op    new time/op    delta
BinaryTree17              2.66s ± 7%     2.54s ± 0%  -4.53%  (p=0.000 n=10+8)
Fannkuch11                3.02s ± 1%     3.02s ± 1%    ~     (p=0.579 n=10+10)
FmtFprintfEmpty          45.6ns ± 1%    42.2ns ± 1%  -7.46%  (p=0.000 n=10+10)
FmtFprintfString         69.8ns ± 1%    70.4ns ± 1%  +0.84%  (p=0.041 n=10+10)
FmtFprintfInt            80.1ns ± 1%    79.0ns ± 1%  -1.35%  (p=0.000 n=10+10)
FmtFprintfIntInt          127ns ± 1%     125ns ± 1%  -1.00%  (p=0.007 n=10+9)
FmtFprintfPrefixedInt     158ns ± 2%     152ns ± 1%  -4.11%  (p=0.000 n=10+10)
FmtFprintfFloat           218ns ± 1%     214ns ± 1%  -1.61%  (p=0.000 n=10+10)
FmtManyArgs               508ns ± 1%     504ns ± 1%  -0.93%  (p=0.001 n=9+10)
GobDecode                6.76ms ± 1%    6.78ms ± 1%    ~     (p=0.353 n=10+10)
GobEncode                5.84ms ± 1%    5.77ms ± 1%  -1.31%  (p=0.000 n=10+9)
Gzip                      223ms ± 1%     218ms ± 1%  -2.39%  (p=0.000 n=10+10)
Gunzip                   40.3ms ± 1%    40.4ms ± 3%    ~     (p=0.796 n=10+10)
HTTPClientServer         73.5µs ± 0%    73.3µs ± 0%  -0.28%  (p=0.000 n=10+9)
JSONEncode               12.7ms ± 1%    12.6ms ± 8%    ~     (p=0.173 n=8+10)
JSONDecode               57.5ms ± 1%    56.1ms ± 2%  -2.40%  (p=0.000 n=10+10)
Mandelbrot200            3.80ms ± 1%    3.86ms ± 6%    ~     (p=0.579 n=10+10)
GoParse                  3.25ms ± 1%    3.23ms ± 1%    ~     (p=0.052 n=10+10)
RegexpMatchEasy0_32      74.4ns ± 1%    76.9ns ± 1%  +3.39%  (p=0.000 n=10+10)
RegexpMatchEasy0_1K       243ns ± 2%     248ns ± 1%  +1.86%  (p=0.000 n=10+8)
RegexpMatchEasy1_32      71.0ns ± 2%    72.8ns ± 1%  +2.55%  (p=0.000 n=10+10)
RegexpMatchEasy1_1K       370ns ± 1%     383ns ± 0%  +3.39%  (p=0.000 n=10+9)
RegexpMatchMedium_32      107ns ± 0%     113ns ± 1%  +5.33%  (p=0.000 n=6+10)
RegexpMatchMedium_1K     35.0µs ± 1%    36.0µs ± 1%  +3.13%  (p=0.000 n=10+10)
RegexpMatchHard_32       1.65µs ± 1%    1.69µs ± 1%  +2.23%  (p=0.000 n=10+9)
RegexpMatchHard_1K       49.8µs ± 1%    50.6µs ± 1%  +1.59%  (p=0.000 n=10+10)
Revcomp                   398ms ± 1%     396ms ± 1%  -0.51%  (p=0.043 n=10+10)
Template                 63.4ms ± 1%    60.8ms ± 0%  -4.11%  (p=0.000 n=10+9)
TimeParse                 318ns ± 1%     322ns ± 1%  +1.10%  (p=0.005 n=10+10)
TimeFormat                323ns ± 1%     336ns ± 1%  +4.15%  (p=0.000 n=10+10)

Updates: #15809.

Change-Id: I55915aaf6d26768e12247f8a8edf14e7630726d1
Reviewed-on: https://go-review.googlesource.com/38061
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
2017-10-13 14:52:04 +00:00
Tobias Klauser
fd9fac2c7c cmd/compile: add two error position tests for the typechecker
Follow CL 41477 and add two more line position tests for yyerror calls
in the typechecker which are currently not tested.

Update #19683

Change-Id: Iacd865195a3bfba87d8c22655382af267aba47a9
Reviewed-on: https://go-review.googlesource.com/70251
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
2017-10-12 20:50:20 +00:00
Ian Lance Taylor
56dec8dde2 test: add test case that gccgo miscompiled
Error was

main.go:7:11: error: import error at 162: expected ‘<type ’

Change-Id: Iacfe4bfa003d7708a21ebc89ad1ab2d4a3b041a8
Reviewed-on: https://go-review.googlesource.com/70290
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
2017-10-12 19:02:09 +00:00
Matthew Dempsky
a509cae90d cmd/compile: record InlCost in export data
Previously, we were treating cross-package function calls as free for
inlining budgeting.

In theory, we should be able to recompute InlCost from the
exported/reimported function bodies. However, that process mutates the
structure of the Node AST enough that it doesn't preserve InlCost. To
avoid unexpected issues, just record and restore InlCost in the export
data.

Fixes #19261.

Change-Id: Iac2bc0d32d4f948b64524aca657051f9fc96d92d
Reviewed-on: https://go-review.googlesource.com/70151
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-10-11 23:20:44 +00:00
Hugues Bruant
4f70a2a699 cmd/compile: inline calls to local closures
Calls to a closure held in a local, non-escaping,
variable can be inlined, provided the closure body
can be inlined and the variable is never written to.

The current implementation has the following limitations:

 - closures with captured variables are not inlined because
   doing so naively triggers invariant violation in the SSA
   phase
 - re-assignment check is currently approximated by checking
   the Addrtaken property of the variable which should be safe
   but may miss optimization opportunities if the address is
   not used for a write before the invocation

Updates #15561

Change-Id: I508cad5d28f027bd7e933b1f793c14dcfef8b5a1
Reviewed-on: https://go-review.googlesource.com/65071
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hugues Bruant <hugues.bruant@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
2017-10-11 22:32:36 +00:00
Cherry Zhang
70576947fd test: skip issue22200b.go on mipsle
It should be skipped on 32-bit architectures.

Change-Id: If7a64b9e90e47c3e8734dd62729bfd2944ae926c
Reviewed-on: https://go-review.googlesource.com/70071
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
2017-10-11 20:37:23 +00:00
Keith Randall
e130dcf051 cmd/compile: abort earlier if stack frame too large
If the stack frame is too large, abort immediately.
We used to generate code first, then abort.
In issue 22200, generating code raised a panic
so we got an ICE instead of an error message.

Change the max frame size to 1GB (from 2GB).
Stack frames between 1.1GB and 2GB didn't used to work anyway,
the pcln table generation would have failed and generated an ICE.

Fixes #22200

Change-Id: I1d918ab27ba6ebf5c87ec65d1bccf973f8c8541e
Reviewed-on: https://go-review.googlesource.com/69810
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-11 18:24:13 +00:00
Keith Randall
624630b824 cmd/compile: fold constant comparisions into SETxxmem ops.
Fixes #22198

Change-Id: I5cb91c73069af8b16a2580d28756efd58c84b690
Reviewed-on: https://go-review.googlesource.com/69990
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-11 18:23:09 +00:00
griesemer
68e390304e cmd/compile/internal/syntax: consider function nesting for error recovery
This re-enables functionality that inadvertently was disabled in the
(long) past.

Also, don't perform branch checks if we had errors in a function
to avoid spurious errors or (worst-case) crashes.

Slightly modified test/fixedbugs/issue14006.go to make sure the
test still reports invalid label errors (the surrounding function
must be syntactically correct).

Change-Id: Id5642930877d7cf3400649094ec75c753b5084b7
Reviewed-on: https://go-review.googlesource.com/69770
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-10-11 00:29:58 +00:00
Cherry Zhang
d63de28711 cmd/compile: intrinsify atomics on MIPS64
Change-Id: Ica65b7a52af9558a05d0a0e1dff0f9ec838f4117
Reviewed-on: https://go-review.googlesource.com/68830
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
2017-10-10 19:43:38 +00:00
Matthew Dempsky
f58c6c9915 cmd/compile: remove outdated TODO about inlining
We've supported inlining methods called as functions for a while now.

Change-Id: I53fba426e45f91d65a38f00456c2ae1527372b50
Reviewed-on: https://go-review.googlesource.com/69530
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-10-10 17:53:22 +00:00
griesemer
b77d9fe0ea cmd/compile: better error message for assignment mismatches
Keep left-to-right order when referring to the number of
variables and values involved.

Fixes #22159.

Change-Id: Iccca12d3222f9d5e049939a9ccec07513c393faa
Reviewed-on: https://go-review.googlesource.com/68690
Reviewed-by: Russ Cox <rsc@golang.org>
2017-10-06 16:35:44 +00:00
Cherry Zhang
a92a77c56f cmd/internal/obj/arm64: fix handling of unaligned offset between 256 and 504
C_PPAUTO was matching offsets that is a multiple 8. But this
condition is dropped in CL 55610, causing unaligned offset
between 256 and 504 mistakenly matched to some classes, e.g.
C_UAUTO8K. This CL restores this condition, also fixes an
error that C_PPAUTO shouldn't match C_PSAUTO, because the
latter is not guaranteed to be multiple of 8. C_PPAUTO_8 is
unnecessary, removed.

Fixes #21992.

Change-Id: I75d5a0e5f5dc3dae335721fbec1bbcd4a3b862f2
Reviewed-on: https://go-review.googlesource.com/65730
Reviewed-by: David Chase <drchase@google.com>
2017-10-05 22:28:17 +00:00
Matthew Dempsky
f22ef70254 cmd/compile: allow := to shadow dot-imported names
Historically, gc optimistically parsed the left-hand side of
assignments as expressions. Later, if it discovered a ":=" assignment,
it rewrote the parsed expressions as declarations.

This failed in the presence of dot imports though, because we lost
information about whether an imported object was named via a bare
identifier "Foo" or a normal qualified "pkg.Foo".

This CL fixes the issue by specially noding the left-hand side of ":="
assignments.

Fixes #22076.

Change-Id: I18190ecdb863112e7d009e1687e6112eec559921
Reviewed-on: https://go-review.googlesource.com/66810
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-10-05 18:07:37 +00:00
Keith Randall
41eabc0fc7 cmd/compile: fix merge rules for panic calls
Use entire inlining call stack to decide whether two panic calls
can be merged. We used to merge panic calls when only the leaf
line numbers matched, but that leads to places higher up the call
stack being merged incorrectly.

Fixes #22083

Change-Id: Ia41400a80de4b6ecf3e5089abce0c42b65e9b38a
Reviewed-on: https://go-review.googlesource.com/67632
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-10-03 09:27:37 +00:00
Daniel Martí
39e523792e cmd/compile: fix another invalid switch case panic
Very similar fix to the one made in golang.org/cl/65655. This time it's
for switches on interface values, as we look for duplicates in a
different manner to keep types in mind.

As before, add a small regression test.

Updates #22001.
Fixes #22063.

Change-Id: I9a55d08999aeca262ad276b4649b51848a627b02
Reviewed-on: https://go-review.googlesource.com/66450
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-09-27 21:19:06 +00:00
Keith Randall
3f04db41a8 cmd/compile: fix sign-extension merging rules
If we have

  y = <int16> (MOVBQSX x)
  z = <int32> (MOVWQSX y)

We used to use this rewrite rule:

(MOVWQSX x:(MOVBQSX _)) -> x

But that resulted in replacing z with a value whose type
is only int16.  Then if z is spilled and restored, it gets
zero extended instead of sign extended.

Instead use the rule

(MOVWQSX (MOVBQSX x)) -> (MOVBQSX x)

The result is has the correct type, so it can be spilled
and restored correctly.  It might mean that a few more extension
ops might not be eliminated, but that's the price for correctness.

Fixes #21963

Change-Id: I6ec82c3d2dbe43cc1fee6fb2bd6b3a72fca3af00
Reviewed-on: https://go-review.googlesource.com/65290
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-09-26 16:24:08 +00:00
Anfernee Yongkun Gui
4cff104771 cmd/compile: fix print/println when input is uint
Fixes #21887

Change-Id: I30e8e03ecfb67a2c4deedc2c8436da4c4782136d
Reviewed-on: https://go-review.googlesource.com/63971
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2017-09-26 04:08:38 +00:00
Daniel Martí
24ca86f308 cmd/compile: fix invalid switch case value panic
This is a regression introduced by myself in golang.org/cl/41852,
confirmed by the program that reproduces the crash that can be seen in
the added test.

Fixes #21988.

Change-Id: I18d5b2b3de63ced84db705b18490b00b16b59e02
Reviewed-on: https://go-review.googlesource.com/65655
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2017-09-24 10:15:52 +00:00
Austin Clements
e97209515a runtime: hide <autogenerated> methods from call stack
The compiler generates wrapper methods to forward interface method
calls (which are always pointer-based) to value methods. These
wrappers appear in the call stack even though they are an
implementation detail. This leaves ugly "<autogenerated>" functions in
stack traces and can throw off skip counts for stack traces.

Fix this by considering these runtime frames in printed stack traces
so they will only be printed if runtime frames are being printed, and
by eliding them from the call stack expansion used by CallersFrames
and Caller.

This removes the test for issue 4388 since that was checking that
"<autogenerated>" appeared in the stack trace instead of something
even weirder. We replace it with various runtime package tests.

Fixes #16723.

Change-Id: Ice3f118c66f254bb71478a664d62ab3fc7125819
Reviewed-on: https://go-review.googlesource.com/45412
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-09-22 22:17:20 +00:00
Matthew Dempsky
39983cf491 cmd/compile: refactor onebitwalktype1
The existing logic tried to advance the offset for each variable's
width, but then tried to undo this logic with the array and struct
handling code. It can all be much simpler by only worrying about
computing offsets within the array and struct code.

While here, include a short-circuit for zero-width arrays to fix a
pedantic compiler failure case.

Passes toolstash-check.

Fixes #20739.

Change-Id: I98af9bb512a33e3efe82b8bf1803199edb480640
Reviewed-on: https://go-review.googlesource.com/64471
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-09-20 18:11:52 +00:00
Matthew Dempsky
7c8a9615c0 cmd/compile: fix stack frame info for calls in receiver slot
Previously, after inlining a call, we made a second pass to rewrite
the AST's position information to record the inlined stack frame. The
call arguments were part of this AST, but it would be incorrect to
rewrite them too, so extra effort was made to temporarily remove them
while the position rewriting was done.

However, this extra logic was only done for regular arguments: it was
not done for receiver arguments. Consequently if m was inlined in
"f().m(g(), h())", g and h would have correct call frames, but f would
appear to be called by m.

The fix taken by this CL is to merge setpos into inlsubst and only
rewrite position information for nodes that were actually copied from
the original function AST body. As a side benefit, this eliminates an
extra AST pass and some AST walking code.

Fixes #21879.

Change-Id: I22b25c208313fc25c358d3a2eebfc9b012400084
Reviewed-on: https://go-review.googlesource.com/64470
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-09-19 18:35:24 +00:00
Matthew Dempsky
4347baac7d cmd/compile: eliminate OXFALL
Previously, we used OXFALL vs OFALL to distinguish fallthrough
statements that had been validated. Because in the Node AST we flatten
statement blocks, OXCASE and OXFALL needed to keep track of their
block scopes for this purpose.

Now that we have an AST that keeps these separate, we can just perform
the validation earlier.

Passes toolstash-check.

Fixes #14540.

Change-Id: I8421eaba16c2b3b72c9c5483b5cf20b14261385e
Reviewed-on: https://go-review.googlesource.com/61130
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-09-19 18:08:50 +00:00
Matthew Dempsky
bb2f0da23a cmd/compile: fix compiler crash on recursive types
By setting both a valid size and alignment for broken recursive types,
we can appease some more safety checks and prevent compiler crashes.

Fixes #21882.

Change-Id: Ibaa137d8aa2c2a9d521462f144d7016c4abfd6e7
Reviewed-on: https://go-review.googlesource.com/64430
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-09-18 21:49:43 +00:00
Kunpei Sakai
5a986eca86 all: fix article typos
a -> an

Change-Id: I7362bdc199e83073a712be657f5d9ba16df3077e
Reviewed-on: https://go-review.googlesource.com/63850
Reviewed-by: Rob Pike <r@golang.org>
2017-09-15 02:39:16 +00:00
Martin Möhrmann
137e4a6c63 cmd/compile: improve single blank variable handling in walkrange
Refactor walkrange to treat "for _ = range a" as "for range a".

This avoids generating some later discarded nodes in the compiler.

Passes toolstash -cmp.

Change-Id: Ifb2e1ca3b8519cbb67e8ad5aad514af9d18f1ec4
Reviewed-on: https://go-review.googlesource.com/61017
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2017-09-12 05:50:54 +00:00
Ian Lance Taylor
c6d019aa63 test: add test case that gccgo crashed on
Change-Id: I4d5d40e1ed3f58b2cdecd6248cb25c8ae9a1b9a1
Reviewed-on: https://go-review.googlesource.com/62531
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2017-09-11 06:03:39 +00:00
Than McIntosh
08347648a6 test: add test that caused gccgo incorrect compilation
Updates #21770

Change-Id: Ic31c3bdae30797f406f25c737b83bbe2de1ed1da
Reviewed-on: https://go-review.googlesource.com/62331
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-09-09 13:32:23 +00:00
Keith Randall
02deb77f6d cmd/compile: fix println()
println with no arguments accidentally doesn't print a newline.

Introduced at CL 55097

Fixes #21808

Change-Id: I9fc7b4271b9b31e4c9b6078f055195dc3907b62c
Reviewed-on: https://go-review.googlesource.com/62390
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-09-08 20:10:48 +00:00
Matthew Dempsky
34db5f0c4d cmd/compile: fix evaluation order for OASOP
Currently, we handle "x op= y" by rewriting as "x = x op y", while
ensuring that any calls or receive operations in 'x' are only
evaluated once. Notably, pointer indirection, indexing operations,
etc. are left alone as it's typically safe to re-evaluate those.

However, those operations were interleaved with evaluating 'y', which
could include function calls that might cause re-evaluation to yield
different memory addresses.

As a fix, simply ensure that we order side-effecting operations in 'y'
before either evaluation of 'x'.

Fixes #21687.

Change-Id: Ib14e77760fda9c828e394e8e362dc9e5319a84b2
Reviewed-on: https://go-review.googlesource.com/60091
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2017-09-05 18:10:17 +00:00
Cholerae Hu
fb165eaffd cmd/compile: combine x*n - y*n into (x-y)*n
Do the similar thing to CL 55143 to reduce IMUL.

Change-Id: I1bd38f618058e3cd74fac181f003610ea13f2294
Reviewed-on: https://go-review.googlesource.com/56252
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2017-09-03 14:29:38 +00:00
Keith Randall
053840dc00 cmd/compile: avoid generating large offsets
The assembler barfs on large offsets. Make sure that all the
instructions that need to have their offsets in an int32
  1) check on any rule that computes offsets for such instructions
  2) change their aux fields so the check builder checks it.

The assembler also silently misassembled offsets between 1<<31
and 1<<32. Add a check in the assembler to barf on those as well.

Fixes #21655

Change-Id: Iebf24bf10f9f37b3ea819ceb7d588251c0f46d7d
Reviewed-on: https://go-review.googlesource.com/59630
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2017-08-28 22:02:59 +00:00
Keith Randall
f1517ec6e5 cmd/compile: remove more nil ptr checks after newobject
For code like the following (where x escapes):

   x := []int{1}

We're currently generating a nil check.  The line above is really 3 operations:

	t := new([1]int)
	t[0] = 1
	x := t[:]

We remove the nil check for t[0] = 1, but not for t[:].

Our current nil check removal rule is too strict about the possible
memory arguments of the nil check. Unlike zeroing or storing to the
result of runtime.newobject, the nilness of runtime.newobject is
always false, even after other stores have happened in the meantime.

Change-Id: I95fad4e3a59c27effdb37c43ea215e18f30b1e5f
Reviewed-on: https://go-review.googlesource.com/58711
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-08-25 03:26:58 +00:00
Michael Munday
744ebfde04 cmd/compile: eliminate stores to unread auto variables
This is a crude compiler pass to eliminate stores to auto variables
that are only ever written to.

Eliminates an unnecessary store to x from the following code:

func f() int {
	var x := 1
	return *(&x)
}

Fixes #19765.

Change-Id: If2c63a8ae67b8c590b6e0cc98a9610939a3eeffa
Reviewed-on: https://go-review.googlesource.com/38746
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2017-08-24 16:53:56 +00:00