Commit graph

188 commits

Author SHA1 Message Date
Than McIntosh
0434d40934 [dev.link] cmd/compile: mark stmp and stkobj symbols as static
Mark compiler-generated ".stmp_%d" and "<fn>.stkobj" symbols as
AttrStatic, so as to tell the linker that they do not need to be
inserted into its name lookup tables.

Change-Id: I59ffd11659b2c54c2d0ad41275d05c3f919e3b88
Reviewed-on: https://go-review.googlesource.com/c/go/+/240497
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2020-07-08 16:04:06 +00:00
Cherry Zhang
95848fc5c6 [dev.link] cmd/compile, cmd/link: remove dead methods if type is not used in interface
Currently, a method of a reachable type is live if it matches a
method of a reachable interface. In fact, we only need to retain
the method if the type is actually converted to an interface. If
the type is never converted to an interface, there is no way to
call the method through an interface method call (but the type
descriptor could still be used, e.g. in calling
runtime.newobject).

A type can be used in an interface in two ways:
- directly converted to interface. (Any interface counts, as it
  is possible to convert one interface to another.)
- obtained by reflection from a related type (e.g. obtaining an
  interface of T from []T).

For the former, we let the compiler emit a marker on the type
descriptor symbol when it is converted to an interface. In the
linker, we only need to check methods of marked types.

For the latter, when the linker visits a marked type, it needs to
visit all its "child" types as marked (i.e. potentially could be
converted to interface).

This reduces binary size:
cmd/compile	18792016	18706096 (-0.5%)
cmd/go		14120572	13398948 (-5.1%)

Change-Id: I4465c7eeabf575f4dc84017214c610fa05ae31fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/237298
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-06-11 22:32:49 +00:00
Cuong Manh Le
0f47c12a29 cmd/compile: do not emit code for discardable blank fields
Fixes #38690

Change-Id: I3544daf617fddc0f89636265c113001178d16b0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/230121
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-05-06 04:34:54 +00:00
Josh Bleecher Snyder
12d1c9b863 cmd/compile: delete gdata
All callers to gdata knew the kind of node they were working with,
so all calls to gdata have been replaced with more specific calls.

Some OADDR nodes were constructed solely for the purpose of
passing them to gdata for unwrapping. In those cases, we can now
cut to the chase.

Passes toolstash-check.

Change-Id: Iacc1abefd7f748cb269661a03768d3367319b0b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/228888
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-04-20 15:37:49 +00:00
Josh Bleecher Snyder
ed5233166f cmd/compile: simplify slicebytes
Use slicesym to implement. Remove len param.

Passes toolstash-check.

Change-Id: Ia6d4fb2a3b476eceeba60979b4dd82b634b43939
Reviewed-on: https://go-review.googlesource.com/c/go/+/228887
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-04-20 15:35:23 +00:00
Cuong Manh Le
54cbb6b0c2 cmd/compile: mark static arrays when initialize map literal as noalg
Same thing as CL 228222 does with static slice.

file      before    after     Δ       %
go        15228932  15228756  -176    -0.001%
addr2line 4429680   4429616   -64     -0.001%
api       5999032   5994904   -4128   -0.069%
asm       5087928   5087864   -64     -0.001%
compile   19727984  19723792  -4192   -0.021%
cover     5290296   5290184   -112    -0.002%
dist      3711816   3711784   -32     -0.001%
doc       4711208   4711176   -32     -0.001%
nm        4379344   4379264   -80     -0.002%
objdump   4773248   4773168   -80     -0.002%
pprof     14856148  14855764  -384    -0.003%
trace     11718212  11718020  -192    -0.002%
vet       8305944   8301768   -4176   -0.050%
total     131377612 131363900 -13712  -0.010%

Change-Id: I5ec00580b1509486c13aca43ad8f5cc7c450b62e
Reviewed-on: https://go-review.googlesource.com/c/go/+/227812
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-14 16:39:49 +00:00
Josh Bleecher Snyder
2222e0601a cmd/compile: mark static slice backing arrays as noalg
file      before    after     Δ       %       
addr2line 4413296   4404160   -9136   -0.207% 
api       5982648   5978232   -4416   -0.074% 
asm       5075640   5057656   -17984  -0.354% 
buildid   2886200   2881304   -4896   -0.170% 
cgo       4854168   4844936   -9232   -0.190% 
compile   19694784  19680752  -14032  -0.071% 
cover     5278008   5269256   -8752   -0.166% 
dist      3699528   3690984   -8544   -0.231% 
doc       4694824   4690408   -4416   -0.094% 
fix       3411336   3411048   -288    -0.008% 
link      6721496   6703320   -18176  -0.270% 
nm        4371152   4357904   -13248  -0.303% 
objdump   4760960   4747680   -13280  -0.279% 
pack      2340824   2336520   -4304   -0.184% 
pprof     14810820  14801188  -9632   -0.065% 
test2json 2861896   2857528   -4368   -0.153% 
trace     11681076  11676228  -4848   -0.042% 
vet       8285464   8276184   -9280   -0.112% 
total     115824120 115665288 -158832 -0.137% 

