1
0
mirror of https://github.com/golang/go synced 2024-11-11 18:31:38 -07:00
Commit Graph

2868 Commits

Author SHA1 Message Date
Filippo Valsorda
a9ccbe0a69 Revert "[release-branch.go1.10] cmd/compile/internal/gc: OMUL should be evaluated when using soft-float"
This reverts commit d74e69c755.

Reason for revert: this fix has been backported to 1.11, and that makes
it ineligible for backport to 1.10, since upgrading to 1.11 is a valid
workaround according to https://github.com/golang/go/wiki/MinorReleases.

Also, this triggers a bug on Wasm that would require a backport of
CL 149965, too.

Updates #28688
Updates #28959

Change-Id: I4e56554ea27b5db83bdb1e9d2103dd24e3da8cfc
Reviewed-on: https://go-review.googlesource.com/c/154297
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
2018-12-14 20:07:16 +00:00
Milan Knezevic
d74e69c755 [release-branch.go1.10] cmd/compile/internal/gc: OMUL should be evaluated when using soft-float
When using soft-float, OMUL might be rewritten to function call
so we should ensure it was evaluated first.

Updates #28688
Fixes #28959

Change-Id: I30b87501782fff62d35151f394a1c22b0d490c6c
Reviewed-on: https://go-review.googlesource.com/c/148837
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit c92e73b702)
Reviewed-on: https://go-review.googlesource.com/c/151343
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2018-12-14 18:48:54 +00:00
Alberto Donizetti
30ccc6284a [release-branch.go1.10] cmd/compile: prevent overflow in walkinrange
In the compiler frontend, walkinrange indiscriminately calls Int64()
on const CTINT nodes, even though Int64's return value is undefined
for anything over 2⁶³ (in practise, it'll return a negative number).

This causes the introduction of bad constants during rewrites of
unsigned expressions, which make the compiler reject valid Go
programs.

This change introduces a preliminary check that Int64() is safe to
call on the consts on hand. If it isn't, walkinrange exits without
doing any rewrite.

Fixes #27247

Change-Id: I2017073cae65468a521ff3262d4ea8ab0d7098d9
Reviewed-on: https://go-review.googlesource.com/130735
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
(cherry picked from commit 42cc4ca30a)
Reviewed-on: https://go-review.googlesource.com/c/131595
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2018-11-01 20:08:12 +00:00
Keith Randall
305b0de625 [release-branch.go1.10] cmd/compile: fix zero extend after float->int conversion
Don't do direct loads from argument slots if the sizes don't match.
This prevents us from loading from a float32 using a uint64 load
during expressions like uint64(math.float32Bits(f)) where f is a float32 arg.

Fixes #25335

Change-Id: I3887d76f78c844ba546243e7721d811c3d4a9700
Reviewed-on: https://go-review.googlesource.com/112637
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/131276
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-08-24 16:18:39 +00:00
Keith Randall
dea961ebd9 [release-branch.go1.10] cmd/compile: reset branch prediction when deleting a branch
When we go from a branch block to a plain block, reset the
branch prediction bit. Downstream passes asssume that if the
branch prediction is set, then the block has 2 successors.

Fixes #23504
Fixes #26851

Change-Id: I2898ec002228b2e34fe80ce420c6939201c0a5aa
Reviewed-on: https://go-review.googlesource.com/88955
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
(cherry picked from commit 4313d7767d)
Reviewed-on: https://go-review.googlesource.com/128855
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2018-08-09 21:44:22 +00:00
Josh Bleecher Snyder
5b6ff694bb [release-branch.go1.10] cmd/compile: fix evaluation of "" < s
Fixes #24935

Change-Id: Ifa79ab3dfe69297eeef85f7193cd5f85e5982bc5
Reviewed-on: https://go-review.googlesource.com/106655
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-on: https://go-review.googlesource.com/108944
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Andrew Bonventre <andybons@golang.org>
2018-04-27 20:52:26 +00:00
Michael Munday
4e19a5498d [release-branch.go1.10] cmd/compile: mark LAA and LAAG as clobbering flags on s390x
The atomic add instructions modify the condition code and so need to
be marked as clobbering flags.

Fixes #24449.

Change-Id: Ic69c8d775fbdbfb2a56c5e0cfca7a49c0d7f6897
Reviewed-on: https://go-review.googlesource.com/101455
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-on: https://go-review.googlesource.com/102788
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
2018-03-29 06:08:34 +00:00
Keith Randall
419e6f0835 [release-branch.go1.10] cmd/compile: fix constant folding of right shifts
The sub-word shifts need to sign-extend before shifting, to avoid
bringing in data from higher in the argument.

Fixes #23812

Change-Id: I0a95a0b49c48f3b40b85765bb4a9bb492be0cd73
Reviewed-on: https://go-review.googlesource.com/93716
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit 755b36aa53)
Reviewed-on: https://go-review.googlesource.com/94215
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-02-15 03:01:30 +00:00
Keith Randall
23e8e197b0 cmd/compile: use unsigned loads for multi-element comparisons
When loading multiple elements of an array into a single register,
make sure we treat them as unsigned.  When treated as signed, the
upper bits might all be set, causing the shift-or combo to clobber
the values higher in the register.

