New special case for booleans and byte-sized integer types
converted to interfaces needs to ensure that the operand is
not too complex, if it were to appear in a parameter list
for example.
Added test, also increased the recursive node dump depth to
a level that was actually useful for an actual bug.
Fixes#19275.
Change-Id: If36ac3115edf439e886703f32d149ee0a46eb2a5
Reviewed-on: https://go-review.googlesource.com/37470
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This should help on the openbsd systems where the test mostly passes.
I don't expect it to help on s390x where the test reliably fails.
But it should give more information when it does fail.
For #19276.
Change-Id: I496c291f2b4b0c747b8dd4315477d87d03010059
Reviewed-on: https://go-review.googlesource.com/37348
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL 35562 substituted zerobase for the pointer for
interfaces containing zero-sized values.
However, it failed to evaluate the zero-sized value
expression for side-effects. Fix that.
The other similar interface value optimizations
are not affected, because they all actually use the
value one way or another.
Fixes#19246
Change-Id: I1168a99561477c63c29751d5cd04cf81b5ea509d
Reviewed-on: https://go-review.googlesource.com/37395
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The new syntax tree introduced with 1.8 represents send statements
(ch <- x) as statements; the old syntax tree represented them as
expressions (and parsed them as such) but complained if they were
used in expression context. As a consequence, some of the errors
that in the past were of the form "ch <- x used as value" now look
like "unexpected <- ..." because a "<-" is not valid according to
Go syntax in those situations. Accept the new error message.
Also: Fine-tune handling of misformed for loop headers.
Also: Minor cleanups/better comments.
Fixes#17590.
Change-Id: Ia541dea1f2f015c1b21f5b3ae44aacdec60a8aba
Reviewed-on: https://go-review.googlesource.com/37386
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The loop-A-encloses-loop-C code did not properly handle the
case where really C was already known to be enclosed by B,
and A was nearest-outer to B, not C.
Fixes#19217.
Change-Id: I755dd768e823cb707abdc5302fed39c11cdb34d4
Reviewed-on: https://go-review.googlesource.com/37340
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Avoid printing a second error message when a field of an undefined
variable is accessed.
Fixes#8440.
Change-Id: I3fe0b11fa3423cec3871cb01b5951efa8ea7451a
Reviewed-on: https://go-review.googlesource.com/36751
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#19012.
Fallback to return signatures without detailed types.
These error message will be of the form of issue:
* https://golang.org/issues/4215
* https://golang.org/issues/6750
So:
func f(x int, y uint) {
return x > y
}
f(10, "a" < 3)
will give errors:
too many errors to return
too many arguments in call to f
instead of:
too many errors to return
have (<T>)
want ()
too many arguments in call to f
have (number, <T>)
want (number, number)
Change-Id: I680abc7cdd8444400e234caddf3ff49c2d69f53d
Reviewed-on: https://go-review.googlesource.com/36806
Reviewed-by: Robert Griesemer <gri@golang.org>
Added a flag to generic and various architectures' atomic
operations that are judged to have observable side effects
and thus cannot be dead-code-eliminated.
Test requires GOMAXPROCS > 1 without preemption in loop.
Fixes#19182.
Change-Id: Id2230031abd2cca0bbb32fd68fc8a58fb912070f
Reviewed-on: https://go-review.googlesource.com/37333
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The rules for folding addresses into load/stores checks sym1 is
not on stack (because the stack offset is not known at that point).
But sym1 could be nil, which invalidates the check. Check merged
sym instead.
Fixes#19137.
Change-Id: I8574da22ced1216bb5850403d8f08ec60a8d1005
Reviewed-on: https://go-review.googlesource.com/37145
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Nil check removal in the same block is disabled due to issue 18725:
because the values are not ordered, a nilcheck may influence a
value that is logically before it. This CL re-enables same-block
nilcheck removal by ordering values in store order first.
Updates #18725.
Change-Id: I287a38525230c14c5412cbcdbc422547dabd54f6
Reviewed-on: https://go-review.googlesource.com/35496
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Until now, the parser set the position for each Node to the position of
the first token belonging to that node. For compatibility with the now
defunct gc parser, in many places that position information was modified
when the gcCompat flag was set (which it was, by default). Furthermore,
in some places, position information was not set at all.
This change removes the gcCompat flag and all associated code, and sets
position information for all nodes in a more principled way, as proposed
by mdempsky (see #16943 for details). Specifically, the position of a
node may not be at the very beginning of the respective production. For
instance for an Operation `a + b`, the position associated with the node
is the position of the `+`. Thus, for `a + b + c` we now get different
positions for the two additions.
This change does not pass toolstash -cmp because position information
recorded in export data and pcline tables is different. There are no
other functional changes.
Added test suite testing the position of all nodes.
Fixes#16943.
Change-Id: I3fc02bf096bc3b3d7d2fa655dfd4714a1a0eb90c
Reviewed-on: https://go-review.googlesource.com/37017
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
CL 35261 introduces special handling of zero-valued STRUCTLIT for
efficient struct zeroing. But it didn't cover all use cases, for
example, CONVNOP STRUCTLIT is not handled.
On the other hand, CL 34566 handles zeroing earlier, so we don't
need the change in CL 35261 for efficient zeroing. Other uses of
zero-valued struct literals are very rare. So undo the change in
walk.go in CL 35261.
Add a test for efficient zeroing.
Fixes#19084.
Change-Id: I0807f7423fb44d47bf325b3c1ce9611a14953853
Reviewed-on: https://go-review.googlesource.com/36955
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Use distinction between explicit and automatically inserted semicolons
to provide a better error message if the condition in an 'if' statement
is missing.
For #18747.
Change-Id: Iac167ae4e5ad53d2dc73f746b4dee9912434bb59
Reviewed-on: https://go-review.googlesource.com/36930
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Added missing nil-check. We will get rid of the gcCompat corrections
shortly but it's still worthwhile having the new test case added.
Fixes#19056.
Change-Id: I35bd938a4d789058da15724e34c05e5e631ecad0
Reviewed-on: https://go-review.googlesource.com/36908
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Add temporaries to reorder the assignment for OAS2XXX nodes.
This makes orderstmt(), rewrite
a, b, c = ...
as
tmp1, tmp2, tmp3 = ...
a, b, c = tmp1, tmp2, tmp3
and
a, ok = ...
as
t1, t2 = ...
a = t1
ok = t2
Fixes#13433.
Change-Id: Id0f5956e3a254d0a6f4b89b5f7b0e055b1f0e21f
Reviewed-on: https://go-review.googlesource.com/34713
Run-TryBot: Dhananjay Nakrani <dhananjayn@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The gcCompat mode was introduced to match the new parser's node position
setup exactly with the positions used by the original parser. Some of the
gcCompat adjustments were required to satisfy syntax error test cases,
and the rest were required to make toolstash cmp pass.
This change removes the former gcCompat adjustments and instead adjusts
the respective test cases as necessary. In some cases this makes the error
lines consistent with the ones reported by gccgo.
Where it has changed, the position associated with a given syntactic construct
is the position (line/col number) of the left-most token belonging to the
construct.
Change-Id: I5b60c00c5999a895c4d6d6e9b383c6405ccf725c
Reviewed-on: https://go-review.googlesource.com/36695
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Starting the error message with "expecting" rather than "missing"
causes the syntax error mechanism to add additional helpful info
(it recognizes "expecting" but not "missing").
Fixes#17328.
Change-Id: I8482ca5e5a6a6b22e0ed0d831b7328e264156334
Reviewed-on: https://go-review.googlesource.com/36637
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Towards better syntax error messages: With this change, the parser knows whether
a semicolon was an actual ';' in the source, or whether it was an automatically
inserted semicolon as result of a '\n' or EOF. Using this information in error
messages makes them more understandable.
For #17328.
Change-Id: I8cd9accee8681b62569d0ecef922d38682b401eb
Reviewed-on: https://go-review.googlesource.com/36636
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The compiler did not emit write barrier for assigning global with
struct literal, like global = T{} where T contains pointer.
The relevant code path is:
walkexpr OAS var_ OSTRUCTLIT
oaslit
anylit OSTRUCTLIT
walkexpr OAS var_ nil
return without adding write barrier
return true
break (without adding write barrier)
This CL makes oaslit not apply to globals. See also CL
https://go-review.googlesource.com/c/36355/ for an alternative
fix.
The downside of this is that it generates static data for zeroing
struct now. Also this only covers global. If there is any lurking
bug with implicit zeroing other than globals, this doesn't fix.
Fixes#18956.
Change-Id: Ibcd27e4fae3aa38390ffa94a32a9dd7a802e4b37
Reviewed-on: https://go-review.googlesource.com/36410
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
For code such as
if a := 10 { ...
the 1.7 compiler reported
a := 10 used as value
while the 1.8 compiler reported
invalid condition, tag, or type switch guard
Changed the error message to match the 1.7 compiler.
Fixes#18915.
Change-Id: I01308862e461922e717f9f8295a9db53d5a914eb
Reviewed-on: https://go-review.googlesource.com/36470
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We shouldn't use CONVNOP for conversions between two different
nonempty interface types, because we want to update the itab
in those situations.
Fixes#18595
After this CL, we are guaranteed that itabs are unique, that is
there is only one itab per compile-time-type/concrete type pair.
See also the tests in CL 35115 and 35116 which make sure this
invariant holds even for shared libraries and plugins.
Unique itabs are required for CL 34810 (faster type switch code).
R=go1.9
Change-Id: Id27d2e01ded706680965e4cb69d7c7a24ac2161b
Reviewed-on: https://go-review.googlesource.com/35119
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
This CL fixes two issues:
1. Load ops were initially always lowered to unsigned loads, even
for signed types. This was fine by itself however LoadReg ops
(used to re-load spilled values) were lowered to signed loads
for signed types. This meant that spills could invalidate
optimizations that assumed the original unsigned load.
2. Types were not always being maintained correctly through rules
designed to eliminate unnecessary zero and sign extensions.
Fixes#18906.
Change-Id: I95785dcadba03f7e3e94524677e7d8d3d3b9b737
Reviewed-on: https://go-review.googlesource.com/36256
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
If there is a defer, and that defer recovers, then the caller
can see all of the output parameters. That means that we must
mark all the output parameters live at any point which might panic.
If there is no defer then this is not necessary. This is implemented.
We could also detect whether there is a recover in any of the defers.
If not, we would need to mark only output params that the defer
actually references (and the closure mechanism already does that).
This is not implemented.
Fixes#18860.
Change-Id: If984fe6686eddce9408bf25e725dd17fc16b8578
Reviewed-on: https://go-review.googlesource.com/36030
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
When we discover a relation x <= len(s), also discover the relation
x <= cap(s). That way, in situations like:
a := s[x:] // tests 0 <= x <= len(s)
b := s[:x] // tests 0 <= x <= cap(s)
the second check can be eliminated.
Fixes#16813
Change-Id: Ifc037920b6955e43bac1a1eaf6bac63a89cfbd44
Reviewed-on: https://go-review.googlesource.com/33633
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alexandru Moșoi <alexandru@mosoi.ro>
Reviewed-by: David Chase <drchase@google.com>
CSE opportunities were being missed for commutative ops. We used to
order the args of commutative ops (by arg ID) once at the start of CSE.
But that may not be enough.
i1 = (Load ptr mem)
i2 = (Load ptr mem)
x1 = (Add i1 j)
x2 = (Add i2 j)
Equivalent commutative ops x1 and x2 may not get their args ordered in
the same way because because at the start of CSE, we don't know that
the i values will be CSEd. If x1 and x2 get opposite orders we won't
CSE them.
Instead, (re)order the args of commutative operations by their
equivalence class IDs each time we partition an equivalence class.
Change-Id: Ic609fa83b85299782a5e85bf93dc6023fccf4b0c
Reviewed-on: https://go-review.googlesource.com/33632
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Todd Neal <todd@tneal.org>
Fixes#10561.
Provides a better diagnostic message for failed type switch
satisfaction in the case that a value receiver is being used
in place of the pointer receiver that implements and satisfies
the interface.
Change-Id: If8c13ba13f2a8d81bf44bac7c3a66c12921ba921
Reviewed-on: https://go-review.googlesource.com/35235
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#8481.
Inform the user that init functions cannot be directly invoked
in user code, as mandated by the spec at:
http://golang.org/ref/spec#Program_initialization_and_execution.
Change-Id: Ib12c0c08718ffd48b76b6f9b13c76bb6612d2e7b
Reviewed-on: https://go-review.googlesource.com/34790
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#15055.
Updates exprfmt printing using fmt verb "%v" to check that n.Left
is non-nil before attempting to print it, otherwise we'll print
the nodes in the list using verb "%.v".
Credit to @mdempsky for this approach and for finding
the root cause of the issue.
Change-Id: I20a6464e916dc70d5565e145164bb9553e5d3865
Reviewed-on: https://go-review.googlesource.com/25361
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Make sure that the lack of an lvalue doesn't
cause extra side-effects.
Updates #18661
Updates #18739
Change-Id: I52eb4b4a5c6f8ff5cddd2115455f853c18112c19
Reviewed-on: https://go-review.googlesource.com/36126
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
When switching to the new parser, I changed cmd/compile to handle iota
per an intuitive interpretation of how nested constant declarations
should work (which also matches go/types).
Note: if we end up deciding that the current spec wording is
intentional (i.e., confirming gccgo's current behavior), the test will
need to be updated to expect 4 instead of 1.
Updates #15550.
Change-Id: I441f5f13209f172b73ef75031f2a9daa5e985277
Reviewed-on: https://go-review.googlesource.com/36122
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Whoever called toint() is expecting the {Mpint, Mpflt, Mpcplx} arg to
be converted to an integer expression, so it never makes sense to
report an error as "constant X truncated to real".
Fixes#11580
Change-Id: Iadcb105f0802358a7f77188c2b1e63fe80c5580c
Reviewed-on: https://go-review.googlesource.com/34638
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
For #18130.
f8b4123613 [dev.typealias] spec: use term 'embedded field' rather than 'anonymous field'
9ecc3ee252 [dev.typealias] cmd/compile: avoid false positive cycles from type aliases
49b7af8a30 [dev.typealias] reflect: add test for type aliases
9bbb07ddec [dev.typealias] cmd/compile, reflect: fix struct field names for embedded byte, rune
43c7094386 [dev.typealias] reflect: fix StructOf use of StructField to match StructField docs
9657e0b077 [dev.typealias] cmd/doc: update for type alias
de2e5459ae [dev.typealias] cmd/compile: declare methods after resolving receiver type
9259f3073a [dev.typealias] test: match gccgo error messages on alias2.go
5d92916770 [dev.typealias] cmd/compile: change Func.Shortname to *Sym
a7c884efc1 [dev.typealias] go/internal/gccgoimporter: support for type aliases
5802cfd900 [dev.typealias] cmd/compile: export/import test cases for type aliases
d7cabd40dd [dev.typealias] go/types: clarified doc string
cc2dcce3d7 [dev.typealias] cmd/compile: a few better comments related to alias types
5c160b28ba [dev.typealias] cmd/compile: improved error message for cyles involving type aliases
b2386dffa1 [dev.typealias] cmd/compile: type-check type alias declarations
ac8421f9a5 [dev.typealias] cmd/compile: various minor cleanups
f011e0c6c3 [dev.typealias] cmd/compile, go/types, go/importer: various alias related fixes
49de5f0351 [dev.typealias] cmd/compile, go/importer: define export format and implement importing of type aliases
5ceec42dc0 [dev.typealias] go/types: export TypeName.IsAlias so clients can use it
aa1f0681bc [dev.typealias] go/types: improved Object printing
c80748e389 [dev.typealias] go/types: remove some more vestiges of prior alias implementation
80d8b69e95 [dev.typealias] go/types: implement type aliases
a917097b5e [dev.typealias] go/build: add go1.9 build tag
3e11940437 [dev.typealias] cmd/compile: recognize type aliases but complain for now (not yet supported)
e0a05c274a [dev.typealias] cmd/gofmt: added test cases for alias type declarations
2e5116bd99 [dev.typealias] go/ast, go/parser, go/printer, go/types: initial type alias support
Change-Id: Ia65f2e011fd7195f18e1dce67d4d49b80a261203
When nilcheck runs, the values in a block are not in any particular
order. So any facts derived from examining the blocks shouldn't be
used until we reach the next block.
This is suboptimal as it won't eliminate nil checks within a block.
But it's probably a better fix for now as it is a much smaller change
than other strategies for fixing this bug.
nilptr3.go changes are mostly because for this pattern:
_ = *p
_ = *p
either nil check is fine to keep, and this CL changes which one
the compiler tends to keep.
There are a few regressions from code like this:
_ = *p
f()
_ = *p
For this pattern, after this CL we issue 2 nil checks instead of one.
(For the curious, this happens because intra-block nil check
elimination now falls to CSE, not nilcheck proper. The former
pattern has two nil checks with the same store argument. The latter
pattern has two nil checks with different store arguments.)
Fixes#18725
Change-Id: I3721b494c8bc9ba1142dc5c4361ea55c66920ac8
Reviewed-on: https://go-review.googlesource.com/35485
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Plus a few minor changes.
For #18130.
Change-Id: Ica6503fe9c888cc05c15b46178423f620c087491
Reviewed-on: https://go-review.googlesource.com/35233
Reviewed-by: Alan Donovan <adonovan@google.com>
Known issues:
- needs many more tests
- duplicate method declarations via type alias names are not detected
- type alias cycle error messages need to be improved
- need to review setup of byte/rune type aliases
For #18130.
Change-Id: Icc2fefad6214e5e56539a9dcb3fe537bf58029f8
Reviewed-on: https://go-review.googlesource.com/35121
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The test is inherently racy and vulnerable to starvation,
and within all.bash on some platforms that means it flakes.
Test is kept because it can be useful standalone to verify
behavior of GOEXPERIMENT=preeemptibleloops, and there is
likely to be further development of this feature in the
future.
There's also some question as to why it is flaking, because
though technically this is permitted, it's very odd in this
simple case.
Fixes#18589.
Change-Id: Ia0dd9037285c4a03122da4012c96981c9cc43b60
Reviewed-on: https://go-review.googlesource.com/35051
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
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>
Fixes#5790.
Fixes#18421.
* Lock in _ = x1/x2 divide by zero runtime panics since
it is actually evaluated and not discarded as in previous
versions before Go1.8.
* Update a test that was skipping over zerodivide tests
that expected runtime panics, enabling us to check for
the expected panics.
Change-Id: I0af0a6ecc19345fa9763ab2e35b275fb2d9d0194
Reviewed-on: https://go-review.googlesource.com/34712
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>
This fixes a bug in -l=3 or higher.
To inline a variadic function, the compiler generates code that constructs
a slice of arguments for the variadic parameter. Consider the function
func Foo(xs ...string)
and the call Foo("hello", "world"). To inline the call to Foo, the
compiler used to generate
xs := [2]string{"hello", "world"}[:]
which doesn't type check:
invalid operation [2]string literal[:] (slice of unaddressable value).
Now, the compiler generates
xs := []string{"hello", "world"}
which does type check.
Fixes#18116.
Change-Id: I0ee531ef2e6cc276db6fb12602b25a46d6d5db21
Reviewed-on: https://go-review.googlesource.com/33671
Reviewed-by: Keith Randall <khr@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>
Reason: Decision to back out current alias implementation.
Leaving import/export related code in place for now.
For #16339.
TBR=mdempsky
Change-Id: Ib0897cab2c1c3dc8a541f2efb9893271b0b0efe2
Reviewed-on: https://go-review.googlesource.com/32757
Reviewed-by: Robert Griesemer <gri@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>
The code to do the conversion is smaller than the
call to the runtime.
The 1-result asserts need to call panic if they fail, but that
code is out of line.
The only conversions left in the runtime are those which
might allocate and those which might need to generate an itab.
Given the following types:
type E interface{}
type I interface { foo() }
type I2 iterface { foo(); bar() }
type Big [10]int
func (b Big) foo() { ... }
This CL inlines the following conversions:
was assertE2T
var e E = ...
b := i.(Big)
was assertE2T2
var e E = ...
b, ok := i.(Big)
was assertI2T
var i I = ...
b := i.(Big)
was assertI2T2
var i I = ...
b, ok := i.(Big)
was assertI2E
var i I = ...
e := i.(E)
was assertI2E2
var i I = ...
e, ok := i.(E)
These are the remaining runtime calls:
convT2E:
var b Big = ...
var e E = b
convT2I:
var b Big = ...
var i I = b
convI2I:
var i2 I2 = ...
var i I = i2
assertE2I:
var e E = ...
i := e.(I)
assertE2I2:
var e E = ...
i, ok := e.(I)
assertI2I:
var i I = ...
i2 := i.(I2)
assertI2I2:
var i I = ...
i2, ok := i.(I2)
Fixes#17405Fixes#8422
Change-Id: Ida2367bf8ce3cd2c6bb599a1814f1d275afabe21
Reviewed-on: https://go-review.googlesource.com/32313
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@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>
- removes the runtime function stringtoslicebytetmp
- removes the generation of calls to stringtoslicebytetmp from the frontend
- adds handling of OSTRARRAYBYTETMP in the backend
This reduces binary sizes and avoids function call overhead.
Change-Id: Ib9988d48549cee663b685b4897a483f94727b940
Reviewed-on: https://go-review.googlesource.com/32158
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Martin Möhrmann <martisch@uos.de>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When we do
var x []byte = ...
y := x[i:]
We can't just use y.ptr = x.ptr + i, as the new pointer may point to the
next object in memory after the backing array.
We used to fix this by doing:
y.cap = x.cap - i
delta := i
if y.cap == 0 {
delta = 0
}
y.ptr = x.ptr + delta
That generates a branch in what is otherwise straight-line code.
Better to do:
y.cap = x.cap - i
mask := (y.cap - 1) >> 63 // -1 if y.cap==0, 0 otherwise
y.ptr = x.ptr + i &^ mask
It's about the same number of instructions (~4, depending on what
parts are constant, and the target architecture), but it is all
inline. It plays nicely with CSE, and the mask can be computed
in parallel with the index (in cases where a multiply is required).
It is a minor win in both speed and space.
Change-Id: Ied60465a0b8abb683c02208402e5bb7ac0e8370f
Reviewed-on: https://go-review.googlesource.com/32022
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This CL completes support for alias declarations in the compiler.
Also:
- increased export format version
- updated various comments
For #16339.
Fixes#17487.
Change-Id: Ic6945fc44c0041771eaf9dcfe973f601d14de069
Reviewed-on: https://go-review.googlesource.com/32090
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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>
Sometimes neither the src nor the dst of an escape edge
contains the line number appropriate to the edge, so add
a field so that can be set correctly.
Also updated some of the explanations to be less jargon-y
and perhaps more informative, and folded bug example into
test.
Cleaned up some of the function/method names in esc.go
and did a quick sanity check that each "bundling" function
was actually called often enough to justify its existence.
Fixes#17459.
Change-Id: Ieba53ab0a6ba1f7a6c4962bc0b702ede9cc3a3cc
Reviewed-on: https://go-review.googlesource.com/31660
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
When the compiler insert write barriers, the frontend makes
conservative decisions at an early stage. This may have false
positives which result in write barriers for stack writes.
A new phase, writebarrier, is added to the SSA backend, to delay
the decision and eliminate false positives. The frontend still
makes conservative decisions. When building SSA, instead of
emitting runtime calls directly, it emits WB ops (StoreWB,
MoveWB, etc.), which will be expanded to branches and runtime
calls in writebarrier phase. Writes to static locations on stack
are detected and write barriers are removed.
All write barriers of stack writes found by the script from
issue #17330 are eliminated (except two false positives).
Fixes#17330.
Change-Id: I9bd66333da9d0ceb64dcaa3c6f33502798d1a0f8
Reviewed-on: https://go-review.googlesource.com/31131
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
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>
It appears to be a vestigial holding ground for bugs.
But we have an issue tracker, and #1909 is there and open.
Change-Id: I912ff222a24c51fab483be0c67dad534f5a84488
Reviewed-on: https://go-review.googlesource.com/31859
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Previously, the check to make sure we only considered constant cases
for duplicates was skipping past integer ranges, because those use
n.List instead of n.Left. Thanks to Emmanuel Odeke for investigating
and helping to identify the root cause.
Fixes#17517.
Change-Id: I46fcda8ed9c346ff3a9647d50b83f1555587b740
Reviewed-on: https://go-review.googlesource.com/31716
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Adapt old test for prove's bounds check elimination.
Added missing rule to generic rules that lead to differences
between 32 and 64 bit platforms on sliceopt test.
Added debugging to prove.go that was helpful-to-necessary to
discover that missing rule.
Lowered debugging level on prove.go from 3 to 1; no idea
why it was previously 3.
Change-Id: I09de206aeb2fced9f2796efe2bfd4a59927eda0c
Reviewed-on: https://go-review.googlesource.com/23290
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@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>
This adds a //go:notinheap pragma for declarations of types that must
not be heap allocated. We ensure these rules by disallowing new(T),
make([]T), append([]T), or implicit allocation of T, by disallowing
conversions to notinheap types, and by propagating notinheap to any
struct or array that contains notinheap elements.
The utility of this pragma is that we can eliminate write barriers for
writes to pointers to go:notinheap types, since the write barrier is
guaranteed to be a no-op. This will let us mark several scheduler and
memory allocator structures as go:notinheap, which will let us
disallow write barriers in the scheduler and memory allocator much
more thoroughly and also eliminate some problematic hybrid write
barriers.
This also makes go:nowritebarrierrec and go:yeswritebarrierrec much
more powerful. Currently we use go:nowritebarrier all over the place,
but it's almost never what you actually want: when write barriers are
illegal, they're typically illegal for a whole dynamic scope. Partly
this is because go:nowritebarrier has been around longer, but it's
also because go:nowritebarrierrec couldn't be used in situations that
had no-op write barriers or where some nested scope did allow write
barriers. go:notinheap eliminates many no-op write barriers and
go:yeswritebarrierrec makes it possible to opt back in to write
barriers, so these two changes will let us use go:nowritebarrierrec
far more liberally.
This updates #13386, which is about controlling pointers from non-GC'd
memory to GC'd memory. That would require some additional pragma (or
pragmas), but could build on this pragma.
Change-Id: I6314f8f4181535dd166887c9ec239977b54940bd
Reviewed-on: https://go-review.googlesource.com/30939
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This pragma cancels the effect of go:nowritebarrierrec. This is useful
in the scheduler because there are places where we enter a function
without a valid P (and hence cannot have write barriers), but then
obtain a P. This allows us to annotate the function with
go:nowritebarrierrec and split out the part after we've obtained a P
into a go:yeswritebarrierrec function.
Change-Id: Ic8ce4b6d3c074a1ecd8280ad90eaf39f0ffbcc2a
Reviewed-on: https://go-review.googlesource.com/30938
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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>
To compile:
m[k] = v
instead of:
mapassign(maptype, m, &k, &v), do
do:
*mapassign(maptype, m, &k) = v
mapassign returns a pointer to the value slot in the map. It is just
like mapaccess except that it will allocate a new slot if k is not
already present in the map.
This makes map accesses faster but potentially larger (codewise).
It is faster because the write into the map is done when the compiler
knows the concrete type, so it can be done with a few store
instructions instead of calling typedmemmove. We also potentially
avoid stack temporaries to hold v.
The code can be larger when the map has pointers in its value type,
since there is a write barrier call in addition to the mapassign call.
That makes the code at the callsite a bit bigger (go binary is 0.3%
bigger).
This CL is in preparation for doing operations like m[k] += v with
only a single runtime call. That will roughly double the speed of
such operations.
Update #17133
Update #5147
Change-Id: Ia435f032090a2ed905dac9234e693972fe8c2dc5
Reviewed-on: https://go-review.googlesource.com/30815
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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>
This is a followup to issue #13805. That change avoid leaks for types that
don't have any pointers for the single assignment form of a dottype expression.
This does the same for the double assignment form.
Fixes#15796
Change-Id: I27474cade0ff1f3025cb6392f47b87b33542bc0f
Reviewed-on: https://go-review.googlesource.com/24906
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
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>
It tests the behavior of the old deleted compiler.
Fixes#17362.
Change-Id: Ia2fdec734c5cbe724a9de562ed71598f67244ab3
Reviewed-on: https://go-review.googlesource.com/30593
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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>
Update gc liveness to remove special conservative treatment
of ambiguously live vars, since there is no longer a need to
protect against GCDEBUG=gcdead.
Change-Id: Id6e2d03218f7d67911e8436d283005a124e6957f
Reviewed-on: https://go-review.googlesource.com/24896
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
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>
Mark nil check operations as faulting if their arg is zero.
This lets the late nilcheck pass remove duplicates.
Fixes#17242.
Change-Id: I4c9938d8a5a1e43edd85b4a66f0b34004860bcd9
Reviewed-on: https://go-review.googlesource.com/29952
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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>
Also adds the 'find leftmost one' instruction (FLOGR) and replaces the
WORD-encoded use of FLOGR in math/big with it.
Change-Id: I18e7cd19e75b8501a6ae8bd925471f7e37ded206
Reviewed-on: https://go-review.googlesource.com/29372
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@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>
No point in calling a function when we can build the interface
using a known type (or itab) and the address of a local.
Get rid of third arg (preallocated stack space) to convT2{I,E}.
Makes go binary smaller by 0.2%
benchmark old ns/op new ns/op delta
BenchmarkEfaceInteger-8 16.7 10.1 -39.52%
Update #17118
Update #15375
Change-Id: I9724a1f802bfa1e3957bf1856b55558278e198a2
Reviewed-on: https://go-review.googlesource.com/29373
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The compiler incorrectly will error when comparing a nil pointer
interface to a nil pointer of any other type. Example:
(*int)(nil) == interface{}(nil)
Will error with "gc: illegal constant expression: *int == interface {}"
Fixes#16702
Change-Id: I1a15d651df2cfca6762b1783a28b377b2e6ff8c6
Reviewed-on: https://go-review.googlesource.com/27591
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
ppc64 has an extraneous variable live in some situations.
We need a better tighten pass to get rid of this extra variable.
I'm working on it, but fix the test in the meantime.
Fixes build for ppc64.
Change-Id: I1efb9ccb234a64f2a1c228abd2b3195f67fbeb41
Reviewed-on: https://go-review.googlesource.com/29353
Reviewed-by: David Chase <drchase@google.com>
Unroll s == "ab" to
len(s) == 2 && s[0] == 'a' && s[1] == 'b'
This generates faster and shorter code
by avoiding a runtime call.
Do something similar for !=.
The cutoff length is 6. This was chosen empirically
by examining binary sizes on arm, arm64, 386, and amd64
using the SSA backend.
For all architectures examined, 4, 5, and 6 were
the ideal cutoff, with identical binary sizes.
The distribution of constant string equality sizes
during 'go build -a std' is:
40.81% 622 len 0
14.11% 215 len 4
9.45% 144 len 1
7.81% 119 len 3
7.48% 114 len 5
5.12% 78 len 7
4.13% 63 len 2
3.54% 54 len 8
2.69% 41 len 6
1.18% 18 len 10
0.85% 13 len 9
0.66% 10 len 14
0.59% 9 len 17
0.46% 7 len 11
0.26% 4 len 12
0.20% 3 len 19
0.13% 2 len 13
0.13% 2 len 15
0.13% 2 len 16
0.07% 1 len 20
0.07% 1 len 23
0.07% 1 len 33
0.07% 1 len 36
A cutoff of length 6 covers most of the cases.
Benchmarks on amd64 comparing a string to a constant of length 3:
Cmp/1same-8 4.78ns ± 6% 0.94ns ± 9% -80.26% (p=0.000 n=20+20)
Cmp/1diffbytes-8 6.43ns ± 6% 0.96ns ±11% -85.13% (p=0.000 n=20+20)
Cmp/3same-8 4.71ns ± 5% 1.28ns ± 5% -72.90% (p=0.000 n=20+20)
Cmp/3difffirstbyte-8 6.33ns ± 7% 1.27ns ± 7% -79.90% (p=0.000 n=20+20)
Cmp/3difflastbyte-8 6.34ns ± 8% 1.26ns ± 9% -80.13% (p=0.000 n=20+20)
The change to the prove test preserves the
existing intent of the test. When the string was
short, there was a new "proved in bounds" report
that referred to individual byte comparisons.
Change-Id: I593ac303b0d11f275672090c5c786ea0c6b8da13
Reviewed-on: https://go-review.googlesource.com/26758
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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>
Get rid of BlockCheck. Josh goaded me into it, and I went
down a rabbithole making it happen.
NilCheck now panics if the pointer is nil and returns void, as before.
BlockCheck is gone, and NilCheck is no longer a Control value for
any block. It just exists (and deadcode knows not to throw it away).
I rewrote the nilcheckelim pass to handle this case. In particular,
there can now be multiple NilCheck ops per block.
I moved all of the arch-dependent nil check elimination done as
part of ssaGenValue into its own proper pass, so we don't have to
duplicate that code for every architecture.
Making the arch-dependent nil check its own pass means I needed
to add a bunch of flags to the opcode table so I could write
the code without arch-dependent ops everywhere.
Change-Id: I419f891ac9b0de313033ff09115c374163416a9f
Reviewed-on: https://go-review.googlesource.com/29120
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
The change corrects the values of the largest float32 value (f1) and the
value of the halfway point between f1 and the next, overflow value (f2).
Fixes#17012
Change-Id: Idaf9997b69d61fafbffdb980d751c9857732e14d
Reviewed-on: https://go-review.googlesource.com/29171
Reviewed-by: Robert Griesemer <gri@golang.org>
