Inl, Inldcl, and InlCost are only applicable to functions with bodies
that can be inlined, so pull them out into a separate Inline type to
make understanding them easier.
A side benefit is that we can check if a function can be inlined by
just checking if n.Func.Inl is non-nil, which simplifies handling of
empty function bodies.
While here, remove some unnecessary Curfn twiddling, and make imported
functions use Inl.Dcl instead of Func.Dcl for consistency for local
functions.
Passes toolstash-check.
Change-Id: Ifd4a80349d85d9e8e4484952b38ec4a63182e81f
Reviewed-on: https://go-review.googlesource.com/104756
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Previously, n.Pos was reassigned to lineno when declare was called,
which might not match where the identifier actually appeared in the
source. This caused a loss of position precision for function
parameters (which were all declared at the last parameter's position),
and required some clumsy workarounds in bimport.go.
This CL changes declare to leave n.Pos alone and also fixes a few
places where n.Pos was not being set correctly.
Change-Id: Ibe5b5fd30609c684367207df701f9a1bfa82867f
Reviewed-on: https://go-review.googlesource.com/104275
Reviewed-by: Robert Griesemer <gri@golang.org>
We used to have three Sym flags for dealing with export/reexport:
Export, Package, and Exported.
Export and Package were used to distinguish whether a symbol is
exported or package-scope (i.e., mutually exclusive), except that for
local declarations Export served double-duty as tracking whether the
symbol had been added to exportlist.
Meanwhile, imported declarations that needed reexporting could be
added to exportlist multiple times, necessitating a flag to track
whether they'd already been written out by exporter.
Simplify all of these into a single OnExportList flag so that we can
ensure symbols on exportlist are present exactly once. Merge
reexportsym into exportsym so there's a single place where we append
to exportlist.
Code that used to set Exported to prevent a symbol from being exported
can now just set OnExportList before calling declare to prevent it
from even appearing on exportlist.
Lastly, drop the IsAlias check in exportsym: we call exportsym too
early for local symbols to detect if they're an alias, and we never
reexport aliases.
Passes toolstash-check.
Change-Id: Icdea3719105dc169fcd7651606589cd08b0a80ff
Reviewed-on: https://go-review.googlesource.com/103865
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently, we reexport any package-scope constant, function, type, or
variable declarations needed by an inlineable function body. However,
now that we have an early pass to walk inlineable function bodies
(golang.org/cl/74110), we can simplify the logic for finding these
declarations.
The binary export format supports writing out type declarations
in-place at their first use. Also, it always writes out constants by
value, so their declarations never need to be reexported.
Notably, we attempted this before (golang.org/cl/36170) and had to
revert it (golang.org/cl/45911). However, this was because while
writing out inline bodies, we could discover variable/function
dependencies. By collecting variable/function dependencies during
inlineable function discovery, we avoid this problem.
While here, get rid of isInlineable. We already typecheck inlineable
function bodies during inlFlood, so it's become a no-op. Just move the
comment explaining parameter numbering to its caller.
Change-Id: Ibbfaafce793733675d3a2ad98791758583055666
Reviewed-on: https://go-review.googlesource.com/103864
Reviewed-by: Robert Griesemer <gri@golang.org>
Eliminates an inconsistency between user functions and generated
functions.
Passes toolstash-check.
Change-Id: I946b511ca81d88a0024b5932cb50f3d8b9e808f4
Reviewed-on: https://go-review.googlesource.com/103863
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Add helper methods that validate n.Op and convert to/from the
appropriate type.
Notably, there was a lot of code in walk.go that thought setting
Etype=1 on an OADDR node affected escape analysis.
Passes toolstash-check.
TBR=marvin
Change-Id: Ieae7c67225c1459c9719f9e6a748a25b975cf758
Reviewed-on: https://go-review.googlesource.com/99535
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Now possible, since stringer just got the -trimprefix flag added.
While at it, simplify a few Op stringifications since we can now use %v,
and no longer have to worry about o<len(opnames).
Passes toolstash -cmp on std cmd.
Fixes#15462.
Change-Id: Icdcde0b0a5eb165d18488918175024da274f782b
Reviewed-on: https://go-review.googlesource.com/76790
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Dave Cheney <dave@cheney.net>
All of these had a return or break in the else body, so flipping the
condition means we can unindent and simplify.
Change-Id: If93e97504480d18a0dac3f2c8ffe57ab8bcb929c
Reviewed-on: https://go-review.googlesource.com/74190
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Previously, anytime we exported a function or method declaration
(which includes methods for every type transitively exported), we
included the inline function bodies, if any. However, in many cases,
it's impossible (or at least very unlikely) for the importing package
to call the method.
For example:
package p
type T int
func (t T) M() { t.u() }
func (t T) u() {}
func (t T) v() {}
T.M and T.u are inlineable, and they're both reachable through calls
to T.M, which is exported. However, t.v is also inlineable, but cannot
be reached.
Exception: if p.T is embedded in another type q.U, p.T.v will be
promoted to q.U.v, and the generated wrapper function could have
inlined the call to p.T.v. However, in practice, this doesn't happen,
and a missed inlining opportunity doesn't affect correctness.
To implement this, this CL introduces an extra flood fill pass before
exporting to mark inline bodies that are actually reachable, so the
exporter can skip over methods like t.v.
This reduces Kubernetes build time (as measured by "time go build -a
k8s.io/kubernetes/cmd/...") on an HP Z620 measurably:
== before ==
real 0m44.658s
user 11m19.136s
sys 0m53.844s
== after ==
real 0m41.702s
user 10m29.732s
sys 0m50.908s
It also significantly cuts down the cost of enabling mid-stack
inlining (-l=4):
== before (-l=4) ==
real 1m19.236s
user 20m6.528s
sys 1m17.328s
== after (-l=4) ==
real 0m59.100s
user 13m12.808s
sys 0m58.776s
Updates #19348.
Change-Id: Iade58233ca42af823a1630517a53848b5d3c7a7e
Reviewed-on: https://go-review.googlesource.com/74110
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@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>
Currently, benchmarking compile performance under -l=4 is confounded
by -l=2 enabling eager typechecking of unused inline function bodies
for debugging. This isn't logically an "inlining aggressiveness"
level, so instead move this logic under the -d umbrella flag.
Change-Id: I713f68952efbe25b6941d3ebc2f3707ccbbd6240
Reviewed-on: https://go-review.googlesource.com/72253
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Previously, we were treating cross-package function calls as free for
inlining budgeting.
In theory, we should be able to recompute InlCost from the
exported/reimported function bodies. However, that process mutates the
structure of the Node AST enough that it doesn't preserve InlCost. To
avoid unexpected issues, just record and restore InlCost in the export
data.
Fixes#19261.
Change-Id: Iac2bc0d32d4f948b64524aca657051f9fc96d92d
Reviewed-on: https://go-review.googlesource.com/70151
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
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>
Previously, we used OXFALL vs OFALL to distinguish fallthrough
statements that had been validated. Because in the Node AST we flatten
statement blocks, OXCASE and OXFALL needed to keep track of their
block scopes for this purpose.
Now that we have an AST that keeps these separate, we can just perform
the validation earlier.
Passes toolstash-check.
Fixes#14540.
Change-Id: I8421eaba16c2b3b72c9c5483b5cf20b14261385e
Reviewed-on: https://go-review.googlesource.com/61130
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
These two special cases are unnecessary:
1) "~b%d" references only appear during walk, to handle "return"
statements implicitly assigning to blank result parameters. Even if
they could appear, the "inlined and customized version" accidentally
diverged from p.sym in golang.org/cl/33911.
2) The Vargen case is already identical to the default case, and it
never overlaps with the remaining "T.method" case.
Passes toolstash-check.
Change-Id: I03f7e5b75b707b43afc8ed6eb90f43ba93ed17ae
Reviewed-on: https://go-review.googlesource.com/63272
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
We only export packages that typechecked successfully, and OTYPE nodes
will always have their Type field set.
Changes the package export format, but only in the compiler-specific
section. No version bump necessary.
Change-Id: I722f5827e73948fceb0432bc8b3b22471fea8f61
Reviewed-on: https://go-review.googlesource.com/63273
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
There shouldn't be any problems setting error's "Orig" (underlying)
type to a separate anonymous interface, as this is already how
go/types defines it.
Change-Id: I44e9c4048ffe362ce329e8306632e38b5ccfecff
Reviewed-on: https://go-review.googlesource.com/61790
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Alan Donovan <adonovan@google.com>
Before CL 36170, we identified all function bodies that needed to be
exported before writing any export data.
With CL 36170, we started identifying additional functions while
exporting function bodies. As a consequence, we cannot use a
range-based for loop for iterating over function bodies anymore.
Fixes#18895.
Change-Id: I9cbefa8d311ca8c9898c8272b2ac365976b02396
Reviewed-on: https://go-review.googlesource.com/45817
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Per gri's suggestion on CL 41623,
add a comment to trackAllTypes
about the trade-offs of enabling it.
Change-Id: Iec42b0da7933543200729003d1b2c6e0d9dcc5f0
Reviewed-on: https://go-review.googlesource.com/42186
Reviewed-by: Brad Fitzpatrick <bradfitz@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>
Followup to previous typenod CL. Changes export data format, but only
the compiler-specific section, so no version bump.
Change-Id: I0c21737141f3d257366b29b2a9211bc7217c39ee
Reviewed-on: https://go-review.googlesource.com/39797
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The textual import/export format is ancient history.
Change-Id: Iebe90bfd9bd3074eb191186d86e5f4286ce3b1f3
Reviewed-on: https://go-review.googlesource.com/39850
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The symExport flag tells whether a symbol is in the export list
already or not (and it's also used to avoid being added to that
list). Exporting is based on that export list - no need to check
again.
Change-Id: I6056f97aa5c24a19376957da29199135c8da35f9
Reviewed-on: https://go-review.googlesource.com/39033
Reviewed-by: Dave Cheney <dave@cheney.net>
CL 37499 allows inlining more functions by ignoring dead code.
However, that dead code can contain non-exportable constructs.
Teach the exporter not to export dead code.
Fixes#19679
Change-Id: Idb1d3794053514544b6f1035d29262aa6683e1e7
Reviewed-on: https://go-review.googlesource.com/38601
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
exporter.nesting was added in c7b9bd74 to mitigate #16369 which was
closed in ee272bbf. Remove the exporter.nesting field as it is now unused.
Change-Id: I07873d1a07d6a08b11994b817a1483ffc2f5e45f
Reviewed-on: https://go-review.googlesource.com/38490
Run-TryBot: Dave Cheney <dave@cheney.net>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The position information recorded now consists of the line-
directive relative filename and line number. It would be
relatively easy to also encode absolute position information
as necessary (by serializing src.PosBase data).
For example, given $GOROOT/src/tmp/x.go:
package p
const C0 = 0
//line c.go:10
const C1 = 1
//line t.go:20
type T int
//line v.go:30
var V T
//line f.go:40
func F() {}
The recorded positions for the exported entities are:
C0 $GOROOT/src/tmp/x.go 3
C1 c.go 10
T t.go 20
V v.go 30
F f.go 40
Fix verified by manual inspection. There's currently no easy way
to test this, but it will eventually be tested when we fix#7311.
Fixes#19391.
Change-Id: I6269067ea58358250fe6dd1f73bdf9e5d2adfe3d
Reviewed-on: https://go-review.googlesource.com/37936
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This makes Sym flags consistent with the rest of the code after
the CL 37445.
No functional changes.
Passes toolstash -cmp.
Change-Id: Ica919f2ab98581371c717fff9a70aeb11058ca17
Reviewed-on: https://go-review.googlesource.com/37847
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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 binary export format embeds type definitions inline as necessary,
so there's no need to add them to exportlist. Also, constants are
embedded directly by value, so they can be omitted too.
Change-Id: Id1879eb97c298a5a52f615cf9883c346c7f7bd69
Reviewed-on: https://go-review.googlesource.com/36170
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Known issues:
- needs many more tests
- duplicate method declarations via type alias names are not detected
- type alias cycle error messages need to be improved
- need to review setup of byte/rune type aliases
For #18130.
Change-Id: Icc2fefad6214e5e56539a9dcb3fe537bf58029f8
Reviewed-on: https://go-review.googlesource.com/35121
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
cmd/compile:
- remove crud from prior alias implementation
- better comments in places
go/types:
- fix TypeName.IsAlias predicate
- more tests
go/importer (go/internal/gcimporter15):
- handle "@" format for anonymous fields using aliases
(currently tested indirectly via x/tools/gcimporter15 tests)
For #18130.
Change-Id: I23a6d4e3a4c2a5c1ae589513da73fde7cad5f386
Reviewed-on: https://go-review.googlesource.com/35101
Reviewed-by: Alan Donovan <adonovan@google.com>
This defines the (tentative) export/import format for type aliases.
The compiler doesn't support type aliases yet, so while the code is present
it is guarded with a flag.
The export format for embedded (anonymous) fields now has three modes (mode 3 is new):
1) The original type name and the anonymous field name are the same, and the name is exported:
we don't need the field name and write "" instead
2) The original type name and the anonymous field name are the same, and the name is not exported:
we don't need the field name and write "?" instead, indicating that there is package info
3) The original type name and the anonymous field name are different:
we do need the field name and write "@" followed by the field name (and possible package info)
For #18130.
Change-Id: I790dad826757233fa71396a210f966c6256b75d3
Reviewed-on: https://go-review.googlesource.com/35100
Reviewed-by: Alan Donovan <adonovan@google.com>
XPos is a compact (8 instead of 16 bytes on a 64bit machine) source
position representation. There is a 1:1 correspondence between each
XPos and each regular Pos, translated via a global table.
In some sense this brings back the LineHist, though positions can
track line and column information; there is a O(1) translation
between the representations (no binary search), and the translation
is factored out.
The size increase with the prior change is brought down again and
the compiler speed is in line with the master repo (measured on
the same "quiet" machine as for prior change):
name old time/op new time/op delta
Template 256ms ± 1% 262ms ± 2% ~ (p=0.063 n=5+4)
Unicode 132ms ± 1% 135ms ± 2% ~ (p=0.063 n=5+4)
GoTypes 891ms ± 1% 871ms ± 1% -2.28% (p=0.016 n=5+4)
Compiler 3.84s ± 2% 3.89s ± 2% ~ (p=0.413 n=5+4)
MakeBash 47.1s ± 1% 46.2s ± 2% ~ (p=0.095 n=5+5)
name old user-ns/op new user-ns/op delta
Template 309M ± 1% 314M ± 2% ~ (p=0.111 n=5+4)
Unicode 165M ± 1% 172M ± 9% ~ (p=0.151 n=5+5)
GoTypes 1.14G ± 2% 1.12G ± 1% ~ (p=0.063 n=5+4)
Compiler 5.00G ± 1% 4.96G ± 1% ~ (p=0.286 n=5+4)
Change-Id: Icc570cc60ab014d8d9af6976f1f961ab8828cc47
Reviewed-on: https://go-review.googlesource.com/34506
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This replaces the src.Pos LineHist-based position tracking with
the syntax.Pos implementation and updates all uses.
The LineHist table is not used anymore - the respective code is still
there but should be removed eventually. CL forthcoming.
Passes toolstash -cmp when comparing to the master repo (with the
exception of a couple of swapped assembly instructions, likely due
to different instruction scheduling because the line-based sorting
has changed; though this is won't affect correctness).
The sizes of various important compiler data structures have increased
significantly (see the various sizes_test.go files); this is probably
the reason for an increase of compilation times (to be addressed). Here
are the results of compilebench -count 5, run on a "quiet" machine (no
apps running besides a terminal):
name old time/op new time/op delta
Template 256ms ± 1% 280ms ±15% +9.54% (p=0.008 n=5+5)
Unicode 132ms ± 1% 132ms ± 1% ~ (p=0.690 n=5+5)
GoTypes 891ms ± 1% 917ms ± 2% +2.88% (p=0.008 n=5+5)
Compiler 3.84s ± 2% 3.99s ± 2% +3.95% (p=0.016 n=5+5)
MakeBash 47.1s ± 1% 47.2s ± 2% ~ (p=0.841 n=5+5)
name old user-ns/op new user-ns/op delta
Template 309M ± 1% 326M ± 2% +5.18% (p=0.008 n=5+5)
Unicode 165M ± 1% 168M ± 4% ~ (p=0.421 n=5+5)
GoTypes 1.14G ± 2% 1.18G ± 1% +3.47% (p=0.008 n=5+5)
Compiler 5.00G ± 1% 5.16G ± 1% +3.12% (p=0.008 n=5+5)
Change-Id: I241c4246cdff627d7ecb95cac23060b38f9775ec
Reviewed-on: https://go-review.googlesource.com/34273
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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>
Various minor adjustments.
Change-Id: Iedfb97989f7bedaa3e9e8993b167e05f162434a7
Reviewed-on: https://go-review.googlesource.com/34136
Reviewed-by: David Lazar <lazard@golang.org>
Adjust cmd/compile accordingly.
This will make it easier to replace the underlying implementation.
Change-Id: I33645850bb18c839b24785b6222a9e028617addb
Reviewed-on: https://go-review.googlesource.com/34133
Reviewed-by: David Lazar <lazard@golang.org>
Matthew Dempsky
Daniel Martí
Robert Griesemer
Josh Bleecher Snyder
Dave Cheney
Aliaksandr Valialkin
David Lazar
Russ Cox
shawnps