Fixes #23719.

Change-Id: Ic87da03e9bd0fe2c60bb214b99f846e4e9446052
Reviewed-on: https://go-review.googlesource.com/92335
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
2018-02-06 18:24:33 +00:00
Cherry Zhang
43288467d2 test: add test for gccgo bug 23545
The fix is CL 91035.

Build only with gccgo at the moment, as it hits issue #23546.

Updates #23545.

Change-Id: I3a1367bb31b04773d31f71016f8fd7bd1855d7b5
Reviewed-on: https://go-review.googlesource.com/89735
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-01-31 20:03:31 +00:00
Keith Randall
7eaa8efbe4 cmd/compile: don't let spills clobber arguments
The compiler allows code to have multiple differently-typed views of a
single argument. For instance, if we have

func f(x float64) {
   y := *(*int64)(unsafe.Pointer(&x))
   ...
}

Then in SSA we get two OpArg ops, one with float64 type and one with
int64 type.

The compiler will try to reuse argument slots for spill slots. It
checks that the argument slot is dead by consulting an interference
graph.

When building the interference graph, we normally ignore cross-type
edges because the values on either end of that edge can't be allocated
to the same slot. (This is just a space-saving optimization.) This
rule breaks down when one of the values is an argument, because of the
multiple views described above. If we're spilling a float64, it is not
enough that the float64 version of x is dead; the int64 version of x
has to be dead also.

Remove the optimization of not recording interference edges if types
don't match. That optimization is incorrect if one of the values
connected by the edge is an argument.

Fixes #23522

Change-Id: I361f85d80fe3bc7249014ca2c3ec887c3dc30271
Reviewed-on: https://go-review.googlesource.com/89335
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-01-23 21:51:55 +00:00
Kunpei Sakai
e858a6b9f0 all: use Fatalf instead of Fatal if format is given
Change-Id: I30e9b938bb19ed4e674c3ea4a1cd389b9c4f0b88
Reviewed-on: https://go-review.googlesource.com/86875
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-01-10 01:35:45 +00:00
Cherry Zhang
7f1c4b3afb cmd/compile: disable "redundant zeroextensions" optimization for Select on AMD64
A Select Op could produce a value with upper 32 bits NOT zeroed,
for example, Div32 is lowered to (Select0 (DIVL x y)).

In theory, we could look into the argument of a Select to decide
whether the upper bits are zeroed. As it is late in release cycle,
just disable this optimization for Select for now.

Fixes #23305.

Change-Id: Icf665a2af9ccb0a7ba0ae00c683c9e349638bf85
Reviewed-on: https://go-review.googlesource.com/85736
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
2018-01-02 21:08:35 +00:00
Than McIntosh
841d865a56 cmd/compile: second attempt at fix for issue 23179
My previous fix for issue 23179 was incomplete; it turns out that if
an unnamed parameter is below a specific size threshold, it gets
register-promoted away by the compiler (hence not encountered during
some parts of DWARF inline info processing), but if it is sufficiently
large, it is allocated to the stack as a named variable and treated as
a regular parameter by DWARF generation. Interestingly, something in
the ppc64le build of k8s causes an unnamed parameter to be retained
(where on amd64 it is deleted), meaning that this wasn't caught in my
amd64 testing.

