Node.Used was written to from the backend
concurrently with reads of Node.Class
for the same ONAME Nodes.
I do not know why it was not failing consistently
under the race detector, but it is a race.
This is likely also a problem with Node.HasVal and Node.HasOpt.
They will be handled in a separate CL.
Fix Used by moving it to gc.Name and making it a separate bool.
There was one non-Name use of Used, marking OLABELs as used.
That is no longer needed, now that goto and label checking
happens early in the front end.
Leave the getters and setters in place,
to ease changing the representation in the future
(or changing to an interface!).
Updates #20144
Change-Id: I9bec7c6d33dcb129a4cfa9d338462ea33087f9f7
Reviewed-on: https://go-review.googlesource.com/42015
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
- 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>
Even very large Types are not very big.
The haspointer cache looks like premature optimization.
Removing them has no detectable compiler performance impact,
and it removes mutable shared state used by the backend.
Updates #15756
Change-Id: I2d2cf03f470f5eef5bcd50ff693ef6a01d481700
Reviewed-on: https://go-review.googlesource.com/38912
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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>
That was easy.
Fixes#14473.
Change-Id: I9d1d20a5c5a9b1423e6c72c0460ee4a78130864f
Reviewed-on: https://go-review.googlesource.com/20521
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The listsort function is no longer used, except in a test. Change the
test to use sort.Sort instead.
Change-Id: Ib634705cc1bc3b1d8fc3795bd4ed2894e6abc284
Reviewed-on: https://go-review.googlesource.com/19964
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Previously, given two Nodes n1 and n2 of different non-PAUTO classes
(e.g., PPARAM and PPARAMOUT), cmpstackvarlt(n1, n2) and
cmpstackvarlt(n2, n1) both returned true, which is nonsense.
This doesn't seem to cause any visible miscompilation problems, but
notably fixing it does cause toolstash/buildall to fail.
Change-Id: I33b2c66e902c5eced875d8fbf18b7cfdc81e8aed
Reviewed-on: https://go-review.googlesource.com/19778
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
A followup CL will rewrite listsort to use the new cmpstackvarlt and
change cmpstackvar to avoid stringsCompare.
Change-Id: Idf0857a3bd67f9e2243ba82aa0bff510612927c3
Reviewed-on: https://go-review.googlesource.com/14611
Reviewed-by: Dave Cheney <dave@cheney.net>
