Commit graph

75 commits

Author SHA1 Message Date
LE Manh Cuong
3db6d46a4e cmd/compile: add marker for skipping dowidth when tracing typecheck
The root cause of #33658 is that fmt.Printf does have side effects when
printing Type.

typefmt for TINTER will call Type.Fields to get all embedded fields and
methods. The thing is that type.Fields itself will call dowidth, which will
expand the embedded interface, make it non-embedded anymore.

To fix it, we add a marker while we are tracing, so dowidth can know and
return immediately without doing anything.

Fixes #33658

Change-Id: Id4b70ff68a3b802675deae96793fdb8f7ef1a4a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/190537
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2019-10-22 18:26:35 +00:00
Matthew Dempsky
616c39f6a6 cmd/compile: remove DDD array types
Currently we handle [...]T array literals by treating [...]T as
special "DDD array" types. However, these array literals are just
composite literal syntax, not a distinct Go type. Moreover,
representing them as Go types contributes to complexity in a number of
unrelated bits of code.

This CL changes OCOMPLIT typechecking to look for the [...]T syntax
and handle it specially, so we can remove DDD arrays.

Passes toolstash-check.

Change-Id: Ibbf701eac4caa7a321e2d10e256658fdfaa8a160
Reviewed-on: https://go-review.googlesource.com/c/go/+/197604
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2019-09-26 20:45:09 +00:00
Matthew Dempsky
7f907b9cee cmd/compile: require -lang=go1.14 for overlapping interfaces
Support for overlapping interfaces is a new (proposed) Go language
feature to be supported in Go 1.14, so it shouldn't be supported under
-lang=go1.13 or earlier.

Fixes #34329.

Change-Id: I5fea5716b7d135476980bc40b4f6e8c611b67735
Reviewed-on: https://go-review.googlesource.com/c/go/+/195678
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2019-09-16 19:43:54 +00:00
Matthew Dempsky
380ef6b759 cmd/compile: simplify {defer,resume}checkwidth logic
This CL extends {defer,resume}checkwidth to support nesting, which
simplifies usage.

Updates #33658.

Change-Id: Ib3ffb8a7cabfae2cbeba74e21748c228436f4726
Reviewed-on: https://go-review.googlesource.com/c/go/+/192721
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
2019-09-03 17:38:32 +00:00
LE Manh Cuong
e87fe0f1f5 cmd/compile: make typecheck set n.Type.Nod when returning OTYPE
typecheck only set n.Type.Nod for declared type, and leave it nil for
anonymous types, type alias. It leads to compiler crashes, because
n.Type.Nod is nil at the time dowidth was called.

Fixing it by set n.Type.Nod right after n.Type initialization if n.Op is
OTYPE.

When embedding interface cycles involve in type alias, it also helps
pointing the error message to the position of the type alias
declaration, instead of position of embedding interface.

Fixes #31872

Change-Id: Ia18391e987036a91f42ba0c08b5506f52d07f683
Reviewed-on: https://go-review.googlesource.com/c/go/+/191540
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-08-29 16:43:07 +00:00
Matthew Dempsky
8d4b685ab5 cmd/compile: allow embedding overlapping interfaces
Quietly drop duplicate methods inherited from embedded interfaces if
they have an identical signature to existing methods.

Updates #6977.

Change-Id: I144151cb7d99695f12b555c0db56207993c56284
Reviewed-on: https://go-review.googlesource.com/c/go/+/187519
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2019-08-26 20:21:21 +00:00
Matthew Dempsky
97edf77903 cmd/compile: refactor expandiface
Move checkdupfields call into expandiface, and inline/simplify offmod.
More prep work for implementing #6977.

Change-Id: I958ae87f761ec25a8fa7298a2a3019eeca5b25ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/187518
Reviewed-by: Robert Griesemer <gri@golang.org>
2019-08-26 19:39:05 +00:00
Matthew Dempsky
63661b7251 cmd/compile: refactor checkdupfields API
Allows avoiding the Type.Fields call, which affects prevents
checkdupfields from being called at the more natural point during
dowidth.