The fix is to insure that "_" params are treated in the same way that
"~r%d" return temps are when matching up post-optimization inlined
routine params with pre-inlining declarations. I've also updated the
test case to include a "_" parameter with a very large size, which
also triggers the bug on amd64.

Fixes #23179.

Change-Id: I961c84cc7a873ad3f8f91db098a5e13896c4856e
Reviewed-on: https://go-review.googlesource.com/84975
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2017-12-20 20:39:16 +00:00
Than McIntosh
0447216316 cmd/compile: fix corner case in DWARF inline info generation
The helper routine for returning pre-inlining parameter declarations
wasn't properly handling the case where you have more than one
parameter named "_" in a function signature; this triggered a map
collision later on when the function was inlined and DWARF was
generated for the inlined routine instance.

Fixes #23179.

Change-Id: I12e5d6556ec5ce08e982a6b53666a4dcc1d22201
Reviewed-on: https://go-review.googlesource.com/84755
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-12-19 19:00:42 +00:00
Russ Cox
c6ea277ac1 test: skip "# package/path" output from cmd/go in errchk
This allows errchk to be used with "go vet" output (as opposed to "go tool vet").

Change-Id: I0009a53c9cb74accd5bd3923c137d6dbf9e46326
Reviewed-on: https://go-review.googlesource.com/83836
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-12-13 21:32:01 +00:00
Matthew Dempsky
13bf4ada80 cmd/compile: remove broken inlining accounting code
We can't currently inline functions that contain closures anyway, so
just delete this budgeting code for now. Re-enable once we can (if
ever) inline functions with nested closures.

Updates #15561.
Fixes #23093.

Change-Id: Idc5f8e042ccfcc8921022e58d3843719d4ab821e
Reviewed-on: https://go-review.googlesource.com/83538
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-12-12 20:26:33 +00:00
Matthew Dempsky
840fad13ec cmd/compile: fix unsafe.Pointer liveness for Syscall-like functions
The package unsafe docs say it's safe to convert an unsafe.Pointer to
uintptr in the argument list to an assembly function, but it was
erroneously only detecting normal pointers converted to unsafe.Pointer
and then to intptr.

Fixes #23051.

Change-Id: Id1be19f6d8f26f2d17ba815191717d2f4f899732
Reviewed-on: https://go-review.googlesource.com/82817
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-12-08 21:34:24 +00:00
Keith Randall
4c800f03c9 cmd/compile: fix large load/store offsets on 386
Pointer arithemetic is done mod 2^32 on 386, so we can just
drop the high bits of any large constant offsets.

The bounds check will make sure wraparounds are never observed.

Fixes #21655

Change-Id: I68ae5bbea9f02c73968ea2b21ca017e5ecb89223
Reviewed-on: https://go-review.googlesource.com/82675
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2017-12-08 03:53:18 +00:00
Keith Randall
dd7cbf3a84 cmd/compile: fix map assignment with panicking right-hand side
Make sure that when we're assigning to a map, we evaluate the
right-hand side before we attempt to insert into the map.

We used to evaluate the left-hand side to a pointer-to-slot-in-bucket
(which as a side effect does len(m)++), then evaluate the right-hand side,
then do the assignment. That clearly isn't correct when the right-hand side
might panic.

Fixes #22881

Change-Id: I42a62870ff4bf480568c9bdbf0bb18958962bdf0
Reviewed-on: https://go-review.googlesource.com/81817
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-12-05 00:10:10 +00:00
Matthew Dempsky
2b9e7c1864 test: disable broken test for 1.10
This test was added recently as a regress test for the spec relaxation
in #9060, but doesn't work correctly yet. Disable for now to fix noopt
builders.

Updates #22444.

