Abstract the typecheck API a bit more so that it is
easier to move into a new package.
Change-Id: Ia0a0146151fa7f6073113e68a2c3f6e42a5d0ad8
Reviewed-on: https://go-review.googlesource.com/c/go/+/279303
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
isIntrinsicCall and ssaDumpInline are the only two "forward references"
to ssa by earlier phases. Make them a bit more explicit so that the
uses and the definitions can end up in different packages.
Change-Id: I02c7a27464fbedef9fee43c0e4094fa08b4d7a5c
Reviewed-on: https://go-review.googlesource.com/c/go/+/279300
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
There are various global variables tracking the state of the
compilation. Collect them in a single global struct instead.
The struct definition is in package ir, but the struct itself is
still in package gc. It may eventually be threaded through the
code, but in the short term will end up in package typecheck.
Change-Id: I019db07aaedaed2c9b67dd45a4e138dc6028e54c
Reviewed-on: https://go-review.googlesource.com/c/go/+/279297
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The prealloc map seems to exist to avoid adding a field to all nodes.
Now we can add a field to just the nodes that need the field,
so let's do that and avoid having a magic global with extra node state
that isn't preserved by operations like Copy nor printed by Dump.
This also makes clear which nodes can be prealloc'ed.
In particular, the code in walkstmt looked up an entry in
prealloc using an ONAME node, but there's no code that
ever stores such an entry, so the lookup never succeeded.
Having fields makes that kind of thing easier to see and fix.
Passes buildall w/ toolstash -cmp.
Change-Id: I418ad0e2847615c08868120c13ee719dc0b2eacb
Reviewed-on: https://go-review.googlesource.com/c/go/+/278915
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
For globals, Name.Offset is used as a way to address a field within
a global during static initialization. This CL replaces that use with
a separate NameOffsetExpr (ONAMEOFFSET) node.
For locals, Name.Offset is the stack frame offset. This CL calls it
that (FrameOffset, SetFrameOffset).
Now there is no longer any use of Name.Offset or Name.SetOffset.
And now that copies of Names are not being made to change their
offsets, we can lock down use of ir.Copy on Names. The only
remaining uses are during inlining and in handling generic system
functions. At both those times you do want to create a new name
and that can be made explicit by calling the new CloneName method
instead. ir.Copy on a name now panics.
Passes buildall w/ toolstash -cmp.
Change-Id: I0b0a25b9d93aeff7cf4e4025ac53faec7dc8603b
Reviewed-on: https://go-review.googlesource.com/c/go/+/278914
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
An automated rewrite will add concrete type assertions after
a test of n.Op(), when n can be safely type-asserted
(meaning, n is not reassigned a different type, n is not reassigned
and then used outside the scope of the type assertion,
and so on).
This sequence of CLs handles the code that the automated
rewrite does not: adding specific types to function arguments,
adjusting code not to call n.Left() etc when n may have multiple
representations, and so on.
This CL focuses on walk.go.
Passes buildall w/ toolstash -cmp.
Change-Id: I7aab57e4077cf10da1994625575c5e42ad114a9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/277921
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
ir.Find is called "any" in C#, Dart, Haskell, Python, R, Ruby, and Rust,
and "any_of" in C++, "anyMatch" in Java, "some" in JavaScript,
"exists in OCaml, and "existsb" in Coq.
(Thanks to Matthew Dempsky for the research.)
This CL changes Find to Any to use the mostly standard name.
It also updates wrapper helpers to use the any terminology:
hasCall -> anyCall
hasCallOrChan -> anyCallOrChan
hasSideEffects -> anySideEffects
Unchanged are "hasNamedResults", "hasUniquePos", and "hasDefaultCase",
which are all about a single node, not any node in the IR tree.
I also renamed hasFall to endsInFallthrough, since its semantics are
neither that of "any" nor that of the remaining "has" functions.
So the new terminology helps separate different kinds of predicates nicely.
Change-Id: I9bb3c9ebf060a30447224be09a5c34ad5244ea0d
Reviewed-on: https://go-review.googlesource.com/c/go/+/278912
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
An automated rewrite will add concrete type assertions after
a test of n.Op(), when n can be safely type-asserted
(meaning, n is not reassigned a different type, n is not reassigned
and then used outside the scope of the type assertion,
and so on).
This sequence of CLs handles the code that the automated
rewrite does not: adding specific types to function arguments,
adjusting code not to call n.Left() etc when n may have multiple
representations, and so on.
This CL focuses on range.go, select.go, and swt.go: the big
control structures.
Passes buildall w/ toolstash -cmp.
Change-Id: I033fe056a7b815edb6e8a06f45c12ffd990f4d45
Reviewed-on: https://go-review.googlesource.com/c/go/+/277929
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
An automated rewrite will add concrete type assertions after
a test of n.Op(), when n can be safely type-asserted
(meaning, n is not reassigned a different type, n is not reassigned
and then used outside the scope of the type assertion,
and so on).
This sequence of CLs handles the code that the automated
rewrite does not: adding specific types to function arguments,
adjusting code not to call n.Left() etc when n may have multiple
representations, and so on.
This CL focuses on ssa.go.
Passes buildall w/ toolstash -cmp.
Change-Id: Iefacc7104dd9469e3c574149791ab0bff29f7fee
Reviewed-on: https://go-review.googlesource.com/c/go/+/277923
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
It seems clear after using these for a week that Find need not return
anything other than a bool saying whether the target was found.
The main reason for not using the boolean earlier was to avoid confusion
with Inspect: for Find, returning true means "it was found! stop walking"
while for Inspect, returning true means "keep walking the children".
But it turns out that none of the uses of Inspect need the boolean.
This makes sense because types can contain expressions, expressions
can contain statements (inside function literals), and so on, so there
are essentially no times when you can say based on the current AST node
that the children are irrelevant to a particular operation.
So this CL makes two changes:
1) Change Find to return a boolean and to take a callback function
returning a boolean. This simplifies all existing calls to Find.
2) Rename Inspect to Visit and change it to take a callback with no
result at all. This simplifies all existing calls to Inspect.
Removing the boolean result from Inspect's callback avoids having
two callbacks with contradictory boolean results in different APIs.
Renaming Inspect to Visit avoids confusion with ast.Inspect.
Passes buildall w/ toolstash -cmp.
Change-Id: I344ebb5e00b6842012be33e779db483c28e5f350
Reviewed-on: https://go-review.googlesource.com/c/go/+/277919
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
An automated rewrite is going to remove the bulk of the calls
to ir.Nod and friends. This CL takes care of the ones that don't
have fixed opcodes and so aren't amenable to automatic rewriting.
Passes buildall w/ toolstash -cmp.
Replay of CL 275886, lost to the bad-merge history rewrite.
Change-Id: I5bf8d1d182f847f4ab44b7e278b752913e30e4c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/277956
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Avoid using the same variable for two different concrete
Node types in other files (beyond walk). This will smooth the
introduction of specific constructors, replacing ir.Nod and friends.
Passes buildall w/ toolstash -cmp.
Replay of CL 275885, lost to the bad-merge history rewrite.
Change-Id: I0da89502a0bd636b8766f01b6f843c7821b3e9ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/277955
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Avoid using the same variable for two different concrete
Node types in walk. This will smooth the introduction of
specific constructors, replacing ir.Nod and friends.
Passes buildall w/ toolstash -cmp.
Replay of CL 275884, lost to the bad-merge history rewrite.
Change-Id: I05628e20a19c9559ed7478526ef6cb2613f735e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/277954
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is a mechanical change to intercept the construction of
all OADDR nodes. We will use the new nodAddr and nodAddrAt
functions to compute the Addrtaken bit.
Change-Id: I90ee3acb8e32540a198a9999284573418729f422
Reviewed-on: https://go-review.googlesource.com/c/go/+/275694
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Dan Scales <danscales@google.com>
This CL adds Ident, which will eventually replace *Name and *PkgName
within the AST for representing uses of declared names. (Originally, I
intended to call it "IdentExpr", but neither go/ast nor
cmd/compile/internal/syntax include the "Expr" suffix for their
respective types.)
To start, this CL converts two uses of *Name to *Ident: the tag
identifier in a TypeSwitchGuard (which doesn't actually declare a
variable by itself), and the not-yet-known placeholder ONONAME
returned by oldname to stand-in for identifiers that might be declared
later in the package.
The TypeSwitchGuard's Name's Used flag was previously used for
detecting whether none of the per-clause variables were used. To avoid
bloating all Idents for this rare use, a "Used" bool is added to
TypeSwitchGuard instead. Eventually it could maybe be packed into
miniNode.bits, but for now this is good enough.
Passes buildall w/ toolstash -cmp.
Change-Id: I393284d86757cbbebd26e1320c7354e2bdcb30b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/276113
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Ending them in a returning switch makes it safe for each case
to do an appropriate type assertion.
Passes buildall w/ toolstash -cmp.
Change-Id: I55d8f0a555006104164d84d27822aa8c5ad68515
Reviewed-on: https://go-review.googlesource.com/c/go/+/275882
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
On ir.Node, ir.Nodes, and ir.Op, # is ignored, so %#v is %v.
On ir.Node, %S is the same as %v.
On types.Type, # is ignored, so %#L is %L, %#v is %v.
On types.Type, 0 is ignored, so %0S is %S.
Rewrite all these using go test cmd/compile -r, plus a
few multiline formats mentioning %0S on types updated by hand.
Now the formats used in the compiler match the documentation
for the format methods, a minor miracle.
Passes buildall w/ toolstash -cmp.
Change-Id: I3d4a3fae543145a68da13eede91166632c5b1ceb
Reviewed-on: https://go-review.googlesource.com/c/go/+/275782
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Move the printing of types.Type and types.Sym out of ir
into package types, where it properly belongs. This wasn't
done originally (when the code was in gc) because the Type
and Sym printing was a bit tangled up with the Node printing.
But now they are untangled and can move into the correct
package.
This CL is automatically generated.
A followup CL will clean up a little bit more by hand.
Passes buildall w/ toolstash -cmp.
[git-generate]
cd src/cmd/compile/internal/ir
rf '
mv FmtMode fmtMode
mv FErr fmtGo
mv FDbg fmtDebug
mv FTypeId fmtTypeID
mv FTypeIdName fmtTypeIDName
mv methodSymName SymMethodName
mv BuiltinPkg LocalPkg BlankSym OrigSym NumImport \
fmtMode fmtGo symFormat sconv sconv2 symfmt SymMethodName \
BasicTypeNames fmtBufferPool InstallTypeFormats typeFormat tconv tconv2 fldconv FmtConst \
typefmt.go
mv typefmt.go cmd/compile/internal/types
'
cd ../types
mv typefmt.go fmt.go
Change-Id: I6f3fd818323733ab8446f00594937c1628760b27
Reviewed-on: https://go-review.googlesource.com/c/go/+/275779
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
nconvFmt calls base.Fatalf if mode is anything but FErr,
proving that the only formats that matter for nodes are
plain %v, %S, and %L. And the nodes formatter can only get to %v.
(%S and %v are the same; we'll clean that up separately.)
Node and Nodes can therefore ignore mode, and all the mode
code can be removed from those implementations, removing
quite a few layers of abstraction.
Op similarly only runs in one mode and can be simplified.
Passes buildall w/ toolstash -cmp.
Change-Id: Ibfd845033e9c68181a20fb81c8f3dd428463920a
Reviewed-on: https://go-review.googlesource.com/c/go/+/275775
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL converts all the generic searching traversal to use ir.Find
instead of relying on direct access to Left, Right, and so on.
Passes buildall w/ toolstash -cmp.
Change-Id: I4d951aef630c00bf333f24be79565cc564694d04
Reviewed-on: https://go-review.googlesource.com/c/go/+/275372
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
It's useful to have quick access to the types.Field that a given
selector or method value expression refer to. Previously we abused Opt
for this, but couldn't do that for OCALLPART because escape analysis
uses Opt.
Now that we have more flexibility, we can simply add additional
pointer fields for this. This also allows getting rid of an unneeded
ONAME node for OCALLPART.
Passes buildall w/ toolstash -cmp.
Change-Id: I980d7bdb19abfd0b6f58a232876861b88dee1e47
Reviewed-on: https://go-review.googlesource.com/c/go/+/275034
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
The next CL adds ConstExpr, which is a more memory efficient
representation for constant expressions than Name. However, currently
a bunch of Val helper methods are defined on Name. This CL changes
them into standalone functions that work with any Node.Val
implementation.
There's also an existing standalone function named Int64Val, which
takes a Type argument to specify what type of integer is expected. So
to avoid collisions, this CL renames it to IntVal.
Passes buildall w/ toolstash -cmp.
[git-generate]
cd src/cmd/compile/internal/ir
rf 'mv Int64Val IntVal'
sed -i -E -e 's/\(n \*Name\) (CanInt64|((I|Ui)nt64|Bool|String)Val)\(/\1(n Node/' name.go
cd ../gc
rf '
ex {
import "cmd/compile/internal/ir"
var n ir.Node
n.CanInt64() -> ir.CanInt64(n)
n.Int64Val() -> ir.Int64Val(n)
n.Uint64Val() -> ir.Uint64Val(n)
n.BoolVal() -> ir.BoolVal(n)
n.StringVal() -> ir.StringVal(n)
}
'
cd ../ir
rf '
mv CanInt64 Int64Val Uint64Val BoolVal StringVal val.go
rm Node.CanInt64 Node.Int64Val Node.Uint64Val Node.BoolVal Node.StringVal
'
Change-Id: I003140bda1690d770fd608bdd087e6d4ff00fb1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/275032
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Recreated manually to push below some CLs it depended on.
Change-Id: I1b3316fcdce39cbb33e5cbb471f5cd1cd2efc1f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/274599
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL improves handling of OCONVNOP nodes during ssa generation,
so it is not toolstash safe.
An OCONVNOP wrapper is necessary for the "for" condition of
certain compiled range loops, and the boolean evaluator was
not looking through them properly, generating unnecessary
temporaries. That change saved 8k of the (13 MB) go binary.
The other changes just streamline the handling of OCONVNOP
to be more like what OSTMTEXPR will be like. They have no
effect on output size but do tweak the ssa graph a little, which
causes different register decisions and therefore different output.
Change-Id: I9e1dcd413b60944e21554c3e3f2bdc9adcee7634
Reviewed-on: https://go-review.googlesource.com/c/go/+/274598
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
For statements like goto that don't need an init, use an
explicit block statement instead of forcing them to have one.
There is also one call to addinit that is being replaced with
a block. That call is the source of much of my confusion
regarding init statements: walkstmt calls addinit on a statement,
whereas all the other uses of addinit are on expressions.
After this CL, they're all expressions.
Passes buildall w/ toolstash -cmp.
Change-Id: Ifdef9d318c236dc1a7567f9e9ef4a6bedd3fe81f
Reviewed-on: https://go-review.googlesource.com/c/go/+/274597
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
OEMPTY is an empty *statement*, but it confusingly
gets handled as an expression in a few places.
More confusingly, OEMPTY often has an init list,
making it not empty at all. Replace uses and analysis
of OEMPTY with OBLOCK instead.
Passes buildall w/ toolstash -cmp.
Change-Id: I8d4fcef151e4f441fa19b1b96da5272d778131d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/274594
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL adds the remaining constructors needed to abstract away
construction of Types, and updates the compiler to use them
throughout. There's now just a couple uses within test cases to
remove.
While at it, I also replace the Func.Outnamed field with a simple
helper function, which reduces the size of function types somewhat.
Passes toolstash/buildall.
Change-Id: If1aa1095c98ae34b00380d0b3531bd63c10ce885
Reviewed-on: https://go-review.googlesource.com/c/go/+/274713
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Just clearing away some scaffolding artifacts from previous
refactorings.
[git-generate]
cd src/cmd/compile/internal/gc
rf '
ex {
import "cmd/compile/internal/ir"
import "cmd/compile/internal/types"
var n *ir.Name; n.Name() -> n
var f *ir.Func; f.Func() -> f
var o types.Object
ir.AsNode(o).Sym() -> o.Sym()
ir.AsNode(o).Type() -> o.Type()
ir.AsNode(o).(*ir.Name) -> o.(*ir.Name)
ir.AsNode(o).(*ir.Func) -> o.(*ir.Func)
var x ir.Node
ir.AsNode(o) != x -> o != x
}
'
Change-Id: I946ec344bd7ee274900a392da53b95308ceaade4
Reviewed-on: https://go-review.googlesource.com/c/go/+/274592
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Using expression nodes restricts the set of valid SetOp operations,
because you can't SetOp across representation. Rewrite various code
to avoid crossing those as-yet-unintroduced boundaries.
This also includes choosing a single representation for any given Op.
For example, OCLOSE starts out as an OCALL, so it starts with a List
of one node and then moves that node to Left. That's no good with
real data structures, so the code picks a single canonical implementation
and prepares it during the conversion from one Op to the next.
In this case, the conversion of an OCALL to an OCLOSE now creates
a new node with Left initialized from the start. This pattern repeats.
Passes buildall w/ toolstash -cmp.
Change-Id: I55a0872c614d883cac9d64976c46aeeaa639e25d
Reviewed-on: https://go-review.googlesource.com/c/go/+/274107
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Using statement nodes restricts the set of valid SetOp operations,
because you can't SetOp across representation. Rewrite various
code to avoid crossing those as-yet-unintroduced boundaries.
In particular, code like
x, y := v.(T)
x, y := f()
x, y := m[k]
x, y := <-c
starts out with Op = OAS2, and then it turns into a specific Op
OAS2DOTTYPE, OAS2FUNC, OAS2MAPR, OAS2RECV, and then
later in walk is lowered to an OAS2 again.
In the middle, the specific forms move the right-hand side from
n.Rlist().First() to n.Right(), and then the conversion to OAS2 moves
it back. This is unnecessary and makes it hard for these all to
share an underlying Node implementation.
This CL changes these specific forms to leave the right-hand side
in n.Rlist().First().
Similarly, OSELRECV2 is really just a temporary form of OAS2.
This CL changes it to use same fields too.
Finally, this CL fixes the printing of OAS2 nodes in ir/fmt.go,
which formerly printed n.Right() instead of n.Rlist().
This results in a (correct!) update to cmd/compile/internal/logopt's
expected output: ~R0 = <N> becomes ~R0 = &y.b.
Passes buildall w/ toolstash -cmp.
Change-Id: I164aa2e17dc55bfb292024de53d7d250192ad64a
Reviewed-on: https://go-review.googlesource.com/c/go/+/274105
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The type syntax is reused to stand in for the actual type once typechecked,
to avoid updating all the possible references to the original type syntax.
So all these implementations allow changing their Op from the raw syntax
like OTMAP to the finished form OTYPE, even though obviously the
representation does not change.
Passes buildall w/ toolstash -cmp.
Change-Id: I4acca1a5b35fa2f48ee08e8f1e5a330a004c284b
Reviewed-on: https://go-review.googlesource.com/c/go/+/274103
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Now there are no longer any generic nodes with a non-nil
associated Func, so node.fn can be deleted. Also all manipulation
of func fields is done with concrete types, so Node.SetFunc can be
deleted, along with generic implementations.
Passes buildall w/ toolstash -cmp.
Change-Id: I4fee99870951ec9dc224f146d87b22e2bfe16889
Reviewed-on: https://go-review.googlesource.com/c/go/+/274099
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Closures are another reference to Funcs,
and it cleans up the code quite a bit to be clear about types.
OCLOSUREVAR is renamed to OCLOSUREREAD to make
clearer that it is unrelated to the list Func.ClosureVars.
Passes buildall w/ toolstash -cmp.
Change-Id: Id0d28df2d4d6e9954e34df7a39ea226995eee937
Reviewed-on: https://go-review.googlesource.com/c/go/+/274098
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Now that we have specific types for ONAME and ODCLFUNC nodes
(*Name and *Func), use them throughout the compiler to be more
precise about what data is being operated on.
This is a somewhat large CL, but once you start applying the types
in a few places, you end up needing to apply them to many other
places to keep everything type-checking. A lot of code also melts
away as types are added.
Passes buildall w/ toolstash -cmp.
Change-Id: I21dd9b945d701c470332bac5394fca744a5b232d
Reviewed-on: https://go-review.googlesource.com/c/go/+/274097
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Before this CL, an ONAME Node was represented by three structs
linked together: a node, a Name, and a Param. Previous CLs removed
OLABEL and OPACK from the set of nodes that knew about Name.
Now Name can be repurposed to *be* the ONAME Node implementation,
replacing three linked structs totaling 152+64+88 = 304 bytes (64-bit)
with a single 232-byte struct.
Many expressions in the code become simpler as well, without having
to use .Param. and sometimes even .Name().
(For a node n where n.Name() != nil, n.Name() == n.(*Name) now.)
Passes buildall w/ toolstash -cmp.
Change-Id: Ie719f1285c05623b9fd2faaa059e5b360a64b3be
Reviewed-on: https://go-review.googlesource.com/c/go/+/274094
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
In preparation for OEMPTY being its own Node implementation,
remove SetOp(OEMPTY) calls that assume other implementations
can be turned into OEMPTY.
Passes buildall w/ toolstash -cmp.
Change-Id: Icac16d12548f35f52a5efa9d09dacf8260f42075
Reviewed-on: https://go-review.googlesource.com/c/go/+/274090
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The plan is to introduce a Node interface that replaces the old *Node pointer-to-struct.
The previous CL defined an interface INode modeling a *Node.
This CL:
- Changes all references outside internal/ir to use INode,
along with many references inside internal/ir as well.
- Renames Node to node.
- Renames INode to Node
So now ir.Node is an interface implemented by *ir.node, which is otherwise inaccessible,
and the code outside package ir is now (clearly) using only the interface.
The usual rule is never to redefine an existing name with a new meaning,
so that old code that hasn't been updated gets a "unknown name" error
instead of more mysterious errors or silent misbehavior. That rule would
caution against replacing Node-the-struct with Node-the-interface,
as in this CL, because code that says *Node would now be using a pointer
to an interface. But this CL is being landed at the same time as another that
moves Node from gc to ir. So the net effect is to replace *gc.Node with ir.Node,
which does follow the rule: any lingering references to gc.Node will be told
it's gone, not silently start using pointers to interfaces. So the rule is followed
by the CL sequence, just not this specific CL.
Overall, the loss of inlining caused by using interfaces cuts the compiler speed
by about 6%, a not insignificant amount. However, as we convert the representation
to concrete structs that are not the giant Node over the next weeks, that speed
should come back as more of the compiler starts operating directly on concrete types
and the memory taken up by the graph of Nodes drops due to the more precise
structs. Honestly, I was expecting worse.
% benchstat bench.old bench.new
name old time/op new time/op delta
Template 168ms ± 4% 182ms ± 2% +8.34% (p=0.000 n=9+9)
Unicode 72.2ms ±10% 82.5ms ± 6% +14.38% (p=0.000 n=9+9)
GoTypes 563ms ± 8% 598ms ± 2% +6.14% (p=0.006 n=9+9)
Compiler 2.89s ± 4% 3.04s ± 2% +5.37% (p=0.000 n=10+9)
SSA 6.45s ± 4% 7.25s ± 5% +12.41% (p=0.000 n=9+10)
Flate 105ms ± 2% 115ms ± 1% +9.66% (p=0.000 n=10+8)
GoParser 144ms ±10% 152ms ± 2% +5.79% (p=0.011 n=9+8)
Reflect 345ms ± 9% 370ms ± 4% +7.28% (p=0.001 n=10+9)
Tar 149ms ± 9% 161ms ± 5% +8.05% (p=0.001 n=10+9)
XML 190ms ± 3% 209ms ± 2% +9.54% (p=0.000 n=9+8)
LinkCompiler 327ms ± 2% 325ms ± 2% ~ (p=0.382 n=8+8)
ExternalLinkCompiler 1.77s ± 4% 1.73s ± 6% ~ (p=0.113 n=9+10)
LinkWithoutDebugCompiler 214ms ± 4% 211ms ± 2% ~ (p=0.360 n=10+8)
StdCmd 14.8s ± 3% 15.9s ± 1% +6.98% (p=0.000 n=10+9)
[Geo mean] 480ms 510ms +6.31%
name old user-time/op new user-time/op delta
Template 223ms ± 3% 237ms ± 3% +6.16% (p=0.000 n=9+10)
Unicode 103ms ± 6% 113ms ± 3% +9.53% (p=0.000 n=9+9)
GoTypes 758ms ± 8% 800ms ± 2% +5.55% (p=0.003 n=10+9)
Compiler 3.95s ± 2% 4.12s ± 2% +4.34% (p=0.000 n=10+9)
SSA 9.43s ± 1% 9.74s ± 4% +3.25% (p=0.000 n=8+10)
Flate 132ms ± 2% 141ms ± 2% +6.89% (p=0.000 n=9+9)
GoParser 177ms ± 9% 183ms ± 4% ~ (p=0.050 n=9+9)
Reflect 467ms ±10% 495ms ± 7% +6.17% (p=0.029 n=10+10)
Tar 183ms ± 9% 197ms ± 5% +7.92% (p=0.001 n=10+10)
XML 249ms ± 5% 268ms ± 4% +7.82% (p=0.000 n=10+9)
LinkCompiler 544ms ± 5% 544ms ± 6% ~ (p=0.863 n=9+9)
ExternalLinkCompiler 1.79s ± 4% 1.75s ± 6% ~ (p=0.075 n=10+10)
LinkWithoutDebugCompiler 248ms ± 6% 246ms ± 2% ~ (p=0.965 n=10+8)
[Geo mean] 483ms 504ms +4.41%
[git-generate]
cd src/cmd/compile/internal/ir
: # We need to do the conversion in multiple steps, so we introduce
: # a temporary type alias that will start out meaning the pointer-to-struct
: # and then change to mean the interface.
rf '
mv Node OldNode
add node.go \
type Node = *OldNode
'
: # It should work to do this ex in ir, but it misses test files, due to a bug in rf.
: # Run the command in gc to handle gc's tests, and then again in ssa for ssa's tests.
cd ../gc
rf '
ex . ../arm ../riscv64 ../arm64 ../mips64 ../ppc64 ../mips ../wasm {
import "cmd/compile/internal/ir"
*ir.OldNode -> ir.Node
}
'
cd ../ssa
rf '
ex {
import "cmd/compile/internal/ir"
*ir.OldNode -> ir.Node
}
'
: # Back in ir, finish conversion clumsily with sed,
: # because type checking and circular aliases do not mix.
cd ../ir
sed -i '' '
/type Node = \*OldNode/d
s/\*OldNode/Node/g
s/^func (n Node)/func (n *OldNode)/
s/OldNode/node/g
s/type INode interface/type Node interface/
s/var _ INode = (Node)(nil)/var _ Node = (*node)(nil)/
' *.go
gofmt -w *.go
sed -i '' '
s/{Func{}, 136, 248}/{Func{}, 152, 280}/
s/{Name{}, 32, 56}/{Name{}, 44, 80}/
s/{Param{}, 24, 48}/{Param{}, 44, 88}/
s/{node{}, 76, 128}/{node{}, 88, 152}/
' sizeof_test.go
cd ../ssa
sed -i '' '
s/{LocalSlot{}, 28, 40}/{LocalSlot{}, 32, 48}/
' sizeof_test.go
cd ../gc
sed -i '' 's/\*ir.Node/ir.Node/' mkbuiltin.go
cd ../../../..
go install std cmd
cd cmd/compile
go test -u || go test -u
Change-Id: I196bbe3b648e4701662e4a2bada40bf155e2a553
Reviewed-on: https://go-review.googlesource.com/c/go/+/272935
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
If we want to break up package gc at all, we will need to move
the compiler IR it defines into a separate package that can be
imported by packages that gc itself imports. This CL does that.
It also removes the TINT8 etc aliases so that all code is clear
about which package things are coming from.
This CL is automatically generated by the script below.
See the comments in the script for details about the changes.
[git-generate]
cd src/cmd/compile/internal/gc
rf '
# These names were never fully qualified
# when the types package was added.
# Do it now, to avoid confusion about where they live.
inline -rm \
Txxx \
TINT8 \
TUINT8 \
TINT16 \
TUINT16 \
TINT32 \
TUINT32 \
TINT64 \
TUINT64 \
TINT \
TUINT \
TUINTPTR \
TCOMPLEX64 \
TCOMPLEX128 \
TFLOAT32 \
TFLOAT64 \
TBOOL \
TPTR \
TFUNC \
TSLICE \
TARRAY \
TSTRUCT \
TCHAN \
TMAP \
TINTER \
TFORW \
TANY \
TSTRING \
TUNSAFEPTR \
TIDEAL \
TNIL \
TBLANK \
TFUNCARGS \
TCHANARGS \
NTYPE \
BADWIDTH
# esc.go and escape.go do not need to be split.
# Append esc.go onto the end of escape.go.
mv esc.go escape.go
# Pull out the type format installation from func Main,
# so it can be carried into package ir.
mv Main:/Sconv.=/-0,/TypeLinkSym/-1 InstallTypeFormats
# Names that need to be exported for use by code left in gc.
mv Isconst IsConst
mv asNode AsNode
mv asNodes AsNodes
mv asTypesNode AsTypesNode
mv basicnames BasicTypeNames
mv builtinpkg BuiltinPkg
mv consttype ConstType
mv dumplist DumpList
mv fdumplist FDumpList
mv fmtMode FmtMode
mv goopnames OpNames
mv inspect Inspect
mv inspectList InspectList
mv localpkg LocalPkg
mv nblank BlankNode
mv numImport NumImport
mv opprec OpPrec
mv origSym OrigSym
mv stmtwithinit StmtWithInit
mv dump DumpAny
mv fdump FDumpAny
mv nod Nod
mv nodl NodAt
mv newname NewName
mv newnamel NewNameAt
mv assertRepresents AssertValidTypeForConst
mv represents ValidTypeForConst
mv nodlit NewLiteral
# Types and fields that need to be exported for use by gc.
mv nowritebarrierrecCallSym SymAndPos
mv SymAndPos.lineno SymAndPos.Pos
mv SymAndPos.target SymAndPos.Sym
mv Func.lsym Func.LSym
mv Func.setWBPos Func.SetWBPos
mv Func.numReturns Func.NumReturns
mv Func.numDefers Func.NumDefers
mv Func.nwbrCalls Func.NWBRCalls
# initLSym is an algorithm left behind in gc,
# not an operation on Func itself.
mv Func.initLSym initLSym
mv nodeQueue NodeQueue
mv NodeQueue.empty NodeQueue.Empty
mv NodeQueue.popLeft NodeQueue.PopLeft
mv NodeQueue.pushRight NodeQueue.PushRight
# Many methods on Node are actually algorithms that
# would apply to any node implementation.
# Those become plain functions.
mv Node.funcname FuncName
mv Node.isBlank IsBlank
mv Node.isGoConst isGoConst
mv Node.isNil IsNil
mv Node.isParamHeapCopy isParamHeapCopy
mv Node.isParamStackCopy isParamStackCopy
mv Node.isSimpleName isSimpleName
mv Node.mayBeShared MayBeShared
mv Node.pkgFuncName PkgFuncName
mv Node.backingArrayPtrLen backingArrayPtrLen
mv Node.isterminating isTermNode
mv Node.labeledControl labeledControl
mv Nodes.isterminating isTermNodes
mv Nodes.sigerr fmtSignature
mv Node.MethodName methodExprName
mv Node.MethodFunc methodExprFunc
mv Node.IsMethod IsMethod
# Every node will need to implement RawCopy;
# Copy and SepCopy algorithms will use it.
mv Node.rawcopy Node.RawCopy
mv Node.copy Copy
mv Node.sepcopy SepCopy
# Extract Node.Format method body into func FmtNode,
# but leave method wrapper behind.
mv Node.Format:0,$ FmtNode
# Formatting helpers that will apply to all node implementations.
mv Node.Line Line
mv Node.exprfmt exprFmt
mv Node.jconv jconvFmt
mv Node.modeString modeString
mv Node.nconv nconvFmt
mv Node.nodedump nodeDumpFmt
mv Node.nodefmt nodeFmt
mv Node.stmtfmt stmtFmt
# Constant support needed for code moving to ir.
mv okforconst OKForConst
mv vconv FmtConst
mv int64Val Int64Val
mv float64Val Float64Val
mv Node.ValueInterface ConstValue
# Organize code into files.
mv LocalPkg BuiltinPkg ir.go
mv NumImport InstallTypeFormats Line fmt.go
mv syntax.go Nod NodAt NewNameAt Class Pxxx PragmaFlag Nointerface SymAndPos \
AsNode AsTypesNode BlankNode OrigSym \
Node.SliceBounds Node.SetSliceBounds Op.IsSlice3 \
IsConst Node.Int64Val Node.CanInt64 Node.Uint64Val Node.BoolVal Node.StringVal \
Node.RawCopy SepCopy Copy \
IsNil IsBlank IsMethod \
Node.Typ Node.StorageClass node.go
mv ConstType ConstValue Int64Val Float64Val AssertValidTypeForConst ValidTypeForConst NewLiteral idealType OKForConst val.go
# Move files to new ir package.
mv bitset.go class_string.go dump.go fmt.go \
ir.go node.go op_string.go val.go \
sizeof_test.go cmd/compile/internal/ir
'
: # fix mkbuiltin.go to generate the changes made to builtin.go during rf
sed -i '' '
s/\[T/[types.T/g
s/\*Node/*ir.Node/g
/internal\/types/c \
fmt.Fprintln(&b, `import (`) \
fmt.Fprintln(&b, ` "cmd/compile/internal/ir"`) \
fmt.Fprintln(&b, ` "cmd/compile/internal/types"`) \
fmt.Fprintln(&b, `)`)
' mkbuiltin.go
gofmt -w mkbuiltin.go
: # update cmd/dist to add internal/ir
cd ../../../dist
sed -i '' '/compile.internal.gc/a\
"cmd/compile/internal/ir",
' buildtool.go
gofmt -w buildtool.go
: # update cmd/compile TestFormats
cd ../..
go install std cmd
cd cmd/compile
go test -u || go test # first one updates but fails; second passes
Change-Id: I5f7caf6b20629b51970279e81231a3574d5b51db
Reviewed-on: https://go-review.googlesource.com/c/go/+/273008
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Move Flag, Debug, Ctxt, Exit, and error messages to
new package cmd/compile/internal/base.
These are the core functionality that everything in gc uses
and which otherwise prevent splitting any other code
out of gc into different packages.
A minor milestone: the compiler source code
no longer contains the string "yy".
[git-generate]
cd src/cmd/compile/internal/gc
rf '
mv atExit AtExit
mv Ctxt atExitFuncs AtExit Exit base.go
mv lineno Pos
mv linestr FmtPos
mv flusherrors FlushErrors
mv yyerror Errorf
mv yyerrorl ErrorfAt
mv yyerrorv ErrorfVers
mv noder.yyerrorpos noder.errorAt
mv Warnl WarnfAt
mv errorexit ErrorExit
mv base.go debug.go flag.go print.go cmd/compile/internal/base
'
: # update comments
sed -i '' 's/yyerrorl/ErrorfAt/g; s/yyerror/Errorf/g' *.go
: # bootstrap.go is not built by default so invisible to rf
sed -i '' 's/Fatalf/base.Fatalf/' bootstrap.go
goimports -w bootstrap.go
: # update cmd/dist to add internal/base
cd ../../../dist
sed -i '' '/internal.amd64/a\
"cmd/compile/internal/base",
' buildtool.go
gofmt -w buildtool.go
Change-Id: I59903c7084222d6eaee38823fd222159ba24a31a
Reviewed-on: https://go-review.googlesource.com/c/go/+/272250
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The debug table is not as haphazard as flags, but there are still
a few mismatches between command-line names and variable names.
This CL moves them all into a consistent home (var Debug, like var Flag).
Code updated automatically using the rf command below.
A followup CL will make a few manual cleanups, leaving this CL
completely automated and easier to regenerate during merge
conflicts.
[git-generate]
cd src/cmd/compile/internal/gc
rf '
add main.go var Debug struct{}
mv Debug_append Debug.Append
mv Debug_checkptr Debug.Checkptr
mv Debug_closure Debug.Closure
mv Debug_compilelater Debug.CompileLater
mv disable_checknil Debug.DisableNil
mv debug_dclstack Debug.DclStack
mv Debug_gcprog Debug.GCProg
mv Debug_libfuzzer Debug.Libfuzzer
mv Debug_checknil Debug.Nil
mv Debug_panic Debug.Panic
mv Debug_slice Debug.Slice
mv Debug_typeassert Debug.TypeAssert
mv Debug_wb Debug.WB
mv Debug_export Debug.Export
mv Debug_pctab Debug.PCTab
mv Debug_locationlist Debug.LocationLists
mv Debug_typecheckinl Debug.TypecheckInl
mv Debug_gendwarfinl Debug.DwarfInl
mv Debug_softfloat Debug.SoftFloat
mv Debug_defer Debug.Defer
mv Debug_dumpptrs Debug.DumpPtrs
mv flag.go:/parse.-d/-1,/unknown.debug/+2 parseDebug
mv debugtab Debug parseDebug \
debugHelpHeader debugHelpFooter \
debug.go
# Remove //go:generate line copied from main.go
rm debug.go:/go:generate/-+
'
Change-Id: I625761ca5659be4052f7161a83baa00df75cca91
Reviewed-on: https://go-review.googlesource.com/c/go/+/272246
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
evconst is one of the largest sources of Op rewrites,
which prevent separating different kinds of nodes
(in this case, arithmetic nodes and OLITERAL nodes).
The change in swt.go is necessary because otherwise
the syntax graph ends up containing that OLEN expression
multiple times, which violates the invariant that it's a tree
except for ONAME, OLITERAL, and OTYPE nodes.
(Before, the OLEN was overwritten by an OLITERAL, so the
invariant still held, but now that we don't overwrite it,
we need a different copy for each instance.)
Passes toolstash -cmp.
Change-Id: Ia004774ab6852fb384805d0f9f9f234b40842811
Reviewed-on: https://go-review.googlesource.com/c/go/+/272869
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
A method expression today is an ONAME that has none of the
invariants or properties of other ONAMEs and is always a special case
(hence the Node.IsMethodExpression method).
Remove the special cases by making a separate Op.
Passes toolstash -cmp.
Change-Id: I7667693c9155d5486a6924dbf75ebb59891c4afc
Reviewed-on: https://go-review.googlesource.com/c/go/+/272867
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The original meaning of type Func was "extra fields factored out
of a few cases of type Node having to do with functions",
but those specific cases didn't necessarily have any relation.
A typical declared function is represented by an ODCLFUNC Node
at its declaration and an ONAME node at its uses, and both those
have a .Func field, but they are *different* Funcs.
Similarly, a closure is represented both by an OCLOSURE Node for
the value itself and an ODCLFUNC Node for the underlying function
implementing the closure. Those too have *different* Funcs,
and the Func.Closure field in one points to the other and vice versa.
This has led to no end of confusion over the years.
This CL elevates type Func to be the canonical identifier for
a given Go function.
This looks like a trivial CL but in fact is the result of a lot of
scaffolding and rewriting, discarded once the result was achieved, to
separate out the three different kinds of Func nodes into three
separate fields, limited in use to each specific Node type, to
understand which Func fields are used by which Node types and what the
possible overlaps are. There were a few overlaps, most notably around
closures, which led to more fields being added to type Func to keep
them separate even though there is now a single Func instead of two
different ones for each function.
A future CL can and should change Curfn to be a *Func instead of
a *Node, finally eliminating the confusion about whether Curfn
is an ODCLFUNC node (as it is most of the time) or an ONAME node
(as it is when type-checking an inlined function body).
Although sizeof_test.go makes it look like Func is growing by two
words, there are now half as many Funcs in a running compilation,
so the memory footprint has actually been reduced substantially.
Change-Id: I598bd96c95728093dc769a835d48f2154a406a61
Reviewed-on: https://go-review.googlesource.com/c/go/+/272253
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Since CL 255217, we've been able to rely on types.UntypedRune to
identify untyped rune literals, rather than needing Mpint.Rune /
CTRUNE. This makes way for switching to using go/constant, which
doesn't have a separate notion of rune constants distinct from integer
constants.
Passes toolstash-check.
Change-Id: I319861f4758aeea17345c101b167cb307e706a0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/272652
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