Change-Id: I724789c860e7fffba1e8e876e2d74dcfba85d75c
Reviewed-on: https://go-review.googlesource.com/c/go/+/187517
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-08-26 19:38:51 +00:00
Robert Griesemer
dc0388c565 cmd/compile: avoid compiler crash for recursive interface type
This change is a simple work-around to avoid a compiler crash
and provide a reasonable error message. A future change should
fix the root cause for this problem.

Fixes #23823.

Change-Id: Ifc80d9f4d35e063c378e54d5cd8d1cf4c0d2ec6a
Reviewed-on: https://go-review.googlesource.com/c/go/+/175518
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-05-07 18:42:17 +00:00
Robert Griesemer
fde4b9ed14 cmd/compile: better documentation around checkwidth
Change-Id: I5c7ec9676b5573c883c196459acea85aa9ff8130
Reviewed-on: https://go-review.googlesource.com/c/146021
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-10-31 04:30:10 +00:00
Matthew Dempsky
62e5215a2a cmd/compile: merge TPTR32 and TPTR64 as TPTR
Change-Id: I0490098a7235458c5aede1135426a9f19f8584a7
Reviewed-on: https://go-review.googlesource.com/c/76312
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2018-10-04 04:08:08 +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
Matthew Dempsky
7759b32a62 cmd/compile: replace Field.Nname.Pos with Field.Pos
For struct fields and methods, Field.Nname was only used to store
position information, which means we're allocating an entire ONAME
Node+Name+Param structure just for one field. We can optimize away
these ONAME allocations by instead adding a Field.Pos field.

Unfortunately, we can't get rid of Field.Nname, because it's needed
for function parameters, so Field grows a little bit and now has more
redundant information in those cases. However, that was already the
case (e.g., Field.Sym and Field.Nname.Sym), and it's still a net win
for allocations as demonstrated by the benchmarks below.

Additionally, by moving the ONAME allocation for function parameters
to funcargs, we can avoid allocating them for function parameters that
aren't used in corresponding function bodies (e.g., interface methods,
function-typed variables, and imported functions/methods without
inline bodies).

name       old time/op       new time/op       delta
Template         254ms ± 6%        251ms ± 6%  -1.04%  (p=0.000 n=487+488)
Unicode          128ms ± 7%        128ms ± 7%    ~     (p=0.294 n=482+467)
GoTypes          862ms ± 5%        860ms ± 4%    ~     (p=0.075 n=488+471)
Compiler         3.91s ± 4%        3.90s ± 4%  -0.39%  (p=0.000 n=468+473)

name       old user-time/op  new user-time/op  delta
Template         339ms ±14%        336ms ±14%  -1.02%  (p=0.001 n=498+494)
Unicode          176ms ±18%        176ms ±25%    ~     (p=0.940 n=491+499)
GoTypes          1.13s ± 8%        1.13s ± 9%    ~     (p=0.157 n=496+493)
Compiler         5.24s ± 6%        5.21s ± 6%  -0.57%  (p=0.000 n=485+489)

name       old alloc/op      new alloc/op      delta
Template        38.3MB ± 0%       37.3MB ± 0%  -2.58%  (p=0.000 n=499+497)
Unicode         29.1MB ± 0%       29.1MB ± 0%  -0.03%  (p=0.000 n=500+493)
GoTypes          116MB ± 0%        115MB ± 0%  -0.65%  (p=0.000 n=498+499)
Compiler         492MB ± 0%        487MB ± 0%  -1.00%  (p=0.000 n=497+498)

name       old allocs/op     new allocs/op     delta
Template          364k ± 0%         360k ± 0%  -1.15%  (p=0.000 n=499+499)
Unicode           336k ± 0%         336k ± 0%  -0.01%  (p=0.000 n=500+493)
GoTypes          1.16M ± 0%        1.16M ± 0%  -0.30%  (p=0.000 n=499+499)
Compiler         4.54M ± 0%        4.51M ± 0%  -0.58%  (p=0.000 n=494+495)