Change-Id: I45c521ae0da7ffb0c6859d6f7220c59828ac6149
Reviewed-on: https://go-review.googlesource.com/81775
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-12-04 19:29:14 +00:00
Than McIntosh
88c2fb9d04 cmd/compile: fix bug in DWARF inl handling of unused autos
The DWARF inline info generation hooks weren't properly
handling unused auto vars in certain cases, triggering an assert (now
fixed). Also with this change, introduce a new autom "flavor" to
use for autom entries that are added to insure that a specific
auto type makes it into the linker (this is a follow-on to the fix
for 22941).

Fixes #22962.

Change-Id: I7a2d8caf47f6ca897b12acb6a6de0eb25f5cac8f
Reviewed-on: https://go-review.googlesource.com/81557
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-12-04 18:36:11 +00:00
Robert Griesemer
088a9ad543 cmd/compile: permit indices of certain non-constant shifts
Per the decision for #14844, index expressions that are non-constant
shifts where the LHS operand is representable as an int are now valid.

Fixes #21693.

Change-Id: Ifafad2c0c65975e0200ce7e28d1db210e0eacd9d
Reviewed-on: https://go-review.googlesource.com/81277
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-12-01 20:39:50 +00:00
Than McIntosh
9372166faa cmd/compile: fix DWARF type symbol buglet
The code that generates the list of DWARF variables for a function
(params and autos) will emit a "no-location" entry in the DWARF for a
user var that appears in the original pre-optimization version of the
function but is no longer around when optimization is complete. The
intent is that if a GDB user types "print foo" (where foo has been
optimized out), the response will be "<optimized out>" as opposed to
"there is no such variable 'foo'). This change fixes said code to
include vars on the autom list for the function, to insure that the
type symbol for the variable makes it to the linker.

Fixes #22941.

Change-Id: Id29f1f39d68fbb798602dfd6728603040624fc41
Reviewed-on: https://go-review.googlesource.com/81415
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-12-01 18:52:50 +00:00
Cherry Zhang
dbb1d198ab cmd/compile: fix loop depth of range expression in escape analysis
ORANGE node's Right node is the expression it is ranging over,
which is evaluated before the loop. In the escape analysis,
we should walk this node without loop depth incremented.

Fixes #21709.

Change-Id: Idc1e4c76e39afb5a344d85f6b497930a488ce5cf
Reviewed-on: https://go-review.googlesource.com/80740
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2017-11-30 13:45:18 +00:00
Matthew Dempsky
2f588ff08f cmd/compile: make -asmhdr work with type aliases
For "type T = U" we were accidentally emitting a #define for "U__size"
instead of "T__size".

Fixes #22877.

Change-Id: I5ed6757d697753ed6d944077c16150759f6e1285
Reviewed-on: https://go-review.googlesource.com/80759
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-11-29 20:40:41 +00:00
Matthew Dempsky
71a9c4430f cmd/compile: fix infinite recursion in isdirectiface
Fixes #22904.

Change-Id: Id504504eda7275c10d3c665add8b7ccd23f65820
Reviewed-on: https://go-review.googlesource.com/80301
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-11-28 21:58:16 +00:00
Matthew Dempsky
3c375f1b7e cmd/compile, go/types: error if main.main is not a function
Fixes #21256.

Change-Id: I3af4c76e734c09d07f15525b793a544a7279b906
Reviewed-on: https://go-review.googlesource.com/79435
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-11-22 19:14:31 +00:00
Keith Randall
48e207d518 cmd/compile: fix mapassign_fast* routines for pointer keys
The signature of the mapassign_fast* routines need to distinguish
the pointerness of their key argument.  If the affected routines
suspend part way through, the object pointed to by the key might
get garbage collected because the key is typed as a uint{32,64}.

This is not a problem for mapaccess or mapdelete because the key
in those situations do not live beyond the call involved.  If the
object referenced by the key is garbage collected prematurely, the
code still works fine.  Even if that object is subsequently reallocated,
it can't be written to the map in time to affect the lookup/delete.

Fixes #22781

Change-Id: I0bbbc5e9883d5ce702faf4e655348be1191ee439
Reviewed-on: https://go-review.googlesource.com/79018
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
2017-11-22 04:30:27 +00:00
Austin Clements
e862f98d1e test: make inline_callers.go test not inline the runtime
CL 76551 modified inline_callers.go to build everything, including the
runtime, with -l=4. While that works in most places (and ideally
should work everywhere), it blows out the nosplit stack on
solaris/amd64.