Change-Id: I66e1985c3a81cd9b2aa72cb4b4a8aa1781e473b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/228222
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
2020-04-14 16:09:47 +00:00
Josh Bleecher Snyder
0a18cbc2e6 cmd/compile: remove gdata layer in slicesym
The previous change moved code around to create slicesym.
This change simplifies slicesym and its callsites
by accepting an int64 for lencap instead of a node,
and by removing all the calls to gdata.
It also stops modifying n,
which avoids the need to make a copy of it.

Passes toolstash-check.

Change-Id: I4d25454d11b4bb8941000244443e3c99eef4bdd0
Reviewed-on: https://go-review.googlesource.com/c/go/+/227550
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-04-09 01:17:53 +00:00
Josh Bleecher Snyder
7096b1700d cmd/compile: refactor static slice symbol creation
This change mostly moves code around to unify it.
A subsequent change will simplify and improve slicesym.

Passes toolstash-check.

Change-Id: I84a877ea747febb2b571d4089ba6d905b51b27ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/227549
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-04-09 01:17:25 +00:00
Josh Bleecher Snyder
376472ddb7 cmd/compile: clean up slice and string offsets/sizes
Minor cleanup:

* Modernize comments.
* Change from int to int64 to avoid conversions.
* Use idiomatic names.

Passes toolstash-check.

Change-Id: I93560c81926c0f4e00f33129cb4846b53bea99e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/227548
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2020-04-09 01:16:43 +00:00
Josh Bleecher Snyder
eb5cd0fb40 cmd/compile: mark Lsyms as readonly earlier
The SSA backend has rules to read the contents of readonly Lsyms.
However, this rule was failing to trigger for many readonly Lsyms.
This is because the readonly attribute that was set on the Node.Name
was not propagated to its Lsym until the dump globals phase, after SSA runs.

To work around this phase ordering problem, introduce Node.SetReadonly,
which sets Node.Name.Readonly and also configures the Lsym
enough that SSA can use it.

This change also fixes a latent problem in the rewrite rule function,
namely that reads past the end of lsym.P were treated as entirely zero,
instead of merely requiring padding with trailing zeros.

