Given code such as
type T struct {
_ string
}
func f() {
var x = T{"space"}
// ...
}
the compiler rewrote the 'var x' line as
var x T
x._ = "space"
The compiler then rejected the assignment to
a blank field, thus rejecting valid code.
It also failed to catch a number of invalid assignments.
And there were insufficient checks for validity
when emitting static data, leading to ICEs.
To fix, check earlier for explicit blanks field names,
explicitly handle legit blanks in sinit,
and don't try to emit static data for nodes
for which typechecking has failed.
Fixes#19482
Change-Id: I594476171d15e6e8ecc6a1749e3859157fe2c929
Reviewed-on: https://go-review.googlesource.com/38006
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
- created new package cmd/compile/internal/types
- moved Pkg, Sym, Type to new package
- to break cycles, for now we need the (ugly) types/utils.go
file which contains a handful of functions that must be installed
early by the gc frontend
- to break cycles, for now we need two functions to convert between
*gc.Node and *types.Node (the latter is a dummy type)
- adjusted the gc's code to use the new package and the conversion
functions as needed
- made several Pkg, Sym, and Type methods functions as needed
- renamed constructors typ, typPtr, typArray, etc. to types.New,
types.NewPtr, types.NewArray, etc.
Passes toolstash-check -all.
Change-Id: I8adfa5e85c731645d0a7fd2030375ed6ebf54b72
Reviewed-on: https://go-review.googlesource.com/39855
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Instead of walking the list of nodes twice,
once to find static entries to add to an array
and once to find dynamic entries to generate code for,
do the split once up front, into two slices.
Then process each slice individually.
This makes the code easier to read
and more importantly, easier to modify.
While we're here, add a TODO to avoid
using temporaries for mapassign_fast calls.
It's not an important TODO;
the generated code would be basically identical.
It would just avoid a minor amount of
pointless SSA optimization work.
Passes toolstash-check.
No measureable compiler performance impact.
Updates #19751
Change-Id: I84a8f2c22f9025c718ef34639059d7bd02a3c406
Reviewed-on: https://go-review.googlesource.com/39351
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Minor cleanup.
This is the only such instance in the compiler.
Change-Id: I4e8ecde57d71867c7e1ac4d17e2154a91dd262b0
Reviewed-on: https://go-review.googlesource.com/39209
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Follow-up to review comments on CL 39193.
Change-Id: I7649af9d70ad73e039061a7a66fea416a7476192
Reviewed-on: https://go-review.googlesource.com/39199
Reviewed-by: Robert Griesemer <gri@golang.org>
The names never occur more than once,
so interning the results is counterproductive.
The impact is not very big, but neither is the fix.
name old time/op new time/op delta
Unicode 90.2ms ± 3% 88.3ms ± 5% -2.10% (p=0.000 n=94+98)
Change-Id: I1e3a24433db4ae0c9a6e98166568941824ff0779
Reviewed-on: https://go-review.googlesource.com/39193
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This makes the overall naming and use of the functions
to create a Type more consistent.
Passes toolstash -cmp.
Change-Id: Ie0d40b42cc32b5ecf5f20502675a225038ea40e4
Reviewed-on: https://go-review.googlesource.com/38354
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We can immediately emit static assignment data rather than queueing
them up to be processed during SSA building.
Passes toolstash -cmp.
Change-Id: I8bcea4b72eafb0cc0b849cd93e9cde9d84f30d5e
Reviewed-on: https://go-review.googlesource.com/37024
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Instead we can just call needwritebarrier when constructing the SSA
representation.
Change-Id: I6fefaad49daada9cdb3050f112889e49dca0047b
Reviewed-on: https://go-review.googlesource.com/34566
Reviewed-by: Cherry Zhang <cherryyz@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>
Change-Id: I429637ca91f7db4144f17621de851a548dc1ce76
Reviewed-on: https://go-review.googlesource.com/34923
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@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>
Various minor adjustments.
Change-Id: Iedfb97989f7bedaa3e9e8993b167e05f162434a7
Reviewed-on: https://go-review.googlesource.com/34136
Reviewed-by: David Lazar <lazard@golang.org>
In sinit.go, gdata can already handle strings and complex, so no
reason to handle them separately.
In obj.go, inline gdatastring and gdatacomplex into gdata, since it's
the only caller. Allows extracting out the common Linksym calls.
Passes toolstash -cmp.
Change-Id: I3cb18d9b4206a8a269c36e0d30a345d8e6caba1f
Reviewed-on: https://go-review.googlesource.com/31498
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Previously, we used OKEY nodes to represent keyed struct literal
elements. The field names were represented by an ONAME node, but this
is clumsy because it's the only remaining case where ONAME was used to
represent a bare identifier and not a variable.
This CL introduces a new OSTRUCTKEY node op for use in struct
literals. These ops instead store the field name in the node's own Sym
field. This is similar in spirit to golang.org/cl/20890.
Significant reduction in allocations for struct literal heavy code
like package unicode:
name old time/op new time/op delta
Template 345ms ± 6% 341ms ± 6% ~ (p=0.141 n=29+28)
Unicode 200ms ± 9% 184ms ± 7% -7.77% (p=0.000 n=29+30)
GoTypes 1.04s ± 3% 1.05s ± 3% ~ (p=0.096 n=30+30)
Compiler 4.47s ± 9% 4.49s ± 6% ~ (p=0.890 n=29+29)
name old user-ns/op new user-ns/op delta
Template 523M ±13% 516M ±17% ~ (p=0.400 n=29+30)
Unicode 334M ±27% 314M ±30% ~ (p=0.093 n=30+30)
GoTypes 1.53G ±10% 1.52G ±10% ~ (p=0.572 n=30+30)
Compiler 6.28G ± 7% 6.34G ±11% ~ (p=0.300 n=30+30)
name old alloc/op new alloc/op delta
Template 44.5MB ± 0% 44.4MB ± 0% -0.35% (p=0.000 n=27+30)
Unicode 39.2MB ± 0% 34.5MB ± 0% -11.79% (p=0.000 n=26+30)
GoTypes 125MB ± 0% 125MB ± 0% -0.12% (p=0.000 n=29+30)
Compiler 515MB ± 0% 515MB ± 0% -0.10% (p=0.000 n=29+30)
name old allocs/op new allocs/op delta
Template 426k ± 0% 424k ± 0% -0.39% (p=0.000 n=29+30)
Unicode 374k ± 0% 323k ± 0% -13.67% (p=0.000 n=29+30)
GoTypes 1.21M ± 0% 1.21M ± 0% -0.14% (p=0.000 n=29+29)
Compiler 4.40M ± 0% 4.39M ± 0% -0.13% (p=0.000 n=29+30)
Passes toolstash/buildall.
Change-Id: Iba4ee765dd1748f67e52fcade1cd75c9f6e13fa9
Reviewed-on: https://go-review.googlesource.com/30974
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Follow up to CL 29134. Generated with gofmt -r 'Nod -> nod', plus
three manual adjustments to the comments in syntax/parser.go
Change-Id: I02920f7ab10c70b6e850457b42d5fe35f1f3821a
Reviewed-on: https://go-review.googlesource.com/29136
Reviewed-by: Matthew Dempsky <mdempsky@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>
After the removal of the old backend many types are no longer referenced
outside internal/gc. Make these functions private so that tools like
honnef.co/go/unused can spot when they become dead code. In doing so
this CL identified several previously public helpers which are no longer
used, so removes them.
This should be the last of the public functions.
Change-Id: I7e9c4e72f86f391b428b9dddb6f0d516529706c3
Reviewed-on: https://go-review.googlesource.com/29134
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
After the removal of the old backend many types are no longer referenced
outside internal/gc. Make these functions private so that tools like
honnef.co/go/unused can spot when they become dead code. In doing so
this CL identified several previously public helpers which are no longer
used, so removes them.
Change-Id: Idc2d485f493206de9d661bd3cb0ecb4684177b32
Reviewed-on: https://go-review.googlesource.com/29133
Run-TryBot: Dave Cheney <dave@cheney.net>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
The ctxt parameter is always set to 0 on entry into anylit so make this
parameter a literal constant, and where possibly remove ctxt as a parameter
where it is known to be a constant zero.
Passes toolstash -cmp.
This is a re-creation of CL 28221 by Dave Cheney.
That CL was graciously reverted in CL 28480
to make merging other CLs easier.
Change-Id: If7a57bf0e27774d9890adbc30af9fabb4aff1058
Reviewed-on: https://go-review.googlesource.com/28483
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
They were almost identical.
Merge them and some of their calling code.
Passes toolstash -cmp.
Change-Id: I9e92a864a6c09c9e18ed52dc247a678467e344ba
Reviewed-on: https://go-review.googlesource.com/26754
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Does not pass toolstash -cmp due to changed export data,
but the cmd/go binary (which doesn't contain export data)
is bit-for-bit identical.
Change-Id: I6b12f9de18cf7da528e9207dccbf8f08c969f142
Reviewed-on: https://go-review.googlesource.com/26753
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The ctxt parameter is always set to 0 on entry into anylit so make this
parameter a literal constant, and where possibly remove ctxt as a parameter
where it is known to be a constant zero.
Change-Id: I3e76e06456d7b1a1ea875ffeb2efefa4a1ff5a7e
Reviewed-on: https://go-review.googlesource.com/28221
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