Fix this by only building the test itself with -l=4.

This undoes some of the changes to this test from CL 73212, which
originally changed the go tool to rebuild all packages with the given
flags. This change modified the expected output of this test, so now
that we can go back to building only the test itself with inlining, we
revert these changes to the expected output. (That CL also changed
log.Fatalf to log.Printf, but didn't add "\n" to the end of the lines,
so this CL fixes that, too.)

Fixes #22797.

Change-Id: I6a91963a59ebe98edbe0921d8717af6b2c2191b0
Reviewed-on: https://go-review.googlesource.com/79197
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-11-22 00:52:54 +00:00
Emmanuel Odeke
5f29a7a705 cmd/compile: hint on wrong case-field names in composite literals
Improve the error message for wrong
case-field names in composite literals,
by mentioning the correct field name.

Given the program:
package main

type it struct {
        ID string
}

func main() {
        i1 := &it{id: "Bar"}
}

just like we do for usage of fields, we now
report wrongly cased fields as hints to give:

ts.go:8:14: unknown field 'id' in struct literal of type it (but does have ID)

instead of before:

ts.go:8:14: unknown field 'id' in struct literal of type it

Fixes #22794

Change-Id: I18cd70e75817025cb1df083503cae306e8d659fd
Reviewed-on: https://go-review.googlesource.com/78545
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-11-20 20:29:43 +00:00
Keith Randall
3abf117704 cmd/compile: add test for array decomposition
This test fails on 1.9.2, but is ok on tip.
CL 77331 has both the 1.9.2 fix and this test, and is on the 1.9 release branch.
This CL is just the test, and is on HEAD.  The buggy code doesn't exist on tip.

Update #22683

Change-Id: I04a24bd6c2d3068e18ca81da3347e2c1366f4447
Reviewed-on: https://go-review.googlesource.com/77332
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-11-13 20:22:22 +00:00
griesemer
ca2a886cba cmd/compile: record original and absolute file names for line directives
Also, with this change, error locations don't print absolute positions
in [] brackets following positions relative to line directives. To get
the absolute positions as well, specify the -L flag.

Fixes #22660.

Change-Id: I9ecfa254f053defba9c802222874155fa12fee2c
Reviewed-on: https://go-review.googlesource.com/77090
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2017-11-13 16:47:41 +00:00
Russ Cox
5993251c01 cmd/go: implement per-package asmflags, gcflags, ldflags, gccgoflags
It has always been problematic that there was no way to specify
tool flags that applied only to the build of certain packages;
it was only to specify flags for all packages being built.
The usual workaround was to install all dependencies of something,
then build just that one thing with different flags. Since the
dependencies appeared to be up-to-date, they were not rebuilt
with the different flags. The new content-based staleness
(up-to-date) checks see through this trick, because they detect
changes in flags. This forces us to address the underlying problem
of providing a way to specify per-package flags.

The solution is to allow -gcflags=pattern=flags, which means
that flags apply to packages matching pattern, in addition to the
usual -gcflags=flags, which is now redefined to apply only to
the packages named on the command line.

See #22527 for discussion and rationale.

Fixes #22527.

Change-Id: I6716bed69edc324767f707b5bbf3aaa90e8e7302
Reviewed-on: https://go-review.googlesource.com/76551
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2017-11-09 15:04:04 +00:00
David Chase
a042221cdb cmd/compile: adjust Pos setting for "empty" blocks
Plain blocks that contain only uninteresting instructions
(that do not have reliable Pos information themselves)
need to have their Pos left unset so that they can
inherit it from their successors.  The "uninteresting"
test was not properly applied and not properly defined.
OpFwdRef does not appear in the ssa.html debugging output,
but at the time of the test these instructions did appear,
and it needs to be part of the test.

Fixes #22365.