This change also adds an amd64 rule needed to fully optimize
the results of this change. It would be better not to need this,
but the zero extension that should handle this for us
gets optimized away too soon (see #36897 for a similar problem).
I have not investigated whether other platforms also need new
rules to take full advantage of the new optimizations.

Compiled code for (interface{})(true) on amd64 goes from:

LEAQ	type.bool(SB), AX
MOVBLZX	""..stmp_0(SB), BX
LEAQ	runtime.staticbytes(SB), CX
ADDQ	CX, BX

to

LEAQ	type.bool(SB), AX
LEAQ	runtime.staticbytes+1(SB), BX

Prior to this change, the readonly symbol rewrite rules
fired a total of 884 times during make.bash.
Afterwards they fire 1807 times.

file    before    after     Δ       %
cgo     4827832   4823736   -4096   -0.085%
compile 24907768  24895656  -12112  -0.049%
fix     3376952   3368760   -8192   -0.243%
pprof   14751700  14747604  -4096   -0.028%
total   120343528 120315032 -28496  -0.024%

Change-Id: I59ea52138276c37840f69e30fb109fd376d579ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/220499
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2020-02-26 19:30:21 +00:00
DQNEO
f07059d949 cmd/compile: rename sizeof_Array and array_* to slice_*
Renames variables sizeof_Array and other array_* variables
that were actually intended for slices and not arrays.

Change-Id: I391b95880cc77cabb8472efe694b7dd19545f31a
Reviewed-on: https://go-review.googlesource.com/c/go/+/180919
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-11-11 12:40:04 +00:00
Matthew Dempsky
46be01f4e0 cmd/compile: remove Addable flag
This flag is supposed to indicate whether the expression is
"addressable"; but in practice, we infer this from other
attributes about the expression (e.g., n.Op and n.Class()).

Passes toolstash-check.

Change-Id: I19352ca07ab5646e232d98e8a7c1c9aec822ddd0
Reviewed-on: https://go-review.googlesource.com/c/go/+/200897
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-10-13 01:48:30 +00:00
Cuong Manh Le
77f5adba55 cmd/compile: don't use statictmps for small object in slice literal
Fixes #21561

Change-Id: I89c59752060dd9570d17d73acbbaceaefce5d8ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/197560
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-08 06:09:26 +00:00
Matthew Dempsky
c33d45a898 cmd/compile: don't statically copy string-typed variables
During package initialization, the compiler tries to optimize:

    var A = "foo"
    var B = A

into

    var A = "foo"
    var B = "foo"

so that we can statically initialize both A and B and skip emitting
dynamic initialization code to assign "B = A".

However, this isn't safe in the presence of cmd/link's -X flag, which
might overwrite an initialized string-typed variable at link time. In
particular, if cmd/link changes A's static initialization, it won't
know it also needs to change B's static initialization.

To address this, this CL disables this optimization for string-typed
variables.

Fixes #34675.

Change-Id: I1c18f3b855f6d7114aeb39f96aaaf1b452b88236
Reviewed-on: https://go-review.googlesource.com/c/go/+/198657
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-03 18:08:32 +00:00
Cuong Manh Le
75da700d0a cmd/compile: consistently use strlit to access constants string values
Passes toolstash-check.

Change-Id: Ieaef20b7649787727b69469f93ffc942022bc079
Reviewed-on: https://go-review.googlesource.com/c/go/+/195198
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2019-09-16 11:41:20 +00:00
Ainar Garipov
0efbd10157 all: fix typos
Use the following (suboptimal) script to obtain a list of possible
typos:

  #!/usr/bin/env sh

  set -x

  git ls-files |\
    grep -e '\.\(c\|cc\|go\)$' |\
    xargs -n 1\
    awk\
    '/\/\// { gsub(/.*\/\//, ""); print; } /\/\*/, /\*\// { gsub(/.*\/\*/, ""); gsub(/\*\/.*/, ""); }' |\
    hunspell -d en_US -l |\
    grep '^[[:upper:]]\{0,1\}[[:lower:]]\{1,\}$' |\
    grep -v -e '^.\{1,4\}$' -e '^.\{16,\}$' |\
    sort -f |\
    uniq -c |\
    awk '$1 == 1 { print $2; }'

Then, go through the results manually and fix the most obvious typos in
the non-vendored code.

Change-Id: I3cb5830a176850e1a0584b8a40b47bde7b260eae
Reviewed-on: https://go-review.googlesource.com/c/go/+/193848
Reviewed-by: Robert Griesemer <gri@golang.org>
2019-09-08 17:28:20 +00:00
Matthew Dempsky
5d0d87ae16 cmd/compile: fix package initialization ordering
This CL rewrites cmd/compile's package-level initialization ordering
algorithm to be compliant with the Go spec. See documentation in
initorder.go for details.

Incidentally, this CL also improves fidelity of initialization loop
diagnostics by including referenced functions in the emitted output
like go/types does.

Fixes #22326.

Change-Id: I7c9ac47ff563df4d4f700cf6195387a0f372cc7b
Reviewed-on: https://go-review.googlesource.com/c/go/+/170062
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2019-05-29 20:29:04 +00:00
Keith Randall
a9e107c85c cmd/compile: make sure to initialize static entries of slices
If a slice's entries are sparse, we decide to initialize it dynamically
instead of statically. That's CL 151319.

But if we do initialize it dynamically, we still need to initialize
the static entries. Typically we do that, but the bug fixed here is
that we don't if the entry's value is itself an array or struct.

To fix, use initKindLocalCode to ensure that both static and
dynamic entries are initialized via code.

Fixes #31987

Change-Id: I1192ffdbfb5cd50445c1206c4a3d8253295201dd
Reviewed-on: https://go-review.googlesource.com/c/go/+/176904
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2019-05-14 16:36:38 +00:00
LE Manh Cuong
004fb5cb8d cmd/compile: fix isStaticCompositeLiteral reports wrong for struct field
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>
2019-05-03 03:49:11 +00:00
Keith Randall
85387aa364 cmd/compile: remove dynamic entry handling from sinit/maplit
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>
2019-04-30 20:26:36 +00:00
Josh Bleecher Snyder
73cb9a1cb3 all: refer to map elements as elements instead of values
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>
2019-04-30 18:18:12 +00:00
Keith Randall
ee59c06acb cmd/compile: evaluate map initializers incrementally
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>
2019-04-29 23:45:15 +00:00
Josh Bleecher Snyder
2693b42466 cmd/compile: don't initialize blank struct fields
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>
2019-04-25 01:30:56 +00:00
Matthew Dempsky
d23bf3daa9 cmd/compile: move sinit.go globals into InitSchedule
Eliminates global state from sinit.go.

Passes toolstash-check.

Updates #22326.

Change-Id: Ie3cb14bff625baa20134d1488962ab02d24f0c15
Reviewed-on: https://go-review.googlesource.com/c/go/+/169899
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-03-28 19:51:18 +00:00
Matthew Dempsky
ee2836048c cmd/compile: change sinit.go functions into methods
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>
2019-03-28 19:28:52 +00:00
Keith Randall
ca36af215f cmd/compile: better write barrier removal when initializing new objects
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>
2019-03-18 21:16:19 +00:00
Alessandro Arzilli
08831b150c cmd/compile: avoid collisions between statictmps and user vars
Avoid name collisions between autogenerated statictmp variables and
user defined global variables.

Fixes #25113

Change-Id: I023eb42a5c2bd2f5352b046d33363faed87084dc
Reviewed-on: https://go-review.googlesource.com/c/142497
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-02-26 22:59:15 +00:00
Keith Randall
4a1a783dda cmd/compile: fix static initializer
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>
2018-12-03 16:48:21 +00:00
Robert Griesemer
a37d95c74a cmd/compile: fix constant index bounds check and error message
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>
2018-11-30 23:48:00 +00:00
Keith Randall
6fff980cf1 cmd/compile: initialize sparse slice literals dynamically
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>
2018-11-26 22:50:48 +00:00
Josh Bleecher Snyder
bc43889566 cmd/compile: bulk rename
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>
2018-11-19 00:02:53 +00:00
Matthew Dempsky
2d58fbac2f cmd/compile: extract gc.eqtype as types.Identical
For symmetry with go/types.Identical.

Passes toolstash-check.

Change-Id: Id19c3956e44ed8e2d9f203d15824322cc5842d3d
Reviewed-on: https://go-review.googlesource.com/c/143180
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2018-10-18 23:44:39 +00:00
Keith Randall
240a30da1b cmd/compile: check order temp has correct type
Followon from CL 140306

Change-Id: Ic71033d2301105b15b60645d895a076107f44a2e
Reviewed-on: https://go-review.googlesource.com/c/142178
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2018-10-15 17:56:35 +00:00
Keith Randall
389e942745 cmd/compile: reuse temporaries in order pass
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>
2018-10-14 05:21:00 +00:00
Keith Randall
9a8372f8bd cmd/compile,runtime: remove ambiguously live logic
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>
2018-10-03 19:54:16 +00:00
Robert Griesemer
ce58a39fca cmd/compile/internal/gc: fix Node.copy and introduce (raw|sep)copy
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>
2018-09-20 22:52:44 +00:00
Matthew Dempsky
2083b5d673 cmd/compile/internal/types: replace Type.Val with Type.Elem
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>
2018-04-24 22:37:52 +00:00
Daniel Martí
2b2348ab14 cmd/compile/internal/gc: add some Node methods
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>
2018-04-16 00:16:55 +00:00
Daniel Martí
60fee0153f cmd/compile: remove last manual node copies
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>
2018-04-10 15:25:31 +00:00
Daniel Martí
fcfea24742 cmd/compile: early return/continue to unindent some code
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>
2018-04-05 11:12:39 +00:00
Daniel Martí
19ee2ef950 cmd/compile: introduce gc.Node.copy method
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>
2018-04-03 12:08:39 +00:00
Matthew Dempsky
0250ef910f cmd/compile: refactor constant rewriting
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>
2018-04-02 04:09:29 +00:00
Matthew Dempsky
26708439ec cmd/compile: refactor order.go into methods
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>
2018-03-05 21:25:24 +00:00
Matthew Dempsky
fcd32885df cmd/compile: refactor method expression detection
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>
2017-10-24 22:21:34 +00:00
Daniel Martí
bb45bc27b5 cmd/compile: make more use of value switches
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>
2017-10-16 19:59:24 +00:00
Daniel Martí
270a789c52 cmd/compile: simplify some declarations
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>
2017-10-15 19:24:40 +00:00
Daniel Martí
ded2c65db3 cmd/compile: simplify a few bits of the code
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>
2017-09-28 20:40:17 +00:00
Keith Randall
91cb9edd5e cmd/compile: improve static map initialization
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>
2017-09-25 18:50:41 +00:00
Martin Möhrmann
098126103e cmd/compile: preserve escape information for map literals
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>
2017-09-11 05:54:46 +00:00