Passes toolstash-check -gcflags=-dwarf=false. Changes DWARF output
because position information is now tracked more precisely for
function parameters.

Change-Id: Ib8077d70d564cc448c5e4290baceab3a4396d712
Reviewed-on: https://go-review.googlesource.com/108217
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2018-04-23 18:05:57 +00:00
griesemer
25159d3af9 cmd/compile: avoid spurious errors for invalid map key types
Instead of trying to validate map key types eagerly in some
cases, delay their validation to the end of type-checking,
when we all type information is present.

Passes go build -toolexec 'toolstash -cmp' -a std .

Fixes #21273.
Fixes #21657.

Change-Id: I532369dc91c6adca1502d6aa456bb06b57e6c7ff
Reviewed-on: https://go-review.googlesource.com/75310
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-11-02 23:53:38 +00:00
Matthew Dempsky
bb2f0da23a cmd/compile: fix compiler crash on recursive types
By setting both a valid size and alignment for broken recursive types,
we can appease some more safety checks and prevent compiler crashes.

Fixes #21882.

Change-Id: Ibaa137d8aa2c2a9d521462f144d7016c4abfd6e7
Reviewed-on: https://go-review.googlesource.com/64430
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-09-18 21:49:43 +00:00
Josh Bleecher Snyder
92363d52c0 cmd/compile: check width of embedded interfaces in expandiface
The code in #20162 contains an embedded interface.

It didn't get dowidth'd by the frontend,
and during DWARF generation, ngotype asked
for a string description of it,
which triggered a request for the number of fields
in the interface, which triggered a dowidth,
which is disallowed in the backend.

The other changes in this CL are to support the test.

Fixes #20162

Change-Id: I4d0be5bd949c361d4cdc89a8ed28b10977e40cf9
Reviewed-on: https://go-review.googlesource.com/42131
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-04-28 20:07:38 +00:00
Josh Bleecher Snyder
85d6a29ae6 cmd/compile: prevent infinite recursion printing types in Fatalf
Updates #20162

Change-Id: Ie289bae0d0be8430e492ac73fd6e6bf36991d4a1
Reviewed-on: https://go-review.googlesource.com/42130
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-04-28 16:08:01 +00:00
Josh Bleecher Snyder
c51559813f cmd/compile: add sizeCalculationDisabled flag
Use it to ensure that dowidth is not called
from the backend on a type whose size
has not yet been calculated.

This is an alternative to CL 42016.

Change-Id: I8c7b4410ee4c2a68573102f6b9b635f4fdcf392e
Reviewed-on: https://go-review.googlesource.com/42018
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-04-28 01:24:52 +00:00
Robert Griesemer
e8475c94b8 cmd/compile/internal/types: shorten struct type names
They are in the types package, no need to mention the Type suffix.

Change-Id: Ie4fe1e3c1793514145e33f9df373d715f63e1aad
Reviewed-on: https://go-review.googlesource.com/39911
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-04-07 03:44:05 +00:00
Robert Griesemer
f68f292820 cmd/compile: factor out Pkg, Sym, and Type into package types
- 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>
2017-04-07 03:04:00 +00:00
Josh Bleecher Snyder
8dafdb1be1 cmd/compile: add Type.WidthCalculated
Prior to this CL, Type.Width != 0 was the mark
of a Type whose Width had been calculated.
As a result, dowidth always recalculated
the width of struct{}.
This, combined with the prohibition on calculating
the width of a FuncArgsStruct and the use of
struct{} as a function argument,
meant that there were circumstances in which
it was forbidden to call dowidth on a type.
This inhibits refactoring to call dowidth automatically,
rather than explicitly.
Instead add a helper method, Type.WidthCalculated,
and implement as Type.Align > 0.
Type.Width is not a good candidate for tracking
whether the width has been calculated;
0 is a value type width, and Width is subject to
too much magic value game-playing.