Change-Id: I99e5b271acd8f6bcfe0f72395f905c7744ea9a02
Reviewed-on: https://go-review.googlesource.com/74252
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2017-11-08 22:39:49 +00:00
Ian Lance Taylor
a4c009f5ae cmd/compile: don't put Noalg types in typelinks
They could get picked up by reflect code, yielding the wrong type.

Fixes #22605

Change-Id: Ie11fb361ca7f3255e662037b3407565c8f0a2c4c
Reviewed-on: https://go-review.googlesource.com/76315
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-11-07 22:29:48 +00:00
griesemer
17ff23f7c8 cmd/compile/internal/syntax: better syntax errors for typos in if/switch/for headers
Be more pessimistic when parsing if/switch/for headers for better error
messages when things go wrong.

Fixes #22581.

Change-Id: Ibb99925291ff53f35021bc0a59a4c9a7f695a194
Reviewed-on: https://go-review.googlesource.com/76290
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-11-06 23:17:24 +00:00
Emmanuel Odeke
0c55495748 cmd/compile: lock in test for column numbers in unused error
Updates #21317

@mdempsky fixed issue #21317 with CL 66810,
so lock a test in to ensure we don't regress.

The test is manual for now before test/run.go
has support for matching column numbers so do
it old school and match expected output after
an exec.

Change-Id: I6c2a66ddf04248f79d17ed7033a3280d50e41562
Reviewed-on: https://go-review.googlesource.com/76150
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-11-06 21:32:06 +00:00
Austin Clements
3a446d8652 cmd/compile: []T where T is go:notinheap does not need write barriers
Currently, assigning a []T where T is a go:notinheap type generates an
unnecessary write barrier for storing the slice pointer.

This fixes this by teaching HasHeapPointer that this type does not
have a heap pointer, and tweaking the lowering of slice assignments so
the pointer store retains the correct type rather than simply lowering
it to a *uint8 store.

Change-Id: I8bf7c66e64a7fefdd14f2bd0de8a5a3596340bab
Reviewed-on: https://go-review.googlesource.com/76027
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-11-06 21:07:57 +00:00
Emmanuel Odeke
0ba4eba864 cmd/compile: test for omitted ICE diagnostics after normal messages
Updates #22389

@mdempsky's CL 70850 fixed the unnecessary
compile stack trace printing during ICE diagnostics.

This CL adds a test to lock in this behavior.

Change-Id: I9ce49923c80b78cb8c0bb5dc4af3c860a43d63ba
Reviewed-on: https://go-review.googlesource.com/74630
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-11-06 08:37:30 +00:00
Hugues Bruant
c4b65fa4cc cmd/compile: inline closures with captures
When inlining a closure with captured variables, walk up the
param chain to find the one that is defined inside the scope
into which the function is being inlined, and map occurrences
of the captures to temporary inlvars, similarly to what is
done for function parameters.

No noticeable impact on compilation speed and binary size.

Minor improvements to go1 benchmarks on darwin/amd64

