1
0
mirror of https://github.com/golang/go synced 2024-09-23 17:10:13 -06:00

[dev.regabi] cmd/compile: change Nodes to be a slice

The Nodes type originally served two purposes:
(1) It provided a representation optimized for empty slices,
allocating only a single word in that case instead of three,
at the cost of a non-empty slice being four words instead of three.
This was particularly important with the old Node representation,
in which most Nodes were full of unused fields.
(2) It provided a few useful helper methods beyond what can be
done with slices.

The downside of Nodes is that the API is a bit overwhelming,
with many ways to spell ordinary slice operations. For example,
reassigning the first node in the list can be done with:

	ns.Slice()[0] = n
	ns.SetIndex(0, n)
	ns.SetFirst(n)
	*ns.Addr(0) = n

And APIs must decide whether to use Nodes or []ir.Node and
then conversions must be inserted when crossing the boundary.

Now that Node structs are specialized to opcode and most Nodes
lists are actually non-empty, it makes sense to simplify Nodes
to make it actually a slice type, so that ordinary slice operations can
be used, and assignments can automatically convert between
Nodes and []ir.Node.

This CL changes the representation to be a slice and adds a new
Take method, which returns the old slice and clears the receiver.

In a future CL, the Nodes method set will simplify down to:

	Copy
	Take
	Append
	Prepend
	Format

with the current methods being rewritten:

	ns.Len() -> len(ns)
	ns.Slice() -> ns
	ns.First() -> ns[0]
	ns.Second() -> ns[1]
	ns.Index(i) -> ns[i]
	ns.Addr(i) -> &ns[i]
	ns.SetIndex(i, n) -> ns[i] = n
	ns.SetFirst(n) -> ns[0] = n
	ns.SetSecond(n) -> ns[1] = n
	ns.Set1(n) -> ns = []Node{n}
	ns.Set2(n, n2) -> ns = []Node{n, n2}
	ns.Set3(n, n2, n3) -> ns = []Node{n, n2, n3}
	AsNodes(slice) -> Nodes(slice)
	ns.AppendNodes(pns) -> ns.Append(pns.Take()...)
	ns.MoveNodes(pns) -> ns = pns.Take()

and then all those other methods will be deleted.

Simplifying the API down to just those five methods will also make it
more reasonable to introduce more specialized slices like Exprs and Stmts
at some point in the future.

But again this CL just changes the representation to a slice,
introduces Take, and leaves the rest alone.

Passes buildall w/ toolstash -cmp.

Change-Id: I309ab8335c69bb582d811c92c17f938dd6e0c4fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/277916
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This commit is contained in:
Russ Cox 2020-12-11 21:29:53 -05:00
parent 4dfc7333f4
commit 114af2a044
5 changed files with 39 additions and 81 deletions

View File

@ -64,12 +64,6 @@ func Copy(n Node) Node {
return c
}
func copyList(x Nodes) Nodes {
c := make([]Node, x.Len())
copy(c, x.Slice())
return AsNodes(c)
}
// DeepCopy returns a “deep” copy of n, with its entire structure copied
// (except for shared nodes like ONAME, ONONAME, OLITERAL, and OTYPE).
// If pos.IsKnown(), it sets the source position of newly allocated Nodes to pos.

View File

