golang.org/cl/174498 add ONAME case to isStaticCompositeLiteral, to
detect global variable as compile-time constant.
It does report wrong for struct field, e.g:
o := one{i: two{i: 42}.i}
field i in two{i: 42} was reported as static composite literal, while it
should not.
In general, adding ONAME case for isStaticCompositeLiteral is probably
wrong.
Fixes#31782
Change-Id: Icde7d43bbb002b75df5c52b948b7126a4265e07b
Reviewed-on: https://go-review.googlesource.com/c/go/+/174837
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The order pass now handles all the dynamic entries.
Update #26552
Followup to CL 174417
Change-Id: Ie924cadb0e0ba36c423868f654f13040100b44c6
Reviewed-on: https://go-review.googlesource.com/c/go/+/174498
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The spec carefully and consistently uses "key" and "element"
as map terminology. The implementation, not so much.
This change attempts to make the implementation consistently
hew to the spec's terminology. Beyond consistency, this has
the advantage of avoid some confusion and naming collisions,
since v and value are very generic and commonly used terms.
I believe that I found all everything, but there are a lot of
non-obvious places for these to hide, and grepping for them is hard.
Hopefully this change changes enough of them that we will start using
elem going forward. Any remaining hidden cases can be removed ad hoc
as they are discovered.
The only externally-facing part of this change is in package reflect,
where there is a minor doc change and a function parameter name change.
Updates #27167
Change-Id: I2f2d78f16c360dc39007b9966d5c2046a29d3701
Reviewed-on: https://go-review.googlesource.com/c/go/+/174523
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
For the code:
m := map[int]int {
a(): b(),
c(): d(),
e(): f(),
}
We used to do:
t1 := a()
t2 := b()
t3 := c()
t4 := d()
t5 := e()
t6 := f()
m := map[int]int{}
m[t1] = t2
m[t3] = t4
m[t5] = t6
After this CL we do:
m := map[int]int{}
t1 := a()
t2 := b()
m[t1] = t2
t3 := c()
t4 := d()
m[t3] = t4
t5 := e()
t6 := f()
m[t5] = t6
Ordering the initialization this way limits the lifetime of the
temporaries involved. In particular, for large maps the number of
simultaneously live temporaries goes from ~2*len(m) to ~2. This change
makes the compiler (regalloc, mostly) a lot happier. The compiler runs
faster and uses a lot less memory.
For #26546, changes compile time of a big map from 8 sec to 0.5 sec.
Fixes#26552
Update #26546
Change-Id: Ib7d202dead3feaf493a464779fd9611c63fcc25f
Reviewed-on: https://go-review.googlesource.com/c/go/+/174417
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
We already skipped blank field initialization in non-global contexts.
This change makes the global context treatment match.
Fixes#31546
Change-Id: I40acce49b0a9deb351ae0da098f4c114e425ec63
Reviewed-on: https://go-review.googlesource.com/c/go/+/173723
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This will make it easier for subsequent CLs to track additional state
during package initialization scheduling.
Passes toolstash-check.
Updates #22326.
Change-Id: I528792ad34f41a4be52951531eb7525a94c9f350
Reviewed-on: https://go-review.googlesource.com/c/go/+/169898
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When initializing a new object, we're often writing
1) to a location that doesn't have a pointer to a heap object
2) a pointer that doesn't point to a heap object
When both those conditions are true, we can avoid the write barrier.
This CL detects case 1 by looking for writes to known-zeroed
locations. The results of runtime.newobject are zeroed, and we
perform a simple tracking of which parts of that object are written so
we can determine what part remains zero at each write.
This CL detects case 2 by looking for addresses of globals (including
the types and itabs which are used in interfaces) and for nil pointers.
Makes cmd/go 0.3% smaller. Some particular cases, like the slice
literal in #29573, can get much smaller.
TODO: we can remove actual zero writes also with this mechanism.
Update #29573
Change-Id: Ie74a3533775ea88da0495ba02458391e5db26cb9
Reviewed-on: https://go-review.googlesource.com/c/go/+/156363
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
staticcopy of a struct or array should recursively call itself, not
staticassign.
This fixes an issue where a struct with a slice in it is copied during
static initialization. In this case, the backing array for the slice
is duplicated, and each copy of the slice refers to a different
backing array, which is incorrect. That issue has existed since at
least Go 1.2.
I'm not sure why this was never noticed. It seems like a pretty
obvious bug if anyone modifies the resulting slice.
In any case, we started to notice when the optimization in CL 140301
landed. Here is basically what happens in issue29013b.go:
1) The error above happens, so we get two backing stores for what
should be the same slice.
2) The code for initializing those backing stores is reused.
But not duplicated: they are the same Node structure.
3) The order pass allocates temporaries for the map operations.
For the first instance, things work fine and two temporaries are
allocated and stored in the OKEY nodes. For the second instance,
the order pass decides new temporaries aren't needed, because
the OKEY nodes already have temporaries in them.
But the order pass also puts a VARKILL of the temporaries between
the two instance initializations.
4) In this state, the code is technically incorrect. But before
CL 140301 it happens to work because the temporaries are still
correctly initialized when they are used for the second time. But then...
5) The new CL 140301 sees the VARKILLs and decides to reuse the
temporary for instance 1 map 2 to initialize the instance 2 map 1
map. Because the keys aren't re-initialized, instance 2 map 1
gets the wrong key inserted into it.
Fixes#29013
Change-Id: I840ce1b297d119caa706acd90e1517a5e47e9848
Reviewed-on: https://go-review.googlesource.com/c/152081
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
While here, rename nonnegintconst to indexconst (because that's
what it is) and add Fatalf calls where we are not expecting the
indexconst call to fail, and fixed wrong comparison in smallintconst.
Fixes#23781.
Change-Id: I86eb13081c450943b1806dfe3ae368872f76639a
Reviewed-on: https://go-review.googlesource.com/c/151599
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
When a slice composite literal is sparse, initialize it dynamically
instead of statically.
s := []int{5:5, 20:20}
To initialize the backing store for s, use 2 constant writes instead
of copying from a static array with 21 entries.
This CL also fixes pathologies in the compiler when the slice is
*very* sparse.
Fixes#23780
Change-Id: Iae95c6e6f6a0e2994675cbc750d7a4dd6436b13b
Reviewed-on: https://go-review.googlesource.com/c/151319
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change does a bulk rename of several identifiers in the compiler.
See #27167 and https://docs.google.com/document/d/19_ExiylD9MRfeAjKIfEsMU1_RGhuxB9sA0b5Zv7byVI/
for context and for discussion of these particular renames.
Commands run to generate this change:
gorename -from '"cmd/compile/internal/gc".OPROC' -to OGO
gorename -from '"cmd/compile/internal/gc".OCOM' -to OBITNOT
gorename -from '"cmd/compile/internal/gc".OMINUS' -to ONEG
gorename -from '"cmd/compile/internal/gc".OIND' -to ODEREF
gorename -from '"cmd/compile/internal/gc".OARRAYBYTESTR' -to OBYTES2STR
gorename -from '"cmd/compile/internal/gc".OARRAYBYTESTRTMP' -to OBYTES2STRTMP
gorename -from '"cmd/compile/internal/gc".OARRAYRUNESTR' -to ORUNES2STR
gorename -from '"cmd/compile/internal/gc".OSTRARRAYBYTE' -to OSTR2BYTES
gorename -from '"cmd/compile/internal/gc".OSTRARRAYBYTETMP' -to OSTR2BYTESTMP
gorename -from '"cmd/compile/internal/gc".OSTRARRAYRUNE' -to OSTR2RUNES
gorename -from '"cmd/compile/internal/gc".Etop' -to ctxStmt
gorename -from '"cmd/compile/internal/gc".Erv' -to ctxExpr
gorename -from '"cmd/compile/internal/gc".Ecall' -to ctxCallee
gorename -from '"cmd/compile/internal/gc".Efnstruct' -to ctxMultiOK
gorename -from '"cmd/compile/internal/gc".Easgn' -to ctxAssign
gorename -from '"cmd/compile/internal/gc".Ecomplit' -to ctxCompLit
Not altered: parameters and local variables (mostly in typecheck.go) named top,
which should probably now be called ctx (and which should probably have a named type).
Also not altered: Field called Top in gc.Func.
gorename -from '"cmd/compile/internal/gc".Node.Isddd' -to IsDDD
gorename -from '"cmd/compile/internal/gc".Node.SetIsddd' -to SetIsDDD
gorename -from '"cmd/compile/internal/gc".nodeIsddd' -to nodeIsDDD
gorename -from '"cmd/compile/internal/types".Field.Isddd' -to IsDDD
gorename -from '"cmd/compile/internal/types".Field.SetIsddd' -to SetIsDDD
gorename -from '"cmd/compile/internal/types".fieldIsddd' -to fieldIsDDD
Not altered: function gc.hasddd, params and local variables called isddd
Also not altered: fmt.go prints nodes using "isddd(%v)".
cd cmd/compile/internal/gc; go generate
I then manually found impacted comments using exact string match
and fixed them up by hand. The comment changes were trivial.
Passes toolstash-check.
Fixes#27167. If this experiment is deemed a success,
we will open a new tracking issue for renames to do
at the end of the 1.13 cycles.
Change-Id: I2dc541533d2ab0d06cb3d31d65df205ecfb151e8
Reviewed-on: https://go-review.googlesource.com/c/150140
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Instead of allocating a new temporary each time one
is needed, keep a list of temporaries which are free
(have already been VARKILLed on every path) and use
one of them.
Should save a lot of stack space. In a function like this:
func main() {
fmt.Printf("%d %d\n", 2, 3)
fmt.Printf("%d %d\n", 4, 5)
fmt.Printf("%d %d\n", 6, 7)
}
The three [2]interface{} arrays used to hold the ... args
all use the same autotmp, instead of 3 different autotmps
as happened previous to this CL.
Change-Id: I2d728e226f81e05ae68ca8247af62014a1b032d3
Reviewed-on: https://go-review.googlesource.com/c/140301
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The previous CL introduced stack objects. This CL removes the old
ambiguously live liveness analysis. After this CL we're relying
on stack objects exclusively.
Update a bunch of liveness tests to reflect the new world.
Fixes#22350
Change-Id: I739b26e015882231011ce6bc1a7f426049e59f31
Reviewed-on: https://go-review.googlesource.com/c/134156
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Node.copy used to make a shallow copy of a node. Often, this is not
correct: If a node n's Orig field pointed to itself, the copy's Orig
field has to be adjusted to point to the copy. Otherwise, if n is
modified later, the copy's Orig appears modified as well (because it
points to n).
This was fixed for one specific case with
https://go-review.googlesource.com/c/go/+/136395 (issue #26855).
This change instead addresses copy in general:
In two cases we don't want the Orig adjustment as it causes escape
analysis output to fail (not match the existing error messages).
rawcopy is used in those cases.
In several cases Orig is set to the copy immediately after making
a copy; a new function sepcopy is used there.
Updates #26855.
Fixes#27765.
Change-Id: Idaadeb5c4b9a027daabd46a2361348f7a93f2b00
Reviewed-on: https://go-review.googlesource.com/136540
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This reduces the API surface of Type slightly (for #25056), but also
makes it more consistent with the reflect and go/types APIs.
Passes toolstash-check.
Change-Id: Ief9a8eb461ae6e88895f347e2a1b7b8a62423222
Reviewed-on: https://go-review.googlesource.com/109138
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Focus on "isfoo" funcs that take a *Node, and conver them to isFoo
methods instead. This makes for more idiomatic Go code, and also more
readable func names.
Found candidates with grep, and applied most changes with sed. The funcs
chosen were isgoconst, isnil, and isblank. All had the same signature,
func(*Node) bool.
While at it, camelCase the isliteral and iszero function names. Don't
move these to methods, as they are only used in the backend part of gc,
which might one day be split into a separate package.
Passes toolstash -cmp on std cmd.
Change-Id: I4df081b12d36c46c253167c8841c5a841f1c5a16
Reviewed-on: https://go-review.googlesource.com/105555
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
When I added the Node.copy method, I converted most of the occurrences
but missed a few.
One of them, used only for gdata, was an unnecessary copy given that
gdata does not modify the node it is passed.
No allocation changes in compilebench.
Passes toolstash -cmp on std cmd.
Change-Id: I7fba5212377b75c6d6b3785e594a30568ff0732e
Reviewed-on: https://go-review.googlesource.com/104937
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
While at it, also simplify a couple of switches.
Doesn't pass toolstash -cmp on std cmd, because orderBlock(&n2.Nbody) is
moved further down to the n3 loop.
Change-Id: I20a2a6c21eb9a183a59572e0fca401a5041fc40a
Reviewed-on: https://go-review.googlesource.com/104416
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When making a shallow copy of a node, various methods were used,
including calling nod(OXXX, nil, nil) and then overwriting it, or
"n1 := *n" and then using &n1.
Add a copy method instead, simplifying all of those and making them
consistent.
Passes toolstash -cmp on std cmd.
Change-Id: I3f3fc88bad708edc712bf6d87214cda4ddc43b01
Reviewed-on: https://go-review.googlesource.com/72710
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Extract all rewrite-to-OLITERAL expressions to use a single setconst
helper function.
Does not pass toolstash-check for two reasons:
1) We now consistently clear Left/Right/etc when rewriting Nodes into
OLITERALs, which results in their inlining complexity being correctly
computed. So more functions can now be inlined.
2) We preserve Pos, so PC line tables change somewhat.
Change-Id: I2b5c293bee7c69c2ccd704677f5aba4ec40e3155
Reviewed-on: https://go-review.googlesource.com/103860
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
No functional changes, just changing all the orderfoo functions
into (*Order).foo methods.
Passes toolstash-check.
Change-Id: Ib9833daa98aff3c645ce56794a414f8472689152
Reviewed-on: https://go-review.googlesource.com/98617
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Eliminates lots of ad hoc code for recognizing the same thing in
different ways.
Passes toolstash-check.
Change-Id: Ic0bb005308e96331b4ef30f455b860e476725b61
Reviewed-on: https://go-review.googlesource.com/73190
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Use them to replace if/else chains with at least three comparisons,
where the code becomes clearly simpler.
Passes toolstash -cmp on std cmd.
Change-Id: Ic98aa3905944ddcab5aef5f9d9ba376853263d94
Reviewed-on: https://go-review.googlesource.com/70934
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reduce the scope of some. Also remove vars that were simply the index or
the value in a range statement. While at it, remove a var that was
exactly the length of a slice.
Also replaced 'bad' with a more clear 'errored' of type bool, and
renamed a single-char name with a comment to a name that is
self-explanatory.
And removed a few unnecessary Index calls within loops.
Passes toolstash -cmp on std cmd.
Change-Id: I26eee5f04e8f7e5418e43e25dca34f89cca5c80a
Reviewed-on: https://go-review.googlesource.com/70930
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Remove an unused type, a few redundant returns and replace a few slice
append loops with a single append.
Change-Id: If07248180bae5631b5b152c6051d9635889997d5
Reviewed-on: https://go-review.googlesource.com/66851
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
When static maps are large, we try to initialize them
by iterating over an array of key/value pairs.
Currently this optimization only works if the keys and values
are of primitive type. This CL improves this optimization
by allowing any static composite literals as well.
Fixes#22010
Change-Id: Ie493e02ab8b8a228a3472b5c6025a33f7b92daf3
Reviewed-on: https://go-review.googlesource.com/66050
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
While some map literals were marked non-escaping that information
was lost when creating the corresponding OMAKE node which made map
literals always heap allocated.
Copying the escape information to the corresponding OMAKE node allows
stack allocation of hmap and a map bucket for non escaping map literals.
Fixes#21830
Change-Id: Ife0b020fffbc513f1ac009352f2ecb110d6889c9
Reviewed-on: https://go-review.googlesource.com/62790
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Prioritized the chunks of code with 8 or more levels of indentation.
Basically early breaks/returns and joining nested ifs.
Change-Id: I6817df1303226acf2eb904a29f2db720e4f7427a
Reviewed-on: https://go-review.googlesource.com/55630
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
We generate code that calls each user init function one at a time.
When there are lots of user init functions,
usually due to generated code, like test/rotate* or
github.com/juju/govmomi/vim25/types,
we can end up with a giant function,
which can be slow to compile.
This CL puts in an escape valve.
When there are more than 500 functions, instead of doing:
init.0()
init.1()
// ...
we construct a static array of functions:
var fns = [...]func(){init.0, init.1, ... }
and call them in a loop.
This generates marginally bigger, marginally worse code,
so we restrict it to cases in which it might start to matter.
500 was selected as a mostly arbitrary threshold for "lots".
Each call uses two Progs, one for PCDATA and one for the call,
so at 500 calls we use ~1000 Progs.
At concurrency==8, we get a Prog cache of about
1000 Progs per worker.
So a threshold of 500 should more or less avoid
exhausting the Prog cache in most cases.
Change-Id: I276b887173ddbf65b2164ec9f9b5eb04d8c753c2
Reviewed-on: https://go-review.googlesource.com/41500
Reviewed-by: Keith Randall <khr@golang.org>
Follow-up to review comments on CL 41797.
Mask the input to set2 and set3, so that at the very least,
we won't corrupt the rest of the flags in case of a bad input.
It also seems more semantically appropriate.
Do minor cleanup in addrescapes. I started on larger cleanup,
but it wasn't clear that it was an improvement.
Add warning comments and sanity checks to Initorder and Class constants,
to attempt to prevent them from overflowing their allotted flag bits.
Passes toolstash-check.
Change-Id: I57b9661ba36f56406aa7a1d8da9b7c70338f9119
Reviewed-on: https://go-review.googlesource.com/41817
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
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>
LE Manh Cuong
Keith Randall
Josh Bleecher Snyder
Matthew Dempsky
Alessandro Arzilli
Keith Randall
Robert Griesemer
Keith Randall
Daniel Martí
Martin Möhrmann
Aliaksandr Valialkin
Hajime Hoshi
Cherry Zhang
Russ Cox
shawnps
David Chase
Austin Clements
Alberto Donizetti
Dave Cheney