For good measure, add a test for #11354.

Change-Id: Ie9a9fb5d924e7a2010c1904ae5e38ed4a38eaeb2
Reviewed-on: https://go-review.googlesource.com/38468
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-03-28 18:06:09 +00:00
Josh Bleecher Snyder
7ac2e413eb cmd/compile: minor cleanup in widstruct
Change-Id: I9e52a2c52b754568412d719b415f91a998d247fe
Reviewed-on: https://go-review.googlesource.com/38467
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-03-23 19:15:51 +00:00
Matthew Dempsky
07de3465be cmd/compile/internal/gc: handle recursive interfaces better
Previously, we handled recursive interfaces by deferring typechecking
of interface methods, while eagerly expanding interface embeddings.

This CL switches to eagerly evaluating interface methods, and
deferring expanding interface embeddings to dowidth. This allows us to
detect recursive interface embeddings with the same mechanism used for
detecting recursive struct embeddings.

Updates #16369.

Change-Id: If4c0320058047f8a2d9b52b9a79de47eb9887f95
Reviewed-on: https://go-review.googlesource.com/38391
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-03-21 01:56:25 +00:00
Matthew Dempsky
07af21308c cmd/compile/internal/gc: eliminate two uses of Type.Pos
Instead we can use t.nod.Pos.

Change-Id: I643ee3226e402e38d4c77e8f328cbe83e55eac5c
Reviewed-on: https://go-review.googlesource.com/38309
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-03-20 20:24:35 +00:00
Matthew Dempsky
09272ae981 cmd/compile/internal/gc: rename Thearch to thearch
Prepared using gorename.

Change-Id: Id55dac9ae5446a8bfeac06e7995b35f4c249eeca
Reviewed-on: https://go-review.googlesource.com/38302
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2017-03-17 22:10:58 +00:00
Aliaksandr Valialkin
ed70f37e73 cmd/compile: pack bool fields in Node, Name, Func and Type structs to bitsets
This reduces compiler memory usage by up to 4% - see compilebench
results below.

name       old time/op     new time/op     delta
Template       245ms ± 4%      241ms ± 2%  -1.88%  (p=0.029 n=10+10)
Unicode        126ms ± 3%      124ms ± 3%    ~     (p=0.105 n=10+10)
GoTypes        805ms ± 2%      813ms ± 3%    ~     (p=0.515 n=8+10)
Compiler       3.95s ± 2%      3.83s ± 1%  -2.96%  (p=0.000 n=9+10)
MakeBash       47.4s ± 4%      46.6s ± 1%  -1.59%  (p=0.028 n=9+10)

name       old user-ns/op  new user-ns/op  delta
Template        324M ± 5%       326M ± 3%    ~     (p=0.935 n=10+10)
Unicode         186M ± 5%       178M ±10%    ~     (p=0.067 n=9+10)
GoTypes        1.08G ± 7%      1.09G ± 4%    ~     (p=0.956 n=10+10)
Compiler       5.34G ± 4%      5.31G ± 1%    ~     (p=0.501 n=10+8)

name       old alloc/op    new alloc/op    delta
Template      41.0MB ± 0%     39.8MB ± 0%  -3.03%  (p=0.000 n=10+10)
Unicode       32.3MB ± 0%     31.0MB ± 0%  -4.13%  (p=0.000 n=10+10)
GoTypes        119MB ± 0%      116MB ± 0%  -2.39%  (p=0.000 n=10+10)
Compiler       499MB ± 0%      487MB ± 0%  -2.48%  (p=0.000 n=10+10)

name       old allocs/op   new allocs/op   delta
Template        380k ± 1%       379k ± 1%    ~     (p=0.436 n=10+10)
Unicode         324k ± 1%       324k ± 0%    ~     (p=0.853 n=10+10)
GoTypes        1.15M ± 0%      1.15M ± 0%    ~     (p=0.481 n=10+10)
Compiler       4.41M ± 0%      4.41M ± 0%  -0.12%  (p=0.007 n=10+10)