@ -140,15 +140,8 @@ func (p *dumper) dump(x reflect.Value, depth int) {
return
}
// special cases
switch v := x.Interface().(type) {
case Nodes:
// unpack Nodes since reflect cannot look inside
// due to the unexported field in its struct
x = reflect.ValueOf(v.Slice())
case src.XPos:
p.printf("%s", base.FmtPos(v))
if pos, ok := x.Interface().(src.XPos); ok {
p.printf("%s", base.FmtPos(pos))
return
}

View File

@ -114,22 +114,22 @@ func (n *miniNode) SetRight(x Node) {
}
}
func (n *miniNode) SetInit(x Nodes) {
if x != (Nodes{}) {
if x != nil {
panic(n.no("SetInit"))
}
}
func (n *miniNode) SetBody(x Nodes) {
if x != (Nodes{}) {
if x != nil {
panic(n.no("SetBody"))
}
}
func (n *miniNode) SetList(x Nodes) {
if x != (Nodes{}) {
if x != nil {
panic(n.no("SetList"))
}
}
func (n *miniNode) SetRlist(x Nodes) {
if x != (Nodes{}) {
if x != nil {
panic(n.no("SetRlist"))
}
}

View File

@ -359,7 +359,7 @@ const (
// Nodes is a pointer to a slice of *Node.
// For fields that are not used in most nodes, this is used instead of
// a slice to save space.
type Nodes struct{ slice *[]Node }
type Nodes []Node
// immutableEmptyNodes is an immutable, empty Nodes list.
// The methods that would modify it panic instead.
@ -367,43 +367,37 @@ var immutableEmptyNodes = Nodes{}
// asNodes returns a slice of *Node as a Nodes value.
func AsNodes(s []Node) Nodes {
return Nodes{&s}
return s
}
// Slice returns the entries in Nodes as a slice.
// Changes to the slice entries (as in s[i] = n) will be reflected in
// the Nodes.
func (n Nodes) Slice() []Node {
if n.slice == nil {
return nil
}
return *n.slice
return n
}
// Len returns the number of entries in Nodes.
func (n Nodes) Len() int {
if n.slice == nil {
return 0
}
return len(*n.slice)
return len(n)
}
// Index returns the i'th element of Nodes.
// It panics if n does not have at least i+1 elements.
func (n Nodes) Index(i int) Node {
return (*n.slice)[i]
return n[i]
}
// First returns the first element of Nodes (same as n.Index(0)).
// It panics if n has no elements.
func (n Nodes) First() Node {
return (*n.slice)[0]
return n[0]
}
// Second returns the second element of Nodes (same as n.Index(1)).
// It panics if n has fewer than two elements.
func (n Nodes) Second() Node {
return (*n.slice)[1]
return n[1]
}
func (n *Nodes) mutate() {
@ -422,64 +416,56 @@ func (n *Nodes) Set(s []Node) {
}
n.mutate()
}
if len(s) == 0 {
n.slice = nil
} else {
// Copy s and take address of t rather than s to avoid
// allocation in the case where len(s) == 0 (which is
// over 3x more common, dynamically, for make.bash).
t := s
n.slice = &t
}
*n = s
}
// Set1 sets n to a slice containing a single node.
func (n *Nodes) Set1(n1 Node) {
n.mutate()
n.slice = &[]Node{n1}
*n = []Node{n1}
}
// Set2 sets n to a slice containing two nodes.
func (n *Nodes) Set2(n1, n2 Node) {
n.mutate()
n.slice = &[]Node{n1, n2}
*n = []Node{n1, n2}
}
// Set3 sets n to a slice containing three nodes.
func (n *Nodes) Set3(n1, n2, n3 Node) {
n.mutate()
n.slice = &[]Node{n1, n2, n3}
*n = []Node{n1, n2, n3}
}
// MoveNodes sets n to the contents of n2, then clears n2.
func (n *Nodes) MoveNodes(n2 *Nodes) {
n.mutate()
n.slice = n2.slice
n2.slice = nil
*n = *n2
*n2 = nil
}
// SetIndex sets the i'th element of Nodes to node.
// It panics if n does not have at least i+1 elements.
func (n Nodes) SetIndex(i int, node Node) {
(*n.slice)[i] = node
n[i] = node
}
// SetFirst sets the first element of Nodes to node.
// It panics if n does not have at least one elements.
func (n Nodes) SetFirst(node Node) {
(*n.slice)[0] = node
n[0] = node
}
// SetSecond sets the second element of Nodes to node.
// It panics if n does not have at least two elements.
func (n Nodes) SetSecond(node Node) {
(*n.slice)[1] = node
n[1] = node
}
// Addr returns the address of the i'th element of Nodes.
// It panics if n does not have at least i+1 elements.
func (n Nodes) Addr(i int) *Node {
return &(*n.slice)[i]
return &n[i]
}
// Append appends entries to Nodes.
@ -488,13 +474,7 @@ func (n *Nodes) Append(a ...Node) {
return
}
n.mutate()
if n.slice == nil {
s := make([]Node, len(a))
copy(s, a)
n.slice = &s
return
}
*n.slice = append(*n.slice, a...)
*n = append(*n, a...)
}
// Prepend prepends entries to Nodes.
@ -504,38 +484,29 @@ func (n *Nodes) Prepend(a ...Node) {
return
}
n.mutate()
if n.slice == nil {
n.slice = &a
} else {
*n.slice = append(a, *n.slice...)
}
*n = append(a, *n...)
}
// Take clears n, returning its former contents.
func (n *Nodes) Take() []Node {
ret := *n
*n = nil
return ret
}
// AppendNodes appends the contents of *n2 to n, then clears n2.
func (n *Nodes) AppendNodes(n2 *Nodes) {
n.mutate()
switch {
case n2.slice == nil:
case n.slice == nil:
n.slice = n2.slice
default:
*n.slice = append(*n.slice, *n2.slice...)
}
n2.slice = nil
*n = append(*n, n2.Take()...)
}
// Copy returns a copy of the content of the slice.
func (n Nodes) Copy() Nodes {
var c Nodes
if n.slice == nil {
return c
if n == nil {
return nil
}
c.slice = new([]Node)
if *n.slice == nil {
return c
}
*c.slice = make([]Node, n.Len())
copy(*c.slice, n.Slice())
c := make(Nodes, n.Len())
copy(c, n)
return c
}

View File

@ -20,8 +20,8 @@ func TestSizeof(t *testing.T) {
_32bit uintptr // size on 32bit platforms
_64bit uintptr // size on 64bit platforms
}{
{Func{}, 168, 288},
{Name{}, 124, 216},
{Func{}, 200, 352},
{Name{}, 132, 232},
}
for _, tt := range tests {