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>
This enables better packing when key and value
types have different alignments.
Cuts 57k off cmd/go.
Change-Id: Ifdd125264caccd7852d622382c94e4689e757978
Reviewed-on: https://go-review.googlesource.com/26669
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Might as well tell the runtime how large the map is going to be.
This avoids grow work and allocations while the map is being built.
Will wait for 1.8.
Fixes#15880Fixes#16279
Change-Id: I377e3e5ec1e2e76ea2a50cc00810adda20ad0e79
Reviewed-on: https://go-review.googlesource.com/23558
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
As in the elimination of PHEAP|PPARAM in CL 23393,
this is something the front end can trivially take care of
and then not bother the back ends with.
It also eliminates some suspect (and only lightly exercised)
code paths in the back ends.
I don't have a smoking gun for this one but it seems
more clearly correct.
Change-Id: I3b3f5e669b3b81d091ff1e2fb13226a6f14c69d5
Reviewed-on: https://go-review.googlesource.com/23431
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Russ Cox <rsc@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>
As a nice side-effect, this allows us to
unify several code paths.
The terminology (low, high, max, simple slice expr,
full slice expr) is taken from the spec and
the examples in the spec.
This is a trial run. The plan, probably for Go 1.8,
is to change slice expressions to use Node.List
instead of OKEY, and to do some similar
tree structure changes for other ops.
Passes toolstash -cmp. No performance change.
all.bash passes with GO_GCFLAGS=-newexport.
Updates #15350
Change-Id: Ic1efdc36e79cdb95ae1636e9817a3ac8f83ab1ac
Reviewed-on: https://go-review.googlesource.com/22425
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
func f(x, y, z *int) {
a := []*int{x,y,z}
...
}
We used to use:
var tmp [3]*int
a := tmp[:]
a[0] = x
a[1] = y
a[2] = z
Now we do:
var tmp [3]*int
tmp[0] = x
tmp[1] = y
tmp[2] = z
a := tmp[:]
Doesn't sound like a big deal, but the compiler has trouble
eliminating write barriers when using the former method because it
doesn't know that the slice points to the stack. In the latter
method, the compiler knows the array is on the stack and as a result
doesn't emit any write barriers.
This turns out to be extremely common when building ... args, like
for calls fmt.Printf.
Makes go binaries ~1% smaller.
Doesn't have a measurable effect on the go1 fmt benchmarks,
unfortunately.
Fixes#14263
Update #6853
Change-Id: I9074a2788ec9e561a75f3b71c119b69f304d6ba2
Reviewed-on: https://go-review.googlesource.com/22395
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Instead of switching on Ctype (which internally uses a type switch)
and then scattering lots of type assertions throughout the CTFOO case
clauses, just use type switches directly on the underlying constant
value.
Passes toolstash/buildall.
Change-Id: I9bc172cc67e5f391cddc15539907883b4010689e
Reviewed-on: https://go-review.googlesource.com/22384
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Instead of using TARRAY for both arrays and slices, create a new
TSLICE kind to handle slices.
Also, get rid of the "DDDArray" distinction. While kinda ugly, it
seems likely we'll need to defer evaluating the constant bounds
expressions for golang.org/issue/13890.
Passes toolstash/buildall.
Change-Id: I8e45d4900e7df3a04cce59428ec8b38035d3cc3a
Reviewed-on: https://go-review.googlesource.com/22329
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously, isStaticCompositeLiteral would
return the wrong value for literals like:
[1]struct{ b []byte }{b: []byte{1}}
Note that the outermost component is an array,
but once we recurse into isStaticCompositeLiteral,
we never check again that arrays are actually arrays.
Instead of adding more logic to the guts of
isStaticCompositeLiteral, allow it to accept
any Node and return the correct answer.
Change-Id: I6af7814a9037bbc7043da9a96137fbee067bbe0e
Reviewed-on: https://go-review.googlesource.com/22247
Reviewed-by: Keith Randall <khr@golang.org>
*p = [5]byte{1,2,3,4,5}
First we allocate a global containing the RHS. Then we copy
that global to a local stack variable, and then copy that local
stack variable to *p. The intermediate copy is unnecessary.
Note that this only works if the RHS is completely constant.
If the code was:
*p = [5]byte{1,2,x,4,5}
this optimization doesn't apply as we have to construct the
RHS on the stack before copying it to *p.
Fixes#12841
Change-Id: I7cd0404ecc7a2d1750cbd8fe1222dba0fa44611f
Reviewed-on: https://go-review.googlesource.com/22192
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This removes all access to Type.Bound
from outside type.go.
Update sinit to make a new type rather than
copy and mutate.
Update bimport to create a new slice type
instead of mutating TDDDFIELD.
These are rare, so the extra allocs are nominal.
I’m not happy about having a setter,
but it appears the most practical route
forward at the moment, and it only has a few uses.
Passes toolstash -cmp.
Change-Id: I174f07c8f336afc656904bde4bdbde4f3ef0db96
Reviewed-on: https://go-review.googlesource.com/21423
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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>
Generated by eg, manually fixed up.
I’m not thrilled about having a setter,
but given the variety of contexts in which this
gets fiddled with, it is the cleanest
available alternative.
Change-Id: Ibdf23e638fe0bdabded014c9e59d557fab8c955f
Reviewed-on: https://go-review.googlesource.com/21341
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This allows us to get rid of Isptr and Issigned. Still some code to
clean up for Isint, Isfloat, and Iscomplex.
CL produced mechanically using gofmt -w -r.
Passes toolstash -cmp.
Change-Id: If4f807bb7f2b357288d2547be2380eb511875786
Reviewed-on: https://go-review.googlesource.com/21339
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Replace Isfixedarray, Isslice, and Isinter with the IsArray, IsSlice,
and IsInterface methods added for SSA. Rewrite performed mechanically
using gofmt -w -r "Isfoo(t) -> t.IsFoo()".
Because the IsFoo methods panic when given a nil pointer, a handful of
call sites had to be modified to check for nil Type values. These
aren't strictly necessary, because nil Type values should only occur
in invalid Go source programs, so it would be okay if we panicked on
them and gave up type checking the rest of the package. However, there
are a couple regress tests that expect we continue, so add checks to
keep those tests passing. (See #15029.)
Passes toolstash -cmp.
Change-Id: I511c6ac4cfdf3f9cbdb3e52a5fa91b6d09d82f80
Reviewed-on: https://go-review.googlesource.com/21336
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This removes almost all direct access to
Type’s heavily overloaded Type field.
Mostly generated by eg, manually checked.
Significant manual changes:
* reflect.go's typPkg used Type indiscriminately.
Use it only for specific etypes.
* gen.go's visitComponents contained a usage of Type
with structs. Using Type for structs no longer
occurs, and the Fatal contained therein has not triggered,
so it has been axed.
* Scary code in cgen.go's cgen_slice is now explicitly scary.
Passes toolstash -cmp.
Change-Id: I2dbfb3c959da7ae239f964d83898c204affcabc6
Reviewed-on: https://go-review.googlesource.com/21331
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