name       old text-bytes  new text-bytes  delta
HelloSize       623k ± 0%       623k ± 0%    ~     (all equal)
CmdGoSize      6.64M ± 0%      6.64M ± 0%    ~     (all equal)

name       old data-bytes  new data-bytes  delta
HelloSize      5.81k ± 0%      5.81k ± 0%    ~     (all equal)
CmdGoSize       238k ± 0%       238k ± 0%    ~     (all equal)

name       old bss-bytes   new bss-bytes   delta
HelloSize       134k ± 0%       134k ± 0%    ~     (all equal)
CmdGoSize       152k ± 0%       152k ± 0%    ~     (all equal)

name       old exe-bytes   new exe-bytes   delta
HelloSize       967k ± 0%       967k ± 0%    ~     (all equal)
CmdGoSize      10.2M ± 0%      10.2M ± 0%    ~     (all equal)

Change-Id: I1f40af738254892bd6c8ba2eb43390b175753d52
Reviewed-on: https://go-review.googlesource.com/37445
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-03-03 21:06:03 +00:00
Russ Cox
47ce87877b all: merge dev.inline into master
Change-Id: I7715581a04e513dcda9918e853fa6b1ddc703770
2017-02-01 09:47:23 -05:00
Russ Cox
9bbb07ddec [dev.typealias] cmd/compile, reflect: fix struct field names for embedded byte, rune
Will also fix type aliases.

Fixes #17766.
For #18130.

Change-Id: I9e1584d47128782152e06abd0a30ef423d5c30d2
Reviewed-on: https://go-review.googlesource.com/35732
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-01-25 18:57:20 +00:00
Robert Griesemer
eab3707d6d [dev.inline] cmd/compile: rename various fields from Lineno to Pos
Various minor adjustments.

Change-Id: Iedfb97989f7bedaa3e9e8993b167e05f162434a7
Reviewed-on: https://go-review.googlesource.com/34136
Reviewed-by: David Lazar <lazard@golang.org>
2016-12-08 21:35:18 +00:00
Matthew Dempsky
832082b44e cmd/compile: remove -A flag
mkbuiltin.go now generates builtin.go using go/ast instead of running
the compiler, so we don't need the -A flag anymore.

Passes toolstash -cmp.

Change-Id: Ifa70f4f3c9feae10c723cbec81a0a47c39610090
Reviewed-on: https://go-review.googlesource.com/31497
Reviewed-by: Robert Griesemer <gri@golang.org>
2016-10-19 20:22:13 +00:00
Matthew Dempsky
f54c0db859 cmd/compile, cmd/cgo: align complex{64,128} like GCC
complex64 and complex128 are treated like [2]float32 and [2]float64,
so it makes sense to align them the same way.

Change-Id: Ic614bcdcc91b080aeb1ad1fed6fc15ba5a2971f8
Reviewed-on: https://go-review.googlesource.com/19800
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2016-10-05 17:44:27 +00:00
Dave Cheney
584978f4b5 cmd/compile/internal/gc: unexport private variables
Change-Id: I14a7c08105e6bdcee04a5cc21d7932e9ca753384
Reviewed-on: https://go-review.googlesource.com/29138
Run-TryBot: Dave Cheney <dave@cheney.net>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-09-15 14:59:35 +00:00
Dave Cheney
d7012ca282 cmd/compile/internal/gc: unexport more helper functions
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>
2016-09-15 13:57:42 +00:00
Dave Cheney
82703f84e4 cmd/compile/internal/gc: unexport helper functions
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>
2016-09-15 05:43:41 +00:00
Robert Griesemer
6537e18f02 cmd/compile: rewrite %1v and %2v formats to %S and %L (short and long)
- also consistently use %v instead of %s when we have a (gc) Formatter
- rewrite done automatically using Formats test in -u (update) mode
- manual update of format strings that were not single string constants
- updated fmt.go, fmt_test.go accordingly
- fmt_test: permit "%T" always