name                     old time/op    new time/op    delta
BinaryTree17-4              2.59s ± 3%     2.58s ± 1%    ~     (p=0.470 n=19+19)
Fannkuch11-4                3.15s ± 2%     3.15s ± 1%    ~     (p=0.647 n=20+19)
FmtFprintfEmpty-4          43.7ns ± 3%    43.4ns ± 4%    ~     (p=0.178 n=18+20)
FmtFprintfString-4         74.0ns ± 2%    77.1ns ± 7%  +4.13%  (p=0.000 n=20+20)
FmtFprintfInt-4            77.2ns ± 3%    79.2ns ± 6%  +2.53%  (p=0.000 n=20+20)
FmtFprintfIntInt-4          112ns ± 4%     112ns ± 2%    ~     (p=0.672 n=20+19)
FmtFprintfPrefixedInt-4     136ns ± 1%     135ns ± 2%    ~     (p=0.827 n=16+20)
FmtFprintfFloat-4           232ns ± 2%     233ns ± 1%    ~     (p=0.194 n=20+20)
FmtManyArgs-4               490ns ± 2%     484ns ± 2%  -1.28%  (p=0.001 n=20+20)
GobDecode-4                6.68ms ± 2%    6.72ms ± 2%    ~     (p=0.113 n=20+19)
GobEncode-4                5.62ms ± 2%    5.71ms ± 2%  +1.64%  (p=0.000 n=20+19)
Gzip-4                      235ms ± 3%     236ms ± 2%    ~     (p=0.607 n=20+19)
Gunzip-4                   37.1ms ± 2%    36.8ms ± 3%    ~     (p=0.060 n=20+20)
HTTPClientServer-4         61.9µs ± 2%    62.7µs ± 4%  +1.24%  (p=0.007 n=18+19)
JSONEncode-4               12.5ms ± 2%    12.4ms ± 3%    ~     (p=0.192 n=20+20)
JSONDecode-4               51.6ms ± 3%    51.0ms ± 3%  -1.19%  (p=0.008 n=20+19)
Mandelbrot200-4            4.12ms ± 6%    4.06ms ± 5%    ~     (p=0.063 n=20+20)
GoParse-4                  3.12ms ± 5%    3.10ms ± 2%    ~     (p=0.402 n=19+19)
RegexpMatchEasy0_32-4      80.7ns ± 2%    75.1ns ± 9%  -6.94%  (p=0.000 n=17+20)
RegexpMatchEasy0_1K-4       197ns ± 2%     186ns ± 2%  -5.43%  (p=0.000 n=20+20)
RegexpMatchEasy1_32-4      77.5ns ± 4%    71.9ns ± 7%  -7.25%  (p=0.000 n=20+18)
RegexpMatchEasy1_1K-4       341ns ± 3%     341ns ± 3%    ~     (p=0.732 n=20+20)
RegexpMatchMedium_32-4      113ns ± 2%     112ns ± 3%    ~     (p=0.102 n=20+20)
RegexpMatchMedium_1K-4     36.6µs ± 2%    35.8µs ± 2%  -2.26%  (p=0.000 n=18+20)
RegexpMatchHard_32-4       1.75µs ± 3%    1.74µs ± 2%    ~     (p=0.473 n=20+19)
RegexpMatchHard_1K-4       52.6µs ± 2%    52.0µs ± 3%  -1.15%  (p=0.005 n=20+20)
Revcomp-4                   381ms ± 4%     377ms ± 2%    ~     (p=0.067 n=20+18)
Template-4                 57.3ms ± 2%    57.7ms ± 2%    ~     (p=0.108 n=20+20)
TimeParse-4                 291ns ± 3%     292ns ± 2%    ~     (p=0.585 n=20+20)
TimeFormat-4                314ns ± 3%     315ns ± 1%    ~     (p=0.681 n=20+20)
[Geo mean]                 47.4µs         47.1µs       -0.73%

name                     old speed      new speed      delta
GobDecode-4               115MB/s ± 2%   114MB/s ± 2%    ~     (p=0.115 n=20+19)
GobEncode-4               137MB/s ± 2%   134MB/s ± 2%  -1.63%  (p=0.000 n=20+19)
Gzip-4                   82.5MB/s ± 3%  82.4MB/s ± 2%    ~     (p=0.612 n=20+19)
Gunzip-4                  523MB/s ± 2%   528MB/s ± 3%    ~     (p=0.060 n=20+20)
JSONEncode-4              155MB/s ± 2%   156MB/s ± 3%    ~     (p=0.192 n=20+20)
JSONDecode-4             37.6MB/s ± 3%  38.1MB/s ± 3%  +1.21%  (p=0.007 n=20+19)
GoParse-4                18.6MB/s ± 4%  18.7MB/s ± 2%    ~     (p=0.405 n=19+19)
RegexpMatchEasy0_32-4     396MB/s ± 2%   426MB/s ± 8%  +7.56%  (p=0.000 n=17+20)
RegexpMatchEasy0_1K-4    5.18GB/s ± 2%  5.48GB/s ± 2%  +5.79%  (p=0.000 n=20+20)
RegexpMatchEasy1_32-4     413MB/s ± 4%   444MB/s ± 6%  +7.46%  (p=0.000 n=20+19)
RegexpMatchEasy1_1K-4    3.00GB/s ± 3%  3.00GB/s ± 3%    ~     (p=0.678 n=20+20)
RegexpMatchMedium_32-4   8.82MB/s ± 2%  8.90MB/s ± 3%  +0.99%  (p=0.044 n=20+20)
RegexpMatchMedium_1K-4   28.0MB/s ± 2%  28.6MB/s ± 2%  +2.32%  (p=0.000 n=18+20)
RegexpMatchHard_32-4     18.3MB/s ± 3%  18.4MB/s ± 2%    ~     (p=0.482 n=20+19)
RegexpMatchHard_1K-4     19.5MB/s ± 2%  19.7MB/s ± 3%  +1.18%  (p=0.004 n=20+20)
Revcomp-4                 668MB/s ± 4%   674MB/s ± 2%    ~     (p=0.066 n=20+18)
Template-4               33.8MB/s ± 2%  33.6MB/s ± 2%    ~     (p=0.104 n=20+20)
[Geo mean]                124MB/s        126MB/s       +1.54%

