Loop breaking with a counter. Benchmarked (see comments),
eyeball checked for sanity on popular loops. This code
ought to handle loops in general, and properly inserts phi
functions in cases where the earlier version might not have.
Includes test, plus modifications to test/run.go to deal with
timeout and killing looping test. Tests broken by the addition
of extra code (branch frequency and live vars) for added
checks turn the check insertion off.
If GOEXPERIMENT=preemptibleloops, the compiler inserts reschedule
checks on every backedge of every reducible loop. Alternately,
specifying GO_GCFLAGS=-d=ssa/insert_resched_checks/on will
enable it for a single compilation, but because the core Go
libraries contain some loops that may run long, this is less
likely to have the desired effect.
This is intended as a tool to help in the study and diagnosis
of GC and other latency problems, now that goal STW GC latency
is on the order of 100 microseconds or less.
Updates #17831.
Updates #10958.
Change-Id: I6206c163a5b0248e3f21eb4fc65f73a179e1f639
Reviewed-on: https://go-review.googlesource.com/33910
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
CALLPART of STRUCTLIT did not check for incomplete initialization
of struct; modify PTRLIT treatment to force zeroing.
Test for structlit, believe this might have also failed for
arraylit.
Fixes#18410.
Change-Id: I511abf8ef850e300996d40568944665714efe1fc
Reviewed-on: https://go-review.googlesource.com/34622
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Fixes#18392.
Avoid nil dereferencing n.Right when dealing with non-existent
self referenced interface methods e.g.
type A interface{
Fn(A.Fn)
}
Instead, infer the symbol name from n.Sym itself.
Change-Id: I60d5f8988e7318693e5c8da031285d8d7347b771
Reviewed-on: https://go-review.googlesource.com/34817
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Fixes#6772.
Lock-in test for invalid range loop: repeated variables in range declaration.
Change-Id: I37dd8b1cd7279abe7810deaf8a5d485c5c3b73ca
Reviewed-on: https://go-review.googlesource.com/34714
Reviewed-by: Keith Randall <khr@golang.org>
Change-Id: I00c97c36e8fdc79582eaed21877e4c8f44568666
Reviewed-on: https://go-review.googlesource.com/34316
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
- make the scanner unconditionally gc compatible
- consistently use "invalid" instead "illegal" in errors
Reviewed in and cherry-picked from https://go-review.googlesource.com/#/c/33896/.
Change-Id: I4c4253e7392f3311b0d838bbe503576c9469b203
Reviewed-on: https://go-review.googlesource.com/34237
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This was a regression from 1.7. See the issue for details.
Fixes#18149.
Change-Id: Ic8f5a35d14edf9254b1275400316cff7aff32a27
Reviewed-on: https://go-review.googlesource.com/33799
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It was supposed to be testing SSA, not amd64.
For #18024
Change-Id: Ibe65d7eb6bed9bc4b3eda68e1eaec5fa39fe8f76
Reviewed-on: https://go-review.googlesource.com/33491
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
The table of rewrites in ssa/cse is not sized appropriately for
ssa IDs that are created during copying of selects into new blocks.
Fixes#17918
Change-Id: I65fe86c6aab5efa679aa473aadc4ee6ea882cd41
Reviewed-on: https://go-review.googlesource.com/33240
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The tree is inconsistent about single l vs double l in those
words in documentation, test messages, and one error value text.
$ git grep -E '[Mm]arshall(|s|er|ers|ed|ing)' | wc -l
42
$ git grep -E '[Mm]arshal(|s|er|ers|ed|ing)' | wc -l
1694
Make it consistently a single l, per earlier decisions. This means
contributors won't be confused by misleading precedence, and it helps
consistency.
Change the spelling in one error value text in newRawAttributes of
crypto/x509 package to be consistent.
This change was generated with:
perl -i -npe 's,([Mm]arshal)l(|s|er|ers|ed|ing),$1$2,' $(git grep -l -E '[Mm]arshall' | grep -v AUTHORS | grep -v CONTRIBUTORS)
Updates #12431.
Follows https://golang.org/cl/14150.
Change-Id: I85d28a2d7692862ccb02d6a09f5d18538b6049a2
Reviewed-on: https://go-review.googlesource.com/33017
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
ppc64le supports both internal and external linking so I don't
think there is any reason for it to skip this test.
Change-Id: I05c80cc25909c0364f0a1fb7d20766b011ea1ebb
Reviewed-on: https://go-review.googlesource.com/32854
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Use a separate symbol for reflect metadata for types with Noalg set.
Fixes#17752.
Change-Id: Icb6cade7e3004fc4108f67df61105dc4085cd7e2
Reviewed-on: https://go-review.googlesource.com/32679
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
when compiling f(a, b, c), we do something like:
*(SP+0) = eval(a)
*(SP+8) = eval(b)
*(SP+16) = eval(c)
call f
If one of those evaluations is later determined to unconditionally panic
(say eval(b) in this example), then the call is deadcode eliminated. But
any previous argument write (*(SP+0)=... here) is still around. Becuase
we only compute the size of the outarg area for calls which are still
around at the end of optimization, the space needed for *(SP+0)=v is not
accounted for and thus the outarg area may be too small.
The fix is to make sure that we evaluate any potentially panicing
operation before we write any of the args to the stack. It turns out
that fix is pretty easy, as we already have such a mechanism available
for function args. We just need to extend it to possibly panicing args
as well.
The resulting code (if b and c can panic, but a can't) is:
tmpb = eval(b)
*(SP+16) = eval(c)
*(SP+0) = eval(a)
*(SP+8) = tmpb
call f
This change tickled a bug in how we find the arguments for intrinsic
calls, so that latent bug is fixed up as well.
Update #16760.
Change-Id: I0bf5edf370220f82bc036cf2085ecc24f356d166
Reviewed-on: https://go-review.googlesource.com/32551
Reviewed-by: Cherry Zhang <cherryyz@google.com>
We generate an OpKeepAlive for the idata portion of the interface
for a runtime.KeepAlive call. But given such an op, we need to keep
the entire containing variable alive, not just the range that was
passed to the OpKeepAlive operation.
Fixes#17710
Change-Id: I90de66ec8065e22fb09bcf9722999ddda289ae6e
Reviewed-on: https://go-review.googlesource.com/32477
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
typecheckcomplit nils out node's type, upon finding new errors.
This hides new errors in children's node as well as the type info
of current node. This change fixes that.
Fixes#17645.
Change-Id: Ib473291f31c7e8fa0307cb1d494e0c112ddd3583
Reviewed-on: https://go-review.googlesource.com/32324
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously, on encountering Func.Nname.Type == nil, typecheckfunc()
returned without initializing Decldepth for that func. This causes
typecheckclosure() to fatal. This change ensures that we initialize
Decldepth in all cases.
Fixes#17588.
Change-Id: I2e3c81ad52e8383395025388989e8dbf03438b68
Reviewed-on: https://go-review.googlesource.com/32415
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is an extension of
https://go-review.googlesource.com/c/31662/
to mark all the temporaries, not just the ssa-generated ones.
Before-and-after ls -l `go tool -n compile` shows a 3%
reduction in size (or rather, a prior 3% inflation for
failing to filter temps out properly.)
Replaced name-dependent "is it a temp?" tests with calls to
*Node.IsAutoTmp(), which depends on AutoTemp. Also replace
calls to istemp(n) with n.IsAutoTmp(), to reduce duplication
and clean up function name space. Generated temporaries
now come with a "." prefix to avoid (apparently harmless)
clashes with legal Go variable names.
Fixes#17644.
Fixes#17240.
Change-Id: If1417f29c79a7275d7303ddf859b51472890fd43
Reviewed-on: https://go-review.googlesource.com/32255
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Improves the error message by moving the field name before the body
of a struct, in the error message for unknown fields for structs.
* Exhibit:
Given program:
package main
import "time"
func main() {
_ = struct {
about string
before map[string]uint
update map[string]int
updateTime time.Time
expect map[string]int
}{
about: "this one",
updates: map[string]int{"gopher": 10},
}
}
* Before:
./issue17631.go:20: unknown struct { about string; before map[string]uint;
update map[string]int; updateTime time.Time; expect map[string]int } field
'updates' in struct literal
* After:
./issue17631.go:20: unknown field 'updates' in struct literal of type { about string;
before map[string]uint; update map[string]int; updateTime time.Time;
expect map[string]int }
Fixes#17631
Change-Id: I76842616411b931b5ad7a76bd42860dfde7739f4
Reviewed-on: https://go-review.googlesource.com/32240
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Change-Id: Iec35f9b62982da40de400397bc456149216303dc
Reviewed-on: https://go-review.googlesource.com/32297
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Use "have" and "want" and multiple lines like other similar error
messages. Also, fix handling of ... and multi-value function calls.
Fixes#17650.
Change-Id: I4850e79c080eac8df3b92a4accf9e470dff63c9a
Reviewed-on: https://go-review.googlesource.com/32261
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes a bug where assignments that should come after a call
were instead being issued before the call.
Fixes#17596Fixes#17618
Change-Id: Ic9ae4c34ae38fc4ccd0604b65345b05896a2c295
Reviewed-on: https://go-review.googlesource.com/32226
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The runtime traceback code assumes non-empty frame has link
link register saved on LR architectures. Make sure it is so in
the assember.
Also make sure that LR is stored before update SP, so the traceback
code will not see a half-updated stack frame if a signal comes
during the execution of function prologue.
Fixes#17381.
Change-Id: I668b04501999b7f9b080275a2d1f8a57029cbbb3
Reviewed-on: https://go-review.googlesource.com/31760
Reviewed-by: Michael Munday <munday@ca.ibm.com>
Bug 15141 was apparently fixed by some other change to the
compiler (this is plausible, it was a weird bug dependent
on a particular way of returning a large named array result),
add the test to ensure that it stays fixed.
Updates #15141.
Change-Id: I3d6937556413fab1af31c5a1940e6931563ce2f3
Reviewed-on: https://go-review.googlesource.com/31972
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reuse the same mechanisms for handling universal builtins like len to
handle unsafe.Sizeof, etc. Allows us to drop package unsafe's export
data, and simplifies some code.
Updates #17508.
Change-Id: I620e0617c24e57e8a2d7cccd0e2de34608779656
Reviewed-on: https://go-review.googlesource.com/31433
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
instrumentnode() accidentally copies parent's already-instrumented nodes
into child's Ninit block. This generates repeated code in race-instrumentation.
This case surfaces only when it duplicates inline-labels, because of
compile time error. In other cases, it silently generates incorrect
instrumented code. This change prevents it from doing so.
Fixes#17449.
Change-Id: Icddf2198990442166307e176b7e20aa0cf6c171c
Reviewed-on: https://go-review.googlesource.com/31317
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
getArgInfo for reflect.makeFuncStub and reflect.methodValueCall is
necessarily special. These have dynamically determined argument maps
that are stored in their context (that is, their *funcval). These
functions are written to store this context at 0(SP) when called, and
getArgInfo retrieves it from there.
This technique works if getArgInfo is passed an active call frame for
one of these functions. However, getArgInfo is also used in
tracebackdefers, where the "call" is not a true call with an active
stack frame, but a deferred call. In this situation, getArgInfo
currently crashes because tracebackdefers passes a frame with sp set
to 0. However, the entire approach used by getArgInfo is flawed in
this situation because the wrapper has not actually executed, and
hence hasn't saved this metadata to any stack frame.
In the defer case, we know the *funcval from the _defer itself, so we
can fix this by teaching getArgInfo to use the *funcval context
directly when its available, and otherwise get it from the active call
frame.
While we're here, this commit simplifies getArgInfo a bit by making it
play more nicely with the type system. Rather than decoding the
*reflect.methodValue that is the wrapper's context as a *[2]uintptr,
just write out a copy of the reflect.methodValue type in the runtime.
Fixes#16331. Fixes#17471.
Change-Id: I81db4d985179b4a81c68c490cceeccbfc675456a
Reviewed-on: https://go-review.googlesource.com/31138
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The error check patterns in this test are more complex than necessary
because f2 gets inlined into f1. This behavior isn't important to the
test, so disable inlining of f2 and simplify the error check patterns.
Change-Id: Ia8aee92a52f9217ad71b89b2931494047e8d2185
Reviewed-on: https://go-review.googlesource.com/31132
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
"abc"[1] is not like 'b', in that -"abc"[1] is uint8 math, not ideal constant math.
Delay the constantification until after ideal constant folding is over.
Fixes#11370.
Change-Id: Iba2fc00ca2455959e7bab8f4b8b4aac14b1f9858
Reviewed-on: https://go-review.googlesource.com/15740
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In some cases the members of the root set from which flood
runs themselves escape, without their referents being also
tagged as escaping. Fix this by reflooding from those roots
whose escape increases, and also enhance the "leak" test to
include reachability from a heap-escaped root.
Fixes#17318.
Change-Id: Ied1e75cee17ede8ca72a8b9302ce8201641ec593
Reviewed-on: https://go-review.googlesource.com/30693
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
With this change, the code in bug #15609 compiles and runs properly:
0000000000401070 <main.jump>:
401070: ff 15 aa 7e 06 00 callq *0x67eaa(%rip) # 468f20 <main.pointer>
401076: c3 retq
0000000000468f20 g O .rodata 0000000000000008 main.pointer
Fixes#15609
Change-Id: Iebb4d5a9f9fff335b693f4efcc97882fe04eefd7
Reviewed-on: https://go-review.googlesource.com/22950
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Fold MOVDaddr ops into MOVXstorezero ops.
Also fold ADDconst into MOVDaddr so we're sure there isn't
(MOVDstorezero (ADDconst (MOVDaddr ..)))
Without this CL, we get:
v1 = MOVDaddr {s}
v2 = VARDEF {s}
v3 = MOVDstorezero v1 v2
The liveness pass thinks the MOVDaddr is a read of s, so s is
incorrectly thought to be live at the start of the function.
Fixes#17194
Change-Id: I2b4a2f13b12aa5b072941ee1c7b89f3793650cdc
Reviewed-on: https://go-review.googlesource.com/30086
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Michael Munday <munday@ca.ibm.com>
Fixes#4215.
Fixes#6750.
Improves the error message for wrong number of arguments by comparing
the signature of the return call site arguments, versus the function's
expected return arguments.
In this CL, the signature representation of:
+ ideal numbers(TIDEAL) ie float*, complex*, rune, int is
"number" instead of "untyped number".
+ idealstring is "string" instead of "untyped string".
+ idealbool is "bool" instead of "untyped bool".
However, the representation of other types remains as the compiler
would produce.
* Example 1(in the error messages, if all lines were printed):
$ cat main.go && go run main.go
package main
func foo() (int, int) {
return 2.3
}
func foo2() {
return int(2), 2
}
func foo3(v int) (a, b, c, d int) {
if v >= 5 {
return 1
}
return 2, 3
}
func foo4(name string) (string, int) {
switch name {
case "cow":
return "moo"
case "dog":
return "dog", 10, true
case "fish":
return ""
default:
return "lizard", 10
}
}
type S int
type T string
type U float64
func foo5() (S, T, U) {
if false {
return ""
} else {
ptr := new(T)
return ptr
}
return new(S), 12.34, 1 + 0i, 'r', true
}
func foo6() (T, string) {
return "T"
}
./issue4215.go:4: not enough arguments to return, got (number) want (int, int)
./issue4215.go:8: too many arguments to return, got (int, number) want ()
./issue4215.go:13: not enough arguments to return, got (number) want (int, int, int, int)
./issue4215.go:15: not enough arguments to return, got (number, number) want (int, int, int, int)
./issue4215.go:21: not enough arguments to return, got (string) want (string, int)
./issue4215.go:23: too many arguments to return, got (string, number, bool) want (string, int)
./issue4215.go:25: not enough arguments to return, got (string) want (string, int)
./issue4215.go:37: not enough arguments to return, got (string) want (S, T, U)
./issue4215.go:40: not enough arguments to return, got (*T) want (S, T, U)
./issue4215.go:42: too many arguments to return, got (*S, number, number, number, bool) want (S, T, U)
./issue4215.go:46: not enough arguments to return, got (string) want (T, string)
./issue4215.go:46: too many errors
* Example 2:
$ cat 6750.go && go run 6750.go
package main
import "fmt"
func printmany(nums ...int) {
for i, n := range nums {
fmt.Printf("%d: %d\n", i, n)
}
fmt.Printf("\n")
}
func main() {
printmany(1, 2, 3)
printmany([]int{1, 2, 3}...)
printmany(1, "abc", []int{2, 3}...)
}
./issue6750.go:15: too many arguments in call to printmany, got (number, string, []int) want (...int)
Change-Id: I6fdce78553ae81770840070e2c975d3e3c83d5d8
Reviewed-on: https://go-review.googlesource.com/25156
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
When processing a fallthrough, the casebody function in swt.go
checks that the last statement has indeed Op == OXFALL (not-processed
fallthrough) before setting it to OFALL (processed fallthrough).
Unfortunately, sometimes the fallthrough statement won't be in the
last node. For example, in
case 0:
return func() int {return 1}()
fallthrough
the compiler generates
autotmp_0 = (func literal)(); return autotmp_0; fallthrough; <node VARKILL>
with an OVARKILL node in the last position. casebody will find that
last.Op != OXFALL, won't mark the fallthrough as processed, and the
fallthrough line will cause a "fallthrough statement out of place" error.
To fix this, we change casebody so that it searches for the fallthrough
statement backwards in the statements list, without assuming that it'll
be in the last position.
Fixes#13262
Change-Id: I366c6caa7fd7442d365bd7a08cc66a552212d9b2
Reviewed-on: https://go-review.googlesource.com/22921
Run-TryBot: Quentin Smith <quentin@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Quentin Smith <quentin@golang.org>
We're dropping this behavior in favor of runtime.KeepAlive.
Implement runtime.KeepAlive as an intrinsic.
Update #15843
Change-Id: Ib60225bd30d6770ece1c3c7d1339a06aa25b1cbc
Reviewed-on: https://go-review.googlesource.com/28310
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
staticassign unwraps all CONVNOPs.
However, in the included test, we need the
CONVNOP for everything to typecheck.
Stop unwrapping unnecessarily.
The code we generate for this example is
suboptimal, but that's not new; see #17113.
Fixes#17111.
Change-Id: I29532787a074a6fe19a5cc53271eb9c84bf1b576
Reviewed-on: https://go-review.googlesource.com/29213
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
When possible, emit static data rather than
init functions for interface values.
This:
* cuts 32k off cmd/go
* removes several error values from runtime init
* cuts the size of the image/color/palette compiled package from 103k to 34k
* reduces the time to build the package in #15520 from 8s to 1.5s
Fixes#6289Fixes#15528
Change-Id: I317112da17aadb180c958ea328ab380f83e640b4
Reviewed-on: https://go-review.googlesource.com/26668
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Includes test case shown to fail with unpatched compiler.
Fixes#17005.
Change-Id: I49b7b1a3f02736d85846a2588018b73f68d50320
Reviewed-on: https://go-review.googlesource.com/28573
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
It was fixed earlier in the Go 1.8 cycle.
Add a test.
Fixes#15895
Change-Id: I5834831235d99b9fcf21b435932cdd7ac6dc2c6e
Reviewed-on: https://go-review.googlesource.com/28476
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
make(T, n, m) returns a slice of type T with length n and capacity m
where "The size arguments n and m must be of integer type or untyped."
https://tip.golang.org/ref/spec#Making_slices_maps_and_channels
The failure to reject typed non-integer size arguments in make
during compile time was uncovered after https://golang.org/cl/27851
changed the generation of makeslice calls.
Fixes #16940
Updates #16949
Change-Id: Ib1e3576f0e6ad199c9b16b7a50c2db81290c63b4
Reviewed-on: https://go-review.googlesource.com/28301
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Intrinsified atomic op produces <value,memory>. Make sure this
memory is considered in the store chain calculation.
Fixes#16948.
Change-Id: I029f164b123a7e830214297f8373f06ea0bf1e26
Reviewed-on: https://go-review.googlesource.com/28350
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Generate a for loop for ranging over strings that only needs to call
the runtime function charntorune for non ASCII characters.
This provides faster iteration over ASCII characters and slightly
faster iteration for other characters.
The runtime function charntorune is changed to take an index from where
to start decoding and returns the index after the last byte belonging
to the decoded rune.
All call sites of charntorune in the runtime are replaced by a for loop
that will be transformed by the compiler instead of calling the charntorune
function directly.
go binary size decreases by 80 bytes.
godoc binary size increases by around 4 kilobytes.
runtime:
name old time/op new time/op delta
RuneIterate/range/ASCII-4 43.7ns ± 3% 10.3ns ± 4% -76.33% (p=0.000 n=44+45)
RuneIterate/range/Japanese-4 72.5ns ± 2% 62.8ns ± 2% -13.41% (p=0.000 n=49+50)
RuneIterate/range1/ASCII-4 43.5ns ± 2% 10.4ns ± 3% -76.18% (p=0.000 n=50+50)
RuneIterate/range1/Japanese-4 72.5ns ± 2% 62.9ns ± 2% -13.26% (p=0.000 n=50+49)
RuneIterate/range2/ASCII-4 43.5ns ± 3% 10.3ns ± 2% -76.22% (p=0.000 n=48+47)
RuneIterate/range2/Japanese-4 72.4ns ± 2% 62.7ns ± 2% -13.47% (p=0.000 n=50+50)
strings:
name old time/op new time/op delta
IndexRune-4 64.7ns ± 5% 22.4ns ± 3% -65.43% (p=0.000 n=25+21)
MapNoChanges-4 269ns ± 2% 157ns ± 2% -41.46% (p=0.000 n=23+24)
Fields-4 23.0ms ± 2% 19.7ms ± 2% -14.35% (p=0.000 n=25+25)
FieldsFunc-4 23.1ms ± 2% 19.6ms ± 2% -14.94% (p=0.000 n=25+24)
name old speed new speed delta
Fields-4 45.6MB/s ± 2% 53.2MB/s ± 2% +16.87% (p=0.000 n=24+25)
FieldsFunc-4 45.5MB/s ± 2% 53.5MB/s ± 2% +17.57% (p=0.000 n=25+24)
Updates #13162
Change-Id: I79ffaf828d82bf9887592f08e5cad883e9f39701
Reviewed-on: https://go-review.googlesource.com/27853
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Martin Möhrmann <martisch@uos.de>
Where possible generate calls to runtime makeslice with int arguments
during compile time instead of makeslice with int64 arguments.
This eliminates converting arguments for calls to makeslice with
int64 arguments for platforms where int64 values do not fit into
arguments of type int.
godoc 386 binary shrinks by approximately 12 kilobyte.
amd64:
name old time/op new time/op delta
MakeSlice-2 29.8ns ± 1% 29.8ns ± 1% ~ (p=1.000 n=24+24)
386:
name old time/op new time/op delta
MakeSlice-2 52.3ns ± 0% 45.9ns ± 0% -12.17% (p=0.000 n=25+22)
Fixes #15357
Change-Id: Icb8701bb63c5a83877d26c8a4b78e782ba76de7c
Reviewed-on: https://go-review.googlesource.com/27851
Run-TryBot: Martin Möhrmann <martisch@uos.de>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The compiler was canonicalizing unnamed types of the form
struct { i int }
across packages, even though an unexported field i should not be
accessible from other packages.
The fix requires both qualifying the field name in the string used by
the compiler to distinguish the type, and ensuring the struct's pkgpath
is set in the rtype version of the data when the type being written is
not part of the localpkg.
Fixes#16616
Change-Id: Ibab160b8b5936dfa47b17dbfd48964a65586785b
Reviewed-on: https://go-review.googlesource.com/27791
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This CL reworks walkcompare for clarity and concision.
It also makes one significant functional change.
(The functional change is hard to separate cleanly
from the cleanup, so I just did them together.)
When inlining and unrolling an equality comparison
for a small struct or array, compare the elements like:
a[0] == b[0] && a[1] == b[1]
rather than
pa := &a
pb := &b
pa[0] == pb[0] && pa[1] == pb[1]
The result is the same, but taking the address
and working through the indirect
forces the backends to generate less efficient code.
This is only an improvement with the SSA backend.
However, every port but s390x now has a working
SSA backend, and switching to the SSA backend
by default everywhere is a priority for Go 1.8.
It thus seems reasonable to start to prioritize
SSA performance over the old backend.
Updates #15303
Sample code:
type T struct {
a, b int8
}
func g(a T) bool {
return a == T{1, 2}
}
SSA before:
"".g t=1 size=80 args=0x10 locals=0x8
0x0000 00000 (badeq.go:7) TEXT "".g(SB), $8-16
0x0000 00000 (badeq.go:7) SUBQ $8, SP
0x0004 00004 (badeq.go:7) FUNCDATA $0, gclocals·23e8278e2b69a3a75fa59b23c49ed6ad(SB)
0x0004 00004 (badeq.go:7) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
0x0004 00004 (badeq.go:8) MOVBLZX "".a+16(FP), AX
0x0009 00009 (badeq.go:8) MOVB AL, "".autotmp_0+6(SP)
0x000d 00013 (badeq.go:8) MOVBLZX "".a+17(FP), AX
0x0012 00018 (badeq.go:8) MOVB AL, "".autotmp_0+7(SP)
0x0016 00022 (badeq.go:8) MOVB $0, "".autotmp_1+4(SP)
0x001b 00027 (badeq.go:8) MOVB $1, "".autotmp_1+4(SP)
0x0020 00032 (badeq.go:8) MOVB $2, "".autotmp_1+5(SP)
0x0025 00037 (badeq.go:8) MOVBLZX "".autotmp_0+6(SP), AX
0x002a 00042 (badeq.go:8) MOVBLZX "".autotmp_1+4(SP), CX
0x002f 00047 (badeq.go:8) CMPB AL, CL
0x0031 00049 (badeq.go:8) JNE 70
0x0033 00051 (badeq.go:8) MOVBLZX "".autotmp_0+7(SP), AX
0x0038 00056 (badeq.go:8) CMPB AL, $2
0x003a 00058 (badeq.go:8) SETEQ AL
0x003d 00061 (badeq.go:8) MOVB AL, "".~r1+24(FP)
0x0041 00065 (badeq.go:8) ADDQ $8, SP
0x0045 00069 (badeq.go:8) RET
0x0046 00070 (badeq.go:8) MOVB $0, AL
0x0048 00072 (badeq.go:8) JMP 61
SSA after:
"".g t=1 size=32 args=0x10 locals=0x0
0x0000 00000 (badeq.go:7) TEXT "".g(SB), $0-16
0x0000 00000 (badeq.go:7) NOP
0x0000 00000 (badeq.go:7) NOP
0x0000 00000 (badeq.go:7) FUNCDATA $0, gclocals·23e8278e2b69a3a75fa59b23c49ed6ad(SB)
0x0000 00000 (badeq.go:7) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
0x0000 00000 (badeq.go:8) MOVBLZX "".a+8(FP), AX
0x0005 00005 (badeq.go:8) CMPB AL, $1
0x0007 00007 (badeq.go:8) JNE 25
0x0009 00009 (badeq.go:8) MOVBLZX "".a+9(FP), CX
0x000e 00014 (badeq.go:8) CMPB CL, $2
0x0011 00017 (badeq.go:8) SETEQ AL
0x0014 00020 (badeq.go:8) MOVB AL, "".~r1+16(FP)
0x0018 00024 (badeq.go:8) RET
0x0019 00025 (badeq.go:8) MOVB $0, AL
0x001b 00027 (badeq.go:8) JMP 20
Change-Id: I120185d58012b7bbcdb1ec01225b5b08d0855d86
Reviewed-on: https://go-review.googlesource.com/22277
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Blank struct fields are regular unexported fields. Two
blank fields are different if they are from different
packages. In order to correctly differentiate them, the
compiler needs the package information. Add it to the
export data.
Fixes#15514.
Change-Id: I421aaca22b542fcd0d66b2d2db777249cad78df6
Reviewed-on: https://go-review.googlesource.com/27639
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Change-Id: I4faf9a55414e217f0c48528efb13ab8fdcd9bb16
Reviewed-on: https://go-review.googlesource.com/24845
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Add a type conversion to uintptr for untyped constants
before the conversion to unsafe.Pointer.
Fixes#16317
Change-Id: Ib85feccad1019e687e7eb6135890b64b82fb87fb
Reviewed-on: https://go-review.googlesource.com/27441
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This makes a bunch of changes to package syntax to tweak line numbers
for AST nodes. For example, short variable declaration statements are
now associated with the location of the ":=" token, and function calls
are associated with the location of the final ")" token. These help
satisfy many unit tests that assume the old parser's behavior.
Because many of these changes are questionable, they're guarded behind
a new "gcCompat" const to make them easy to identify and revisit in
the future.
A handful of remaining tests are too difficult to make behave
identically. These have been updated to execute with -newparser=0 and
comments explaining why they need to be fixed.
all.bash now passes with both the old and new parsers.
Change-Id: Iab834b71ca8698d39269f261eb5c92a0d55a3bf4
Reviewed-on: https://go-review.googlesource.com/27199
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This time with the cherry-pick from the proper patch of
the old CL.
Stack size increased.
Corrected NaN-comparison glitches.
Marked g register as clobbered by calls.
Fixed shared libraries.
live_ssa.go still disabled because of differences.
Presumably turning on more optimization will fix
both the stack size and the live_ssa.go glitches.
Enhanced debugging output for shared libs test.
Rebased onto master.
Updates #16010.
Change-Id: I40864faf1ef32c118fb141b7ef8e854498e6b2c4
Reviewed-on: https://go-review.googlesource.com/27159
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
In CSE if a tuple generator is CSE'd to a different block, its
selectors are copied to the same block. In this case, also CES
the copied selectors.
Test copied from Keith's CL 27202.
Fixes#16741.
Change-Id: I2fc8b9513d430f10d6104275cfff5fb75d3ef3d9
Reviewed-on: https://go-review.googlesource.com/27236
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Don't fold constant factors into a multiply
beyond the capacity of a MULQ instruction (32 bits).
Fixes#16733
Change-Id: Idc213c6cb06f7c94008a8cf9e60a9e77d085fd89
Reviewed-on: https://go-review.googlesource.com/27160
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
SSA compiler on AMD64 may spill Duff-adjusted address as scalar. If
the object is on stack and the stack moves, the spilled address become
invalid.
Making the spill pointer-typed does not work. The Duff-adjusted address
points to the memory before the area to be zeroed and may be invalid.
This may cause stack scanning code panic.
Fix it by doing Duff-adjustment in genValue, so the intermediate value
is not seen by the reg allocator, and will not be spilled.
Add a test to cover both cases. As it depends on allocation, it may
be not always triggered.
Fixes#16515.
Change-Id: Ia81d60204782de7405b7046165ad063384ede0db
Reviewed-on: https://go-review.googlesource.com/25309
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
If we don't mark them as needzero, we have a live pointer variable
containing possible garbage, which will baffle the GC.
Fixes#16249.
Change-Id: I7c423ceaca199ddd46fc2c23e5965e7973f07584
Reviewed-on: https://go-review.googlesource.com/24715
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The compiler was treating all global function literals as occurring in a
function named "glob", which caused a symbol name collision when there
was an actual function named "glob". Fixed by adding a period.
Fixes#16193.
Change-Id: I67792901a8ca04635ba41d172bfaee99944f594d
Reviewed-on: https://go-review.googlesource.com/24500
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Make sure the pointer to the heap copy of an output parameter is kept
live throughout the function. The function could panic at any point,
and then a defer could recover. Thus, we need the pointer to the heap
copy always available so the post-deferreturn code can copy the return
value back to the stack.
Before this CL, the pointer to the heap copy could be considered dead in
certain situations, like code which is reverse dominated by a panic call.
Fixes#16095.
Change-Id: Ic3800423e563670e5b567b473bf4c84cddb49a4c
Reviewed-on: https://go-review.googlesource.com/24213
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This is a regression from 1.6. The respective code in importimport
(export.go) was not exactly replicated with the new importer. Also
copied over the missing cyclic import check.
Added test cases.
Fixes#16133.
Change-Id: I1e0a39ff1275ca62a8054874294d400ed83fb26a
Reviewed-on: https://go-review.googlesource.com/24312
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
A straight conversion from a type T to an interface type I, where T does
not implement I, should always panic with an interface conversion error
that shows the missing method. This was not happening if the conversion
was done once using the comma-ok form (the result would not be OK) and
then again in a straight conversion. Due to an error in the runtime
package the second conversion was failing with a nil pointer
dereference.
Fixes#16130.
Change-Id: I8b9fca0f1bb635a6181b8b76de8c2385bb7ac2d2
Reviewed-on: https://go-review.googlesource.com/24284
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michel Lespinasse <walken@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This adds 8 bytes of binary size to every type that has methods. It is
the smallest change I could come up with for 1.7.
Fixes#16037
Change-Id: Ibe15c3165854a21768596967757864b880dbfeed
Reviewed-on: https://go-review.googlesource.com/24070
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Special case for rewriting OAS inits omitted OASWB, added
that and OAS2FUNC. The special case cannot be default case,
that causes racewalk to fail in horrible ways.
Fixes#16008.
Change-Id: Ie0d2f5735fe9d8255a109597b36d196d4f86703a
Reviewed-on: https://go-review.googlesource.com/23954
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The generated code for interface stubs sometimes just messes
with a few of the args and then tail-calls to the target routine.
The args that aren't explicitly modified appear to not be used.
But they are used, by the thing we're tail calling.
Fixes#16016
Change-Id: Ib9b3a8311bb714a201daee002885fcb59e0463fa
Reviewed-on: https://go-review.googlesource.com/23960
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This matches the behavior of the legacy backend.
Fixes#15975 (if this is the intended behavior)
Change-Id: Id277959069b8b8bf9958fa8f2cbc762c752a1a19
Reviewed-on: https://go-review.googlesource.com/23820
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Post-liveness fix, the slices on both sides can now be
indirects of & variables. The cgen code handles those
cases just fine.
Fixes#15988
Change-Id: I378ad1d5121587e6107a9879c167291a70bbb9e4
Reviewed-on: https://go-review.googlesource.com/23863
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Make sure auto names don't conflict with function names. Before this CL,
we confused name a.len (the len field of the slice a) with a.len (the function
len declared on a).
Fixes#15961
Change-Id: I14913de697b521fb35db9a1b10ba201f25d552bb
Reviewed-on: https://go-review.googlesource.com/23789
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Adding a .def suffix for DWARF info collided with the DWARF info,
without the suffix, for a method named def. Change the suffix to ..def
instead.
Fixes#15926.
Change-Id: If1bf1bcb5dff1d7f7b79f78e3f7a3bbfcd2201bb
Reviewed-on: https://go-review.googlesource.com/23733
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
If memory might be unaligned, zero it one byte at a time
instead of 4 bytes at a time.
Fixes#15902
Change-Id: I4eff0840e042e2f137c1a4028f08793eb7dfd703
Reviewed-on: https://go-review.googlesource.com/23587
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
The liveness computation of parameters generally was never
correct, but forcing all parameters to be live throughout the
function covered up that problem. The new SSA back end is
too clever: even though it currently keeps the parameter values live
throughout the function, it may find optimizations that mean
the current values are not written back to the original parameter
stack slots immediately or ever (for example if a parameter is set
to nil, SSA constant propagation may replace all later uses of the
parameter with a constant nil, eliminating the need to write the nil
value back to the stack slot), so the liveness code must now
track the actual operations on the stack slots, exposing these
problems.
One small problem in the handling of arguments is that nodarg
can return ONAME PPARAM nodes with adjusted offsets, so that
there are actually multiple *Node pointers for the same parameter
in the instruction stream. This might be possible to correct, but
not in this CL. For now, we fix this by using n.Orig instead of n
when considering PPARAM and PPARAMOUT nodes.
The major problem in the handling of arguments is general
confusion in the liveness code about the meaning of PPARAM|PHEAP
and PPARAMOUT|PHEAP nodes, especially as contrasted with PAUTO|PHEAP.
The difference between these two is that when a local variable "moves"
to the heap, it's really just allocated there to start with; in contrast,
when an argument moves to the heap, the actual data has to be copied
there from the stack at the beginning of the function, and when a
result "moves" to the heap the value in the heap has to be copied
back to the stack when the function returns
This general confusion is also present in the SSA back end.
The PHEAP bit worked decently when I first introduced it 7 years ago (!)
in 391425ae. The back end did nothing sophisticated, and in particular
there was no analysis at all: no escape analysis, no liveness analysis,
and certainly no SSA back end. But the complications caused in the
various downstream consumers suggest that this should be a detail
kept mainly in the front end.
This CL therefore eliminates both the PHEAP bit and even the idea of
"heap variables" from the back ends.
First, it replaces the PPARAM|PHEAP, PPARAMOUT|PHEAP, and PAUTO|PHEAP
variable classes with the single PAUTOHEAP, a pseudo-class indicating
a variable maintained on the heap and available by indirecting a
local variable kept on the stack (a plain PAUTO).
Second, walkexpr replaces all references to PAUTOHEAP variables
with indirections of the corresponding PAUTO variable.
The back ends and the liveness code now just see plain indirected
variables. This may actually produce better code, but the real goal
here is to eliminate these little-used and somewhat suspect code
paths in the back end analyses.
The OPARAM node type goes away too.
A followup CL will do the same to PPARAMREF. I'm not sure that
the back ends (SSA in particular) are handling those right either,
and with the framework established in this CL that change is trivial
and the result clearly more correct.
Fixes#15747.
Change-Id: I2770b1ce3cbc93981bfc7166be66a9da12013d74
Reviewed-on: https://go-review.googlesource.com/23393
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The importer had several bugs with respect to labels and gotos:
- it didn't create a new ONAME node for label names (label dcl,
goto, continue, and break)
- it overwrote the symbol for gotos with the dclstack
- it didn't set the dclstack for labels
In the process changed export format slightly to always assume
a label name for labels and gotos, and never assume a label for
fallthroughs.
For fallthroughs and switch cases, now also set Xoffset like in
the parser. (Not setting it, i.e., using 0 was ok since this is
only used for verifying correct use of fallthroughs, which was
checked already. But it's an extra level of verification of the
import.)
Fixes#15838.
Change-Id: I3637f6314b8651c918df0c8cd70cd858c92bd483
Reviewed-on: https://go-review.googlesource.com/23445
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 21462 and CL 21463 made this message say explicitly that the problem
was a struct field in a map, but the word "directly" is unnecessary,
sounds wrong, and makes the error long.
Change-Id: I2fb68cdaeb8bd94776b8022cf3eae751919ccf6f
Reviewed-on: https://go-review.googlesource.com/23373
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Non-syntax errors are always counted to determine if to exit
early, but then deduplication eliminates them. This can lead
to situations which report "too many errors" and only one
error is shown.
De-duplicate non-syntax errors early, at least the ones that
appear consecutively, and only count the ones actually being
shown. This doesn't work perfectly as they may not appear in
sequence, but it's cheap and good enough.
Fixes#14136.
Change-Id: I7b11ebb2e1e082f0d604b88e544fe5ba967af1d7
Reviewed-on: https://go-review.googlesource.com/23259
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
They get rewritten to NEWs, and they must be marked as escaping
so walk doesn't try to allocate them back onto the stack.
Fixes#15733
Change-Id: I433033e737c3de51a9e83a5a273168dbc9110b74
Reviewed-on: https://go-review.googlesource.com/23223
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run live vars test only on ssa builds.
We can't just drop KeepAlive ops during regalloc. We need
to replace them with copies.
Change-Id: Ib4b3b1381415db88fdc2165fc0a9541b73ad9759
Reviewed-on: https://go-review.googlesource.com/23225
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Introduce a KeepAlive op which makes sure that its argument is kept
live until the KeepAlive. Use KeepAlive to mark pointer input
arguments as live after each function call and at each return.
We do this change only for pointer arguments. Those are the
critical ones to handle because they might have finalizers.
Doing compound arguments (slices, structs, ...) is more complicated
because we would need to track field liveness individually (we do
that for auto variables now, but inputs requires extra trickery).
Turn off the automatic marking of args as live. That way, when args
are explicitly nulled, plive will know that the original argument is
dead.
The KeepAlive op will be the eventual implementation of
runtime.KeepAlive.
Fixes#15277
Change-Id: I5f223e65d99c9f8342c03fbb1512c4d363e903e5
Reviewed-on: https://go-review.googlesource.com/22365
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Previously statements like
f(unsafe.Pointer(g()), int(h()))
would be reordered into a sequence of statements like
autotmp_g := g()
autotmp_h := h()
f(unsafe.Pointer(autotmp_g), int(autotmp_h))
which can leave g's temporary value on the stack as a uintptr, rather
than an unsafe.Pointer. Instead, recognize uintptr-to-unsafe.Pointer
conversions when reordering function calls to instead produce:
autotmp_g := unsafe.Pointer(g())
autotmp_h := h()
f(autotmp_g, int(autotmp_h))
Fixes#15329.
Change-Id: I2cdbd89d233d0d5c94791513a9fd5fd958d11ed5
Reviewed-on: https://go-review.googlesource.com/22273
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
See #15604. This was a bug in a CL that has since been
rolled back. Adding a test to challenge the next attempter.
Change-Id: Ic43be254ea6eaab0071018cdc61d9b1c21f19cbf
Reviewed-on: https://go-review.googlesource.com/23000
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
In regalloc, a sparse map is preallocated for later use by
spill-in-loop sinking. However, variables (spills) are added
during register allocation before spill sinking, and a map
query involving any of these new variables will index out of
bounds in the map.
To fix:
1) fix the queries to use s.orig[v.ID].ID instead, to ensure
proper indexing. Note that s.orig will be nil for values
that are not eligible for spilling (like memory and flags).
2) add a test.
Fixes#15585.
Change-Id: I8f2caa93b132a0f2a9161d2178320d5550583075
Reviewed-on: https://go-review.googlesource.com/22911
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The problem was fixed by the rollback in CL 22930.
This CL just adds a test to prevent regressions.
Fixes#15602
Change-Id: I37453f6e18ca43081266fe7f154c6d63fbaffd9b
Reviewed-on: https://go-review.googlesource.com/22931
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The new export format keeps track of all types that are exported.
If a type is seen that was exported before, only a reference to
that type is emitted. The importer maintains a list of all the
seen types and uses that list to resolve type references.
The existing compiler infrastructure's invariants assumes that
only named types are referred to before they are fully set up.
Referring to unnamed incomplete types causes problems. One of
the issues was #15548.
Added a new internal flag 'trackAllTypes' to enable/disable
this type tracking. With this change only named types are
tracked.
Verified that this fix also addresses #15548, even w/o the
prior fix for that issue (in fact that prior fix is turned
off if trackAllTypes is disabled because it's not needed).
The test for #15548 covers also this change.
For #15548.
Change-Id: Id0b3ff983629703d025a442823f99649fd728a56
Reviewed-on: https://go-review.googlesource.com/22839
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The boolean destination in an OAS2DOTTYPE expression craps out during
compilation when trying to assign to a map entry because, unlike slice entries,
map entries are not directly addressable in memory. The solution is to
properly order the boolean destination node so that map entries are set
via autotmp variables.
Fixes#14678
Change-Id: If344e8f232b5bdac1b53c0f0d21eeb43ab17d3de
Reviewed-on: https://go-review.googlesource.com/22833
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Accidentally checked in the version of file c.go that doesn't
exhibit the bug - hence the test was not testing the bug fix.
Double-checked that this version exposes the bug w/o the fix.
Change-Id: Ie4dc455229d1ac802a80164b5d549c2ad4d971f5
Reviewed-on: https://go-review.googlesource.com/22837
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
external linking is now supported.
Change-Id: I13e90c39dad86e60781adecdbe8e6bc9e522f740
Reviewed-on: https://go-review.googlesource.com/19811
Reviewed-by: Minux Ma <minux@golang.org>
The underlying issues have been fixed.
All the individual fixes have their own tests,
but it's still useful to have a plain source test.
Fixes#15084
Change-Id: I06c485a7d0716201bd57d1f3be53668dddd7ec14
Reviewed-on: https://go-review.googlesource.com/22426
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
The result of ODOTPTR, as well as a bunch of other ops,
should be the type of the result, not always a pointer type.
This fixes an amd64p32 bug where we were incorrectly truncating
a 64-bit slice index to 32 bits, and then barfing on a weird
load-64-bits-but-then-truncate-to-32-bits op that doesn't exist.
Fixes#15252
Change-Id: Ie62f4315fffd79f233e5449324ccc0879f5ac343
Reviewed-on: https://go-review.googlesource.com/22094
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Map keys are currently validated in multiple locations but share
a common validation routine. The problem is that early validations
should be lenient enough to allow for forward types while the final
validations should not. The final validations should fail on forward
types since they've already settled.
This change also separates the key type checking from the creation
of the map via typMap. Instead of the mapqueue being populated in
copytype() by checking the map line number, it's populated in the
same block that validates the key type. This isolates key validation
logic while type checking.
Fixes#14988
Change-Id: Ia47cf6213585d6c63b3a35249104c0439feae658
Reviewed-on: https://go-review.googlesource.com/21830
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Be more careful about inserting instrumentation in racewalk.
If the node being instrumented is an OAS, and it has a non-
empty Ninit, then append instrumentation to the Ninit list
rather than letting it be inserted before the OAS (and the
compilation of its init list). This deals with the case that
the Ninit list defines a variable used in the RHS of the OAS.
Fixes#15091.
Change-Id: Iac91696d9104d07f0bf1bd3499bbf56b2e1ef073
Reviewed-on: https://go-review.googlesource.com/21771
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: David Chase <drchase@google.com>
Make sure the results of unsigned constant-folded
shifts are sign-extended into the AuxInt field.
Fixes#15175
Change-Id: I3490d1bc3d9b2e1578ed30964645508577894f58
Reviewed-on: https://go-review.googlesource.com/21586
Reviewed-by: Alexandru Moșoi <alexandru@mosoi.ro>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Identify this assignment case and instead of the more general error
prog.go:6: cannot assign to students["sally"].age
produce
prog.go:6: cannot directly assign to struct field students["sally"].age in map
that explains why the assignment is not possible.
Fixes#13779.
Change-Id: I90c10b445f907834fc1735aa66e44a0f447aa74f
Reviewed-on: https://go-review.googlesource.com/21462
Reviewed-by: David Chase <drchase@google.com>
The issue was seen when inlining an exported function that contained
a fallthrough statement.
Fixes#15071
Change-Id: I1e8215ad49d57673dba7e8f8bd2ed8ad290dc452
Reviewed-on: https://go-review.googlesource.com/21452
Reviewed-by: Dave Cheney <dave@cheney.net>
Added a debug flag "-d closure" to explain compilation of
closures (should this be done some other way? Should we
rewrite the "-m" flag to "-d escapes"?) Used this to
discover that cause was an OXXX node in the captured vars
list, and in turn noticed that OXXX nodes are explicitly
ignored in all other processing of captured variables.
Couldn't figure out a reproducer, did verify that this OXXX
was not caused by an unnamed return value (which is one use
of these). Verified lack of heap allocation by examining -S
output.
Assembly:
(runtime/mgc.go:1371) PCDATA $0, $2
(runtime/mgc.go:1371) CALL "".notewakeup(SB)
(runtime/mgc.go:1377) LEAQ "".gcBgMarkWorker.func1·f(SB), AX
(runtime/mgc.go:1404) MOVQ AX, (SP)
(runtime/mgc.go:1404) MOVQ "".autotmp_2242+88(SP), CX
(runtime/mgc.go:1404) MOVQ CX, 8(SP)
(runtime/mgc.go:1404) LEAQ go.string."GC worker (idle)"(SB), AX
(runtime/mgc.go:1404) MOVQ AX, 16(SP)
(runtime/mgc.go:1404) MOVQ $16, 24(SP)
(runtime/mgc.go:1404) MOVB $20, 32(SP)
(runtime/mgc.go:1404) MOVQ $0, 40(SP)
(runtime/mgc.go:1404) PCDATA $0, $2
(runtime/mgc.go:1404) CALL "".gopark(SB)
Added a check for compiling_runtime to ensure that this is
caught in the future. Added a test to test the check.
Verified that 1.5.3 did NOT reject the test case when
compiled with -+ flag, so this is not a recently added bug.
Cause of bug is two-part -- there was no leaking closure
detection ever, and instead it relied on capture-of-variables
to trigger compiling_runtime test, but closures improved in
1.5.3 so that mere capture of a value did not also capture
the variable, which thus allowed closures to escape, as well
as this case where the escape was spurious. In
fixedbugs/issue14999.go, compare messages for f and g;
1.5.3 would reject g, but not f. 1.4 rejects both because
1.4 heap-allocates parameter x for both.
Fixes#14999.
Change-Id: I40bcdd27056810628e96763a44f2acddd503aee1
Reviewed-on: https://go-review.googlesource.com/21322
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The change in 20907 fixed varexpr but broke aliased. After that change,
a reference to a field in a struct would not be seen as aliasing itself.
Before that change, it would, but only because all fields in a struct
aliased everything.
This CL changes the compiler to consider all references to a field as
aliasing all other fields in that struct. This is imperfect--a
reference to one field does not alias another field--but is a simple fix
for the immediate problem. A better fix would require tracking the
specific fields as well.
Fixes#15042.
Change-Id: I5c95c0dd7b0699e53022fce9bae2e8f50d6d1d04
Reviewed-on: https://go-review.googlesource.com/21390
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Pushed from an old client by mistake. These are the
missing changes.
Change-Id: Ia8d61c5c0bde907369366ea9ea98711823342803
Reviewed-on: https://go-review.googlesource.com/21349
Reviewed-by: Todd Neal <todd@tneal.org>
We need to make sure all the bounds checks pass before issuing
a load which combines several others. We do this by issuing the
combined load at the last load's block, where "last" = closest to
the leaf of the dominator tree.
Fixes#15002
Change-Id: I7358116db1e039a072c12c0a73d861f3815d72af
Reviewed-on: https://go-review.googlesource.com/21246
Reviewed-by: Todd Neal <todd@tneal.org>
Previously, cmd/compile rejected constant int->string conversions if
the integer value did not fit into an "int" value. Also, runtime
incorrectly truncated 64-bit values to 32-bit before checking if
they're a valid Unicode code point. According to the Go spec, both of
these cases should instead yield "\uFFFD".
Fixes#15039.
Change-Id: I3c8a3ad9a0780c0a8dc1911386a523800fec9764
Reviewed-on: https://go-review.googlesource.com/21344
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This only tests amd64 because it's currently broken on non-SSA
backends.
Fixes#8613
Change-Id: I6bc501c81c395e533bb9c7335789750e0c6b7a8f
Reviewed-on: https://go-review.googlesource.com/21325
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This makes the rounding bug fix in math/big for issue 14651 available
to the compiler.
- changes to cmd/compile/internal/big fully automatic via script
- added test case for issue
- updated old test case with correct test data
Fixes#14651.
Change-Id: Iea37a2cd8d3a75f8c96193748b66156a987bbe40
Reviewed-on: https://go-review.googlesource.com/20818
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
An instruction consisting of all 0s causes an illegal instruction
signal on s390x. Since 0s are the default in this test this CL just
makes it explicit.
Change-Id: Id6e060eed1a588f4b10a4e4861709fcd19b434ac
Reviewed-on: https://go-review.googlesource.com/20962
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The biggest change is that each test is now responsible for managing
the starting and stopping of its parallel subtests.
The "Main" test could be run as a tRunner as well. This shows that
the introduction of subtests is merely a generalization of and
consistent with the current semantics.
Change-Id: Ibf8388c08f85d4b2c0df69c069326762ed36a72e
Reviewed-on: https://go-review.googlesource.com/18893
Reviewed-by: Russ Cox <rsc@golang.org>
Step 2 of stream-lining parameter parsing
- do parameter validity checks in parser
- two passes instead of multiple (and theoretically quadratic) passes
when checking parameters
- removes the need for OKEY and some ONONAME nodes in those passes
This removes allocation of ~123K OKEY (incl. some ONONAME) nodes
out of a total of ~10M allocated nodes when running make.bash, or
a reduction of the number of alloacted nodes by ~1.2%.
Change-Id: I4a8ec578d0ee2a7b99892ac6b92e56f8e0415f03
Reviewed-on: https://go-review.googlesource.com/20748
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
The location of VARDEFs is incorrect for PPARAMOUT variables
which are also used as temporary locations. We put in VARDEFs
when setting the variable at return time, but when the location
is also used as a temporary the lifetime values are wrong.
Fix copyelim to update the names map properly. This is a
real name bug fix which, as a result, allows me to
write a reasonable test to trigger the PPARAMOUT bug.
This is kind of a band-aid fix for #14591. A more pricipled
fix (which allows values to be stored in the return variable
earlier than the return point) will be harder.
Fixes#14591
Change-Id: I7df8ae103a982d1f218ed704c080d7b83cdcfdd9
Reviewed-on: https://go-review.googlesource.com/20457
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Make sure we do any just-before-return cleanup on all paths out of a
function, including when recovering. Each exit path should include
deferreturn (if there are any defers) and then the exit
code (e.g. copying heap-escaping return values back to the stack).
Introduce a Defer SSA block type which has two outgoing edges - one the
fallthrough edge (the defer was queued successfully) and one which
immediately returns (the defer had a successful recover() call and
normal execution should resume at the return point).
Fixes#14725
Change-Id: Iad035c9fd25ef8b7a74dafbd7461cf04833d981f
Reviewed-on: https://go-review.googlesource.com/20486
Reviewed-by: David Chase <drchase@google.com>
In increment and decrement statements, explicit check that the type
of operand is numeric earlier. This avoids a related but less clear
error about converting "1" to be emitted.
So, when compiling
package main
func main() {
var x bool
x++
}
instead of emitting two errors
prog.go:5: cannot convert 1 to type bool
prog.go:5: invalid operation: x++ (non-numeric type bool)
just emits the second error.
Fixes#12525.
Change-Id: I6e81330703765bef0d6eb6c57098c1336af7c799
Reviewed-on: https://go-review.googlesource.com/20245
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The new check corresponds to the (etype != TANY || Debug['A'] != 0)
that was lost in golang.org/cl/19936.
Fixes#14652.
Change-Id: Iec3788ff02529b3b0f0d4dd92ec9f3ef20aec849
Reviewed-on: https://go-review.googlesource.com/20271
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When the linker was written in C, command line arguments were passed
around as null-terminated byte arrays which encouraged checking
characters one at a time. In Go, that can easily lead to
out-of-bounds panics.
Use the more idiomatic strings.HasPrefix when checking cmd/link's -B
argument to avoid the panic, and replace the manual hex decode with
use of the encoding/hex package.
Fixes#14636
Change-Id: I45f765bbd8cf796fee1a9a3496178bf76b117827
Reviewed-on: https://go-review.googlesource.com/20211
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The changes to internal/big are completely automatic
by running vendor.bash in that directory.
Also added respective test case.
For #14553.
Change-Id: I98b124bcc9ad9e9bd987943719be27864423cb5d
Reviewed-on: https://go-review.googlesource.com/20199
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