Change-Id: I8f0704286aba5704600ad0c4a4484005b79b905d
Reviewed-on: https://go-review.googlesource.com/28954
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2016-09-12 20:07:25 +00:00
Robert Griesemer
8d0bbe2b48 cmd/compile: implement fmt.Formatter for *Type formats %s, %v
Change-Id: I878ac549430abc7859c30d176d52d52ce02c5827
Reviewed-on: https://go-review.googlesource.com/28333
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2016-09-08 21:34:41 +00:00
Russ Cox
b6dc3e6f66 cmd/compile: fix liveness computation for heap-escaped parameters
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>
2016-05-27 03:19:52 +00:00
Matthew Dempsky
40f1d0ca9f cmd/compile: split TSLICE into separate Type kind
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>
2016-04-21 21:03:22 +00:00
Josh Bleecher Snyder
f38f43d029 cmd/compile: shrink gc.Type in half
Many of Type's fields are etype-specific.
This CL organizes them into their own auxiliary types,
duplicating a few fields as necessary,
and adds an Extra field to hold them.
It also sorts the remaining fields for better struct packing.
It also improves documentation for most fields.

This reduces the size of Type at the cost of some extra allocations.
There's no CPU impact; memory impact below.
It also makes the natural structure of Type clearer.

Passes toolstash -cmp on all architectures.

Ideas for future work in this vein:

(1) Width and Align probably only need to be
stored for Struct and Array types.
The refactoring to accomplish this would hopefully
also eliminate TFUNCARGS and TCHANARGS entirely.

(2) Maplineno is sparsely used and could probably better be
stored in a separate map[*Type]int32, with mapqueue updated
to store both a Node and a line number.

(3) The Printed field may be removable once the old (non-binary)
importer/exported has been removed.

(4) StructType's fields field could be changed from *[]*Field to []*Field,
which would remove a common allocation.

(5) I believe that Type.Nod can be moved to ForwardType. Separate CL.

name       old alloc/op     new alloc/op     delta
Template       57.9MB ± 0%      55.9MB ± 0%  -3.43%        (p=0.000 n=50+50)
Unicode        38.3MB ± 0%      37.8MB ± 0%  -1.39%        (p=0.000 n=50+50)
GoTypes         185MB ± 0%       180MB ± 0%  -2.56%        (p=0.000 n=50+50)
Compiler        824MB ± 0%       806MB ± 0%  -2.19%        (p=0.000 n=50+50)

name       old allocs/op    new allocs/op    delta
Template         486k ± 0%        497k ± 0%  +2.25%        (p=0.000 n=50+50)
Unicode          377k ± 0%        379k ± 0%  +0.55%        (p=0.000 n=50+50)
GoTypes         1.39M ± 0%       1.42M ± 0%  +1.63%        (p=0.000 n=50+50)
Compiler        5.52M ± 0%       5.57M ± 0%  +0.84%        (p=0.000 n=47+50)

Change-Id: I828488eeb74902b013d5ae4cf844de0b6c0dfc87
Reviewed-on: https://go-review.googlesource.com/21611
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-04-06 19:10:10 +00:00
Josh Bleecher Snyder
fda831ed3f cmd/compile: encapsulate reads of gc.Type.Funarg
Changes generated with eg and then manually
checked and in some cases simplified.

Passes toolstash -cmp.

Change-Id: I2119f37f003368ce1884d2863b406d6ffbfe38c7
Reviewed-on: https://go-review.googlesource.com/21563
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2016-04-06 02:49:18 +00:00
Josh Bleecher Snyder
3a0783c504 cmd/compile: use NumElem instead of Type.Bound
This eliminates all direct reads of Type.Bound
outside type.go.