Updates #15561
Updates #18270

Change-Id: I980086efe28b36aa27f81577065e2a729ff03d4e
Reviewed-on: https://go-review.googlesource.com/72490
Reviewed-by: Hugues Bruant <hugues.bruant@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-11-05 04:18:05 +00:00
Hugues Bruant
483e298daa cmd/compile: fix reassignment check
CL 65071 enabled inlining for local closures with no captures.

To determine safety of inlining a call sites, we check whether the
variable holding the closure has any assignments after its original
definition.

Unfortunately, that check did not catch OAS2MAPR and OAS2DOTTYPE,
leading to incorrect inlining when a variable holding a closure was
subsequently reassigned through a type conversion or a 2-valued map
access.

There was another more subtle issue wherein reassignment check would
always return a false positive for closure calls inside other
closures. This was caused by the Name.Curfn field of local variables
pointing to the OCLOSURE node instead of the corresponding ODCLFUNC,
which resulted in reassigned walking an empty Nbody and thus never
seeing any reassignments.

This CL fixes these oversights and adds many more tests for closure
inlining which ensure not only that inlining triggers but also the
correctness of the resulting code.

Updates #15561

Change-Id: I74bdae849c4ecfa328546d6d62b512e8d54d04ce
Reviewed-on: https://go-review.googlesource.com/75770
Reviewed-by: Hugues Bruant <hugues.bruant@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-11-03 20:09:26 +00:00
griesemer
25159d3af9 cmd/compile: avoid spurious errors for invalid map key types
Instead of trying to validate map key types eagerly in some
cases, delay their validation to the end of type-checking,
when we all type information is present.

Passes go build -toolexec 'toolstash -cmp' -a std .

Fixes #21273.
Fixes #21657.

Change-Id: I532369dc91c6adca1502d6aa456bb06b57e6c7ff
Reviewed-on: https://go-review.googlesource.com/75310
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-11-02 23:53:38 +00:00
Martin Möhrmann
fbfc2031a6 cmd/compile: specialize map creation for small hint sizes
Handle make(map[any]any) and make(map[any]any, hint) where
hint <= BUCKETSIZE special to allow for faster map initialization
and to improve binary size by using runtime calls with fewer arguments.

Given hint is smaller or equal to BUCKETSIZE in which case
overLoadFactor(hint, 0)  is false and no buckets would be allocated by makemap:
* If hmap needs to be allocated on the stack then only hmap's hash0
  field needs to be initialized and no call to makemap is needed.
* If hmap needs to be allocated on the heap then a new special
  makehmap function will allocate hmap and intialize hmap's
  hash0 field.

Reduces size of the godoc by ~36kb.

AMD64
name         old time/op    new time/op    delta
NewEmptyMap    16.6ns ± 2%     5.5ns ± 2%  -66.72%  (p=0.000 n=10+10)
NewSmallMap    64.8ns ± 1%    56.5ns ± 1%  -12.75%  (p=0.000 n=9+10)

Updates #6853

Change-Id: I624e90da6775afaa061178e95db8aca674f44e9b
Reviewed-on: https://go-review.googlesource.com/61190
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2017-11-02 17:03:45 +00:00
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