Change-Id: I0a9a72539f8f4c0de7f5e05e1821936bf7db5eb7
Reviewed-on: https://go-review.googlesource.com/21421
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-04-01 20:03:19 +00:00
Josh Bleecher Snyder
e775b8df7a cmd/compile: add sliceBound
Add a constant for the magic -1 for slice bounds.
Use it.
Enforce more aggressively that bounds must be
slice, ddd, or non-negative.
Remove ad hoc check in plive.go.
Check bounds before constructing an array type
when typechecking.

All changes are manual.

Passes toolstash -cmp.

Change-Id: I9fd9cc789d7d4b4eea3b30b24037a254d3788add
Reviewed-on: https://go-review.googlesource.com/21348
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2016-03-31 18:41:30 +00:00
Josh Bleecher Snyder
8640b51df8 cmd/compile: add Type.Elem
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>
2016-03-30 21:21:55 +00:00
Josh Bleecher Snyder
e3c7497327 cmd/compile: add typWrapper and Type.Wrapped
Passes toolstash -cmp.

Change-Id: I7dffd9bc5bab323590df6fb591bf1e73edf2e465
Reviewed-on: https://go-review.googlesource.com/21305
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-03-30 17:19:50 +00:00
Josh Bleecher Snyder
331f962508 cmd/compile: use IsSlice and IsArray instead of checking Bound
Changes generated by eg and manually checked.

Isfixedarray, Isslice, and many other
Type-related functions in subr.go should
either be deleted or moved to type.go.
Later, though; the game now is cleanup via encapsulation.

Passes toolstash -cmp.

Change-Id: I83dd8816f6263b74367d23c2719a08c362e330f9
Reviewed-on: https://go-review.googlesource.com/21303
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-03-30 17:18:29 +00:00
Josh Bleecher Snyder
093a9a1f56 cmd/compile: encapsulate map value type
Passes toolstash -cmp.

Change-Id: I83af544974e1e91e0810e13321afb3e665dcdf12
Reviewed-on: https://go-review.googlesource.com/21248
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2016-03-29 05:26:32 +00:00
Josh Bleecher Snyder
272df158ac cmd/compile: clean up ... Bound marker
This mostly a mechanical change.
However, the change in assignop (subr.go) is a bug fix.
The code didn’t match the comment,
and the comment was correct.
Nevertheless, this CL passes toolstash -cmp.

The last direct reference to dddBound outside
type.go (in typecheck.go) will go away
in a future CL.

Change-Id: Ifb1691e0a07f906712c18c4a4cd23060807a5da5
Reviewed-on: https://go-review.googlesource.com/21235
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2016-03-28 23:33:30 +00:00
Matthew Dempsky
1b2fbb49c8 cmd/compile: cleanup alg.go for Field slices
Passes toolstash -cmp.

Change-Id: Ie41d7e74847c44a8fd174731374339c6c32b1460
Reviewed-on: https://go-review.googlesource.com/21231
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-03-28 20:13:59 +00:00
Matthew Dempsky
62dddd4770 cmd/compile: rename Field's Width field to Offset
gorename -from '"cmd/compile/internal/gc".Field.Width' -to Offset

Passes toolstash -cmp.

Change-Id: I310538a1f60bbab470a6375e813e9d5eb52c5bbf
Reviewed-on: https://go-review.googlesource.com/21230
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-03-28 20:13:51 +00:00
Matthew Dempsky
f6bca3f32d cmd/compile: eliminate a bunch of IterFields/IterMethods calls
This is an automated rewrite of all the calls of the form:

    for f, it := IterFields(t); f != nil; f = it.Next() { ... }

Followup CLs will work on cleaning up the remaining cases.

Change-Id: Ic1005ad45ae0b50c63e815e34e507e2d2644ba1a
Reviewed-on: https://go-review.googlesource.com/20794
Reviewed-by: David Crawshaw <crawshaw@golang.org>
2016-03-17 19:38:15 +00:00