The standard library has plenty of polished encoder/decoder
implementations. No need for another ad-hoc one.
I considered using encoding/gob instead, but these strings go into the
package data part of the object file, so it's important they don't
contain "\n$$\n". Package json escapes newlines in strings, so it's
safe to use here.
Change-Id: I998655524ccee7365c2c8e9a843e6975e95a3e62
Reviewed-on: https://go-review.googlesource.com/106463
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This used to be duplicated in methcmp and siglt, because Sig used its
own representation for Syms. Instead, just use Syms, and add a
(*Sym).Less method that both methcmp and siglt can use.
Also, prune some impossible cases purportedly related to blank
methods: the Go spec disallows blank methods in interface method sets,
and addmethod drops blank methods without actually recording them in
the type's method set.
Passes toolstash-check.
Updates #24693.
Change-Id: I24e981659b68504d71518160486989a82505f513
Reviewed-on: https://go-review.googlesource.com/105936
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There were multiple ad hoc ways to create method symbols, with subtle
and confusing differences between them. This CL unifies them into a
single well-documented encoding and implementation.
This introduces some inconsequential changes to symbol format for the
sake of simplicity and consistency. Two notable changes:
1) Symbol construction is now insensitive to the package currently
being compiled. Previously, non-exported methods on anonymous types
received different method symbols depending on whether the method was
local or imported.
2) Symbols for method values parenthesized non-pointer receiver types
and non-exported method names, and also always package-qualified
non-exported method names. Now they use the same rules as normal
method symbols.
The methodSym function is also now stricter about rejecting
non-sensical method/receiver combinations. Notably, this means that
typecheckfunc needs to call addmethod to validate the method before
calling declare, which also means we no longer emit errors about
redeclaring bogus methods.
Change-Id: I9501c7a53dd70ef60e5c74603974e5ecc06e2003
Reviewed-on: https://go-review.googlesource.com/104876
Reviewed-by: Robert Griesemer <gri@golang.org>
The tool has gotten better over time, so re-generating the files brings
some advantages like fewer objects, dropping the use of fmt, and
dropping unnecessary bounds checks.
While at it, add the missing go:generate line for obj.AddrType.
Change-Id: I120c9795ee8faddf5961ff0384b9dcaf58d831ff
Reviewed-on: https://go-review.googlesource.com/100015
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
With its new -linecomment flag, it is now possible to use stringer on
values whose strings aren't valid identifiers. This is the case with
tokens and operators in Go.
Operator alredy had inline comments with each operator's string
representation; only minor modifications were needed. The inline
comments were added to each of the token names, using the same strategy.
Comments that were previously inline or part of the string arrays were
moved to the line immediately before the name they correspond to.
Finally, declare tokStrFast as a function that uses the generated arrays
directly. Avoiding the branch and strconv call means that we avoid a
performance regression in the scanner, perhaps due to the lack of
mid-stack inlining.
Performance is not affected. Measured with 'go test -run StdLib -fast'
on an X1 Carbon Gen2 (i5-4300U @ 1.90GHz, 8GB RAM, SSD), the best of 5
runs before and after the changes are:
parsed 1709399 lines (3763 files) in 1.707402159s (1001169 lines/s)
allocated 449.282Mb (263.137Mb/s)
parsed 1709329 lines (3765 files) in 1.706663154s (1001562 lines/s)
allocated 449.290Mb (263.256Mb/s)
Change-Id: Idcc4f83393fcadd6579700e3602c09496ea2625b
Reviewed-on: https://go-review.googlesource.com/95357
Reviewed-by: Robert Griesemer <gri@golang.org>
Reuse even more memory, and keep track of it in a long-lived debugState
object rather than piecemeal in the Cache.
Change-Id: Ib6936b4e8594dc6dda1f59ece753c00fd1c136ba
Reviewed-on: https://go-review.googlesource.com/92404
Reviewed-by: David Chase <drchase@google.com>
Completely redesign and reimplement location list generation to be more
efficient, and hopefully not too hard to understand.
RegKills are gone. Instead of using the regalloc's liveness
calculations, redo them using the Ops' clobber information. Besides
saving a lot of Values, this avoids adding RegKills to blocks that would
be empty otherwise, which was messing up optimizations. This does mean
that it's much harder to tell whether the generation process is buggy
(there's nothing to cross-check it with), and there may be disagreements
with GC liveness. But the performance gain is significant, and it's nice
not to be messing with earlier compiler phases.
The intermediate representations are gone. Instead of producing
ssa.BlockDebugs, then dwarf.LocationLists, and then finally real
location lists, go directly from the SSA to a (mostly) real location
list. Because the SSA analysis happens before assembly, it stores
encoded block/value IDs where PCs would normally go. It would be easier
to do the SSA analysis after assembly, but I didn't want to retain the
SSA just for that.
Generation proceeds in two phases: first, it traverses the function in
CFG order, storing the state of the block at the beginning and end. End
states are used to produce the start states of the successor blocks. In
the second phase, it traverses in program text order and produces the
location lists. The processing in the second phase is redundant, but
much cheaper than storing the intermediate representation. It might be
possible to combine the two phases somewhat to take advantage of cases
where the CFG matches the block layout, but I haven't tried.
Location lists are finalized by adding a base address selection entry,
translating each encoded block/value ID to a real PC, and adding the
terminating zero entry. This probably won't work on OSX, where dsymutil
will choke on the base address selection. I tried emitting CU-relative
relocations for each address, and it was *very* bad for performance --
it uses more memory storing all the relocations than it does for the
actual location list bytes. I think I'm going to end up synthesizing the
relocations in the linker only on OSX, but TBD.
TestNexting needs updating: with more optimizations working, the
debugger doesn't stop on the continue (line 88) any more, and the test's
duplicate suppression kicks in. Also, dx and dy live a little longer
now, but they have the correct values.
Change-Id: Ie772dfe23a4e389ca573624fac4d05401ae32307
Reviewed-on: https://go-review.googlesource.com/89356
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Change dump file names to group them alphabetically in directory
listings, in pass run order.
Change-Id: I8070578a5b4a3a7983dcc527ea1cfdb10a6d7d24
Reviewed-on: https://go-review.googlesource.com/83958
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This changes the assembly language output to use the
innermost (instead of outermost) position for line
number and file.
The file is printed separately, only when it changes,
to remove redundant and space-consuming noise from the
output.
Unknown positions have line number "?"
The output format was changed slightly to make it
easier to read.
One source of gratuitous variation in debugging output was
removed.
Change-Id: I1fd9c8b0ddd82766488582fb684dce4b04f35723
Reviewed-on: https://go-review.googlesource.com/78895
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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>
Instead of the hand-written control flow analysis in debug info
generation, use a reverse postorder traversal, which is basically the
same thing. It should be slightly faster.
More importantly, the previous version simply gave up in the case of
non-reducible functions, and produced output that caused a later stage
to crash. It turns out that there's a non-reducible function in
compress/flate, so that wasn't a theoretical issue.
With this change, all blocks will be visited, even for non-reducible
functions.
Change-Id: Id47536764ee93203c6b4105a1a3013fe3265aa12
Reviewed-on: https://go-review.googlesource.com/73110
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
In order to improve the line numbering for debuggers,
it's necessary to trace lines through compilation.
This makes it (much) easier to follow.
The format of the last column of the ssa.html output was
also changed to reduce the spamminess of the file name,
which is usually the same and makes it far harder to read
instructions and line numbers, and to make it wider and also
able to break words when wrapping (long path names still
can push off the end otherwise; side-to-side scrolling was
tried but was more annoying than the occasional wrapped
line).
Sample output now, where [...] is elision for sake of making
the CL character-counter happy -- and the (##) line numbers
are rendered in italics and a smaller font (11 point) under
control of a CSS class "line-number".
genssa
# /Users/drchase/[...]/ssa/testdata/hist.go
00000 (35) TEXT "".main(SB)
00001 (35) FUNCDATA $0, gclocals·7be4bb[...]1e8b(SB)
00002 (35) FUNCDATA $1, gclocals·9ab98a[...]4568(SB)
v920 00003 (36) LEAQ ""..autotmp_31-640(SP), DI
v858 00004 (36) XORPS X0, X0
v6 00005 (36) LEAQ -48(DI), DI
v6 00006 (36) DUFFZERO $277
v576 00007 (36) LEAQ ""..autotmp_31-640(SP), AX
v10 00008 (36) TESTB AX, (AX)
b1 00009 (36) JMP 10
and from an earlier phase:
b18: ← b17
v242 (47) = Copy <mem> v238
v243 (47) = VarKill <mem> {.autotmp_16} v242
v244 (48) = Addr <**bufio.Scanner> {scanner} v2
v245 (48) = Load <*bufio.Scanner> v244 v243
[...]
v279 (49) = Store <mem> {int64} v277 v276 v278
v280 (49) = Addr <*error> {.autotmp_18} v2
v281 (49) = Load <error> v280 v279
v282 (49) = Addr <*error> {err} v2
v283 (49) = VarDef <mem> {err} v279
v284 (49) = Store <mem> {error} v282 v281 v283
v285 (47) = VarKill <mem> {.autotmp_18} v284
v286 (47) = VarKill <mem> {.autotmp_17} v285
v287 (50) = Addr <*error> {err} v2
v288 (50) = Load <error> v287 v286
v289 (50) = NeqInter <bool> v288 v51
If v289 → b21 b22 (line 50)
Change-Id: I3f46310918f965761f59e6f03ea53067237c28a8
Reviewed-on: https://go-review.googlesource.com/69591
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The information that's used to generate DWARF location lists is very
ssa.Value centric; it uses Values as start and end coordinates to define
ranges. That mostly works fine, but control flow instructions don't come
from Values, so the ranges couldn't cover them.
Control flow instructions are generated when the SSA representation is
converted to assembly, so that's the best place to extend the ranges
to cover them. (Before that, there's nothing to refer to, and afterward
the boundaries between blocks have been lost.) That requires block
information in the debugInfo type, which then flows down to make
everything else awkward. On the plus side, there's a little less copying
slices around than there used to be, so it should be a little faster.
Previously, the ranges for empty blocks were not very meaningful. That
was fine, because they had no Values to cover, so no debug information
was generated for them. But they do have control flow instructions
(that's why they exist) and so now it's important that the information
be correct. Introduce two sentinel values, BlockStart and BlockEnd, that
denote the boundary of a block, even if the block is empty. BlockEnd
replaces the previous SurvivedBlock flag.
There's one more problem: the last instruction in the function will be a
control flow instruction, so any live ranges need to be extended past
it. But there's no instruction after it to use as the end of the range.
Instead, leave the EndProg field of those ranges as nil and fix it up to
point to past the end of the assembled text at the very last moment.
Change-Id: I81f884020ff36fd6fe8d7888fc57c99412c4245b
Reviewed-on: https://go-review.googlesource.com/63010
Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
strings.IndexByte was introduced in go1.2 and it can be used
effectively wherever the second argument to strings.Index is
exactly one byte long.
This avoids generating unnecessary string symbols and saves
a few calls to strings.Index.
Change-Id: I1ab5edb7c4ee9058084cfa57cbcc267c2597e793
Reviewed-on: https://go-review.googlesource.com/65930
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We used to have {Arg,Auto,Extern}Symbol structs with which we wrapped
a *gc.Node or *obj.LSym before storing them in the Aux field
of an ssa.Value. This let the SSA part of the compiler distinguish
between autos and args, for example. We no longer need the wrappers
as we can query the underlying objects directly.
There was also some sloppy usage, where VarDef had a *gc.Node
directly in its Aux field, whereas the use of that variable had
that *gc.Node wrapped in an AutoSymbol. Thus the Aux fields didn't
match (using ==) when they probably should.
This sloppy usage cleanup is the only thing in the CL that changes the
generated code - we can get rid of some more unused auto variables if
the matching happens reliably.
Removing this wrapper also lets us get rid of the varsyms cache
(which was used to prevent wrapping the same *gc.Node twice).
Change-Id: I0dedf8f82f84bfee413d310342b777316bd1d478
Reviewed-on: https://go-review.googlesource.com/64452
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Went mainly for the ones that make no sense, such as the ones
mid-sentence or after commas.
Change-Id: Ie245d2c19cc7428a06295635cf6a9482ade25ff0
Reviewed-on: https://go-review.googlesource.com/57293
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Just to get rid of lots of .Name() stutter in printf calls.
Change-Id: I86cf00b3f7b2172387a1c6a7f189c1897fab6300
Reviewed-on: https://go-review.googlesource.com/56630
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Debuggers use DWARF information to find local variables on the
stack and in registers. Prior to this CL, the DWARF information for
functions claimed that all variables were on the stack at all times.
That's incorrect when optimizations are enabled, and results in
debuggers showing data that is out of date or complete gibberish.
After this CL, the compiler is capable of representing variable
locations more accurately, and attempts to do so. Due to limitations of
the SSA backend, it's not possible to be completely correct.
There are a number of problems in the current design. One of the easier
to understand is that variable names currently must be attached to an
SSA value, but not all assignments in the source code actually result
in machine code. For example:
type myint int
var a int
b := myint(int)
and
b := (*uint64)(unsafe.Pointer(a))
don't generate machine code because the underlying representation is the
same, so the correct value of b will not be set when the user would
expect.
Generating the more precise debug information is behind a flag,
dwarflocationlists. Because of the issues described above, setting the
flag may not make the debugging experience much better, and may actually
make it worse in cases where the variable actually is on the stack and
the more complicated analysis doesn't realize it.
A number of changes are included:
- Add a new pseudo-instruction, RegKill, which indicates that the value
in the register has been clobbered.
- Adjust regalloc to emit RegKills in the right places. Significantly,
this means that phis are mixed with StoreReg and RegKills after
regalloc.
- Track variable decomposition in ssa.LocalSlots.
- After the SSA backend is done, analyze the result and build location
lists for each LocalSlot.
- After assembly is done, update the location lists with the assembled
PC offsets, recompose variables, and build DWARF location lists. Emit the
list as a new linker symbol, one per function.
- In the linker, aggregate the location lists into a .debug_loc section.
TODO:
- currently disabled for non-X86/AMD64 because there are no data tables.
go build -toolexec 'toolstash -cmp' -a std succeeds.
With -dwarflocationlists false:
before: f02812195637909ff675782c0b46836a8ff01976
after: 06f61e8112a42ac34fb80e0c818b3cdb84a5e7ec
benchstat -geomean /tmp/220352263 /tmp/621364410
completed 15 of 15, estimated time remaining 0s (eta 3:52PM)
name old time/op new time/op delta
Template 199ms ± 3% 198ms ± 2% ~ (p=0.400 n=15+14)
Unicode 96.6ms ± 5% 96.4ms ± 5% ~ (p=0.838 n=15+15)
GoTypes 653ms ± 2% 647ms ± 2% ~ (p=0.102 n=15+14)
Flate 133ms ± 6% 129ms ± 3% -2.62% (p=0.041 n=15+15)
GoParser 164ms ± 5% 159ms ± 3% -3.05% (p=0.000 n=15+15)
Reflect 428ms ± 4% 422ms ± 3% ~ (p=0.156 n=15+13)
Tar 123ms ±10% 124ms ± 8% ~ (p=0.461 n=15+15)
XML 228ms ± 3% 224ms ± 3% -1.57% (p=0.045 n=15+15)
[Geo mean] 206ms 377ms +82.86%
name old user-time/op new user-time/op delta
Template 292ms ±10% 301ms ±12% ~ (p=0.189 n=15+15)
Unicode 166ms ±37% 158ms ±14% ~ (p=0.418 n=15+14)
GoTypes 962ms ± 6% 963ms ± 7% ~ (p=0.976 n=15+15)
Flate 207ms ±19% 200ms ±14% ~ (p=0.345 n=14+15)
GoParser 246ms ±22% 240ms ±15% ~ (p=0.587 n=15+15)
Reflect 611ms ±13% 587ms ±14% ~ (p=0.085 n=15+13)
Tar 211ms ±12% 217ms ±14% ~ (p=0.355 n=14+15)
XML 335ms ±15% 320ms ±18% ~ (p=0.169 n=15+15)
[Geo mean] 317ms 583ms +83.72%
name old alloc/op new alloc/op delta
Template 40.2MB ± 0% 40.2MB ± 0% -0.15% (p=0.000 n=14+15)
Unicode 29.2MB ± 0% 29.3MB ± 0% ~ (p=0.624 n=15+15)
GoTypes 114MB ± 0% 114MB ± 0% -0.15% (p=0.000 n=15+14)
Flate 25.7MB ± 0% 25.6MB ± 0% -0.18% (p=0.000 n=13+15)
GoParser 32.2MB ± 0% 32.2MB ± 0% -0.14% (p=0.003 n=15+15)
Reflect 77.8MB ± 0% 77.9MB ± 0% ~ (p=0.061 n=15+15)
Tar 27.1MB ± 0% 27.0MB ± 0% -0.11% (p=0.029 n=15+15)
XML 42.7MB ± 0% 42.5MB ± 0% -0.29% (p=0.000 n=15+15)
[Geo mean] 42.1MB 75.0MB +78.05%
name old allocs/op new allocs/op delta
Template 402k ± 1% 398k ± 0% -0.91% (p=0.000 n=15+15)
Unicode 344k ± 1% 344k ± 0% ~ (p=0.715 n=15+14)
GoTypes 1.18M ± 0% 1.17M ± 0% -0.91% (p=0.000 n=15+14)
Flate 243k ± 0% 240k ± 1% -1.05% (p=0.000 n=13+15)
GoParser 327k ± 1% 324k ± 1% -0.96% (p=0.000 n=15+15)
Reflect 984k ± 1% 982k ± 0% ~ (p=0.050 n=15+15)
Tar 261k ± 1% 259k ± 1% -0.77% (p=0.000 n=15+15)
XML 411k ± 0% 404k ± 1% -1.55% (p=0.000 n=15+15)
[Geo mean] 439k 755k +72.01%
name old text-bytes new text-bytes delta
HelloSize 694kB ± 0% 694kB ± 0% -0.00% (p=0.000 n=15+15)
name old data-bytes new data-bytes delta
HelloSize 5.55kB ± 0% 5.55kB ± 0% ~ (all equal)
name old bss-bytes new bss-bytes delta
HelloSize 133kB ± 0% 133kB ± 0% ~ (all equal)
name old exe-bytes new exe-bytes delta
HelloSize 1.04MB ± 0% 1.04MB ± 0% ~ (all equal)
Change-Id: I991fc553ef175db46bb23b2128317bbd48de70d8
Reviewed-on: https://go-review.googlesource.com/41770
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
When package ssa was created, Type was in package gc.
To avoid circular dependencies, we used an interface (ssa.Type)
to represent type information in SSA.
In the Go 1.9 cycle, gri extricated the Type type from package gc.
As a result, we can now use it in package ssa.
Now, instead of package types depending on package ssa,
it is the other way.
This is a more sensible dependency tree,
and helps compiler performance a bit.
Though this is a big CL, most of the changes are
mechanical and uninteresting.
Interesting bits:
* Add new singleton globals to package types for the special
SSA types Memory, Void, Invalid, Flags, and Int128.
* Add two new Types, TSSA for the special types,
and TTUPLE, for SSA tuple types.
ssa.MakeTuple is now types.NewTuple.
* Move type comparison result constants CMPlt, CMPeq, and CMPgt
to package types.
* We had picked the name "types" in our rules for the handy
list of types provided by ssa.Config. That conflicted with
the types package name, so change it to "typ".
* Update the type comparison routine to handle tuples and special
types inline.
* Teach gc/fmt.go how to print special types.
* We can now eliminate ElemTypes in favor of just Elem,
and probably also some other duplicated Type methods
designed to return ssa.Type instead of *types.Type.
* The ssa tests were using their own dummy types,
and they were not particularly careful about types in general.
Of necessity, this CL switches them to use *types.Type;
it does not make them more type-accurate.
Unfortunately, using types.Type means initializing a bit
of the types universe.
This is prime for refactoring and improvement.
This shrinks ssa.Value; it now fits in a smaller size class
on 64 bit systems. This doesn't have a giant impact,
though, since most Values are preallocated in a chunk.
name old alloc/op new alloc/op delta
Template 37.9MB ± 0% 37.7MB ± 0% -0.57% (p=0.000 n=10+8)
Unicode 28.9MB ± 0% 28.7MB ± 0% -0.52% (p=0.000 n=10+10)
GoTypes 110MB ± 0% 109MB ± 0% -0.88% (p=0.000 n=10+10)
Flate 24.7MB ± 0% 24.6MB ± 0% -0.66% (p=0.000 n=10+10)
GoParser 31.1MB ± 0% 30.9MB ± 0% -0.61% (p=0.000 n=10+9)
Reflect 73.9MB ± 0% 73.4MB ± 0% -0.62% (p=0.000 n=10+8)
Tar 25.8MB ± 0% 25.6MB ± 0% -0.77% (p=0.000 n=9+10)
XML 41.2MB ± 0% 40.9MB ± 0% -0.80% (p=0.000 n=10+10)
[Geo mean] 40.5MB 40.3MB -0.68%
name old allocs/op new allocs/op delta
Template 385k ± 0% 386k ± 0% ~ (p=0.356 n=10+9)
Unicode 343k ± 1% 344k ± 0% ~ (p=0.481 n=10+10)
GoTypes 1.16M ± 0% 1.16M ± 0% -0.16% (p=0.004 n=10+10)
Flate 238k ± 1% 238k ± 1% ~ (p=0.853 n=10+10)
GoParser 320k ± 0% 320k ± 0% ~ (p=0.720 n=10+9)
Reflect 957k ± 0% 957k ± 0% ~ (p=0.460 n=10+8)
Tar 252k ± 0% 252k ± 0% ~ (p=0.133 n=9+10)
XML 400k ± 0% 400k ± 0% ~ (p=0.796 n=10+10)
[Geo mean] 428k 428k -0.01%
Removing all the interface calls helps non-trivially with CPU, though.
name old time/op new time/op delta
Template 178ms ± 4% 173ms ± 3% -2.90% (p=0.000 n=94+96)
Unicode 85.0ms ± 4% 83.9ms ± 4% -1.23% (p=0.000 n=96+96)
GoTypes 543ms ± 3% 528ms ± 3% -2.73% (p=0.000 n=98+96)
Flate 116ms ± 3% 113ms ± 4% -2.34% (p=0.000 n=96+99)
GoParser 144ms ± 3% 140ms ± 4% -2.80% (p=0.000 n=99+97)
Reflect 344ms ± 3% 334ms ± 4% -3.02% (p=0.000 n=100+99)
Tar 106ms ± 5% 103ms ± 4% -3.30% (p=0.000 n=98+94)
XML 198ms ± 5% 192ms ± 4% -2.88% (p=0.000 n=92+95)
[Geo mean] 178ms 173ms -2.65%
name old user-time/op new user-time/op delta
Template 229ms ± 5% 224ms ± 5% -2.36% (p=0.000 n=95+99)
Unicode 107ms ± 6% 106ms ± 5% -1.13% (p=0.001 n=93+95)
GoTypes 696ms ± 4% 679ms ± 4% -2.45% (p=0.000 n=97+99)
Flate 137ms ± 4% 134ms ± 5% -2.66% (p=0.000 n=99+96)
GoParser 176ms ± 5% 172ms ± 8% -2.27% (p=0.000 n=98+100)
Reflect 430ms ± 6% 411ms ± 5% -4.46% (p=0.000 n=100+92)
Tar 128ms ±13% 123ms ±13% -4.21% (p=0.000 n=100+100)
XML 239ms ± 6% 233ms ± 6% -2.50% (p=0.000 n=95+97)
[Geo mean] 220ms 213ms -2.76%
Change-Id: I15c7d6268347f8358e75066dfdbd77db24e8d0c1
Reviewed-on: https://go-review.googlesource.com/42145
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Currently one needs to refer to the sources to have a list of accepted
debug keys. We can copy what 'ssa/help' does and introspect the list of
debug keys to print a more detailed help:
$ go tool compile -d help
usage: -d arg[,arg]* and arg is <key>[=<value>]
<key> is one of:
append print information about append compilation
closure print information about closure compilation
disablenil disable nil checks
dclstack run internal dclstack check
gcprog print dump of GC programs
nil print information about nil checks
panic do not hide any compiler panic
slice print information about slice compilation
typeassert print information about type assertion inlining
wb print information about write barriers
export print export data
pctab print named pc-value table
ssa/help print help about SSA debugging
<value> is key-specific.
Key "pctab" supports values:
"pctospadj", "pctofile", "pctoline", "pctoinline", "pctopcdata"
For '-d help' to be discoverable, a hint is given in the -d flag
description.
A last thing, today at least one go file needs to be provided to get to
the code printing ssa/help.
$ go tool compile -d ssa/help foo.go
Add a check so one can just do '-d help' or '-d ssa/help'
Caught by trybot: I needed to update fmt_test.go as I'm introducing the
usage of %-*s in a format string.
Fixes#20041
Change-Id: Ib2858b038c1bcbe644aa3b1a371009710c6d957d
Reviewed-on: https://go-review.googlesource.com/41091
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
goobj.importPathToPrefix is 3x faster than gc.pathToPrefix so rename and
move it to cmd/internal/objabi which is already imported by both goobj and
gc.
Change-Id: I10eda5bce95ef6d5d888818c5c47258c2833ea45
Reviewed-on: https://go-review.googlesource.com/40875
Run-TryBot: Todd Neal <todd@tneal.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Replace derecursed postorder computation with one that
mimics DFS traversal.
Corrected outerinner function in loopfinder
Leave enhanced checks in place.
Change-Id: I657ba5e89c88941028d6d4c72e9f9056e30f1ce8
Reviewed-on: https://go-review.googlesource.com/40872
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
The compiler handled gcargs and gclocals LSyms unusually.
It generated placeholder symbols (makefuncdatasym),
filled them in, and then renamed them for content-addressability.
This is an important binary size optimization;
the same locals information occurs over and over.
This CL continues to treat these LSyms unusually,
but in a slightly more explicit way,
and importantly for concurrent compilation,
in a way that does not require concurrent
modification of Ctxt.Hash.
Instead of creating gcargs and gclocals in the usual way,
by creating a types.Sym and then an obj.LSym,
we add them directly to obj.FuncInfo,
initialize them in obj.InitTextSym,
and deduplicate and add them to ctxt.Data at the end.
Then the backend's job is simply to fill them in
and rename them appropriately.
Updates #15756
name old alloc/op new alloc/op delta
Template 38.8MB ± 0% 38.7MB ± 0% -0.22% (p=0.016 n=5+5)
Unicode 29.8MB ± 0% 29.8MB ± 0% ~ (p=0.690 n=5+5)
GoTypes 113MB ± 0% 113MB ± 0% -0.24% (p=0.008 n=5+5)
SSA 1.25GB ± 0% 1.24GB ± 0% -0.39% (p=0.008 n=5+5)
Flate 25.3MB ± 0% 25.2MB ± 0% -0.43% (p=0.008 n=5+5)
GoParser 31.7MB ± 0% 31.7MB ± 0% -0.22% (p=0.008 n=5+5)
Reflect 78.2MB ± 0% 77.6MB ± 0% -0.80% (p=0.008 n=5+5)
Tar 26.6MB ± 0% 26.3MB ± 0% -0.85% (p=0.008 n=5+5)
XML 42.4MB ± 0% 41.9MB ± 0% -1.04% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
Template 378k ± 0% 377k ± 1% ~ (p=0.151 n=5+5)
Unicode 321k ± 1% 321k ± 0% ~ (p=0.841 n=5+5)
GoTypes 1.14M ± 0% 1.14M ± 0% -0.47% (p=0.016 n=5+5)
SSA 9.71M ± 0% 9.67M ± 0% -0.33% (p=0.008 n=5+5)
Flate 233k ± 1% 232k ± 1% ~ (p=0.151 n=5+5)
GoParser 316k ± 0% 315k ± 0% -0.49% (p=0.016 n=5+5)
Reflect 979k ± 0% 972k ± 0% -0.75% (p=0.008 n=5+5)
Tar 250k ± 0% 247k ± 1% -0.92% (p=0.008 n=5+5)
XML 392k ± 1% 389k ± 0% -0.67% (p=0.008 n=5+5)
Change-Id: Idc36186ca9d2f8214b5f7720bbc27b6bb22fdc48
Reviewed-on: https://go-review.googlesource.com/40697
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>
This is a straightforward refactoring,
to reduce the scope of upcoming changes.
The symbol size and AttrLocal=true was not
set universally, but it appears not to matter,
since toolstash -cmp is happy.
Passes toolstash-check -all.
Change-Id: I7f8392f939592d3a1bc6f61dec992f5661f42fca
Reviewed-on: https://go-review.googlesource.com/39791
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
- parser creates sensible nodes in case of syntax errors instead of nil
- a new BadExpr node is used in places where we can't do better
- fixed error message for incorrect type switch guard
- minor cleanups
Fixes#19663.
Change-Id: I028394c6db9cba7371f0e417ebf93f594659786a
Reviewed-on: https://go-review.googlesource.com/38653
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Remove size AuxInt in Store, and alignment in Move/Zero. We still
pass size AuxInt to Move/Zero, as it is used for partial Move/Zero
lowering (e.g. cmd/compile/internal/ssa/gen/386.rules:288).
SizeAndAlign is gone.
Passes "toolstash -cmp" on std.
Change-Id: I1ca34652b65dd30de886940e789fcf41d521475d
Reviewed-on: https://go-review.googlesource.com/38150
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The fmtmode and fmtpkgpfx globals stand in the
way of making the compiler more concurrent (#15756).
This CL removes them.
The natural way to eliminate a global is to explicitly
thread it as a parameter through all function calls.
However, most of the functions in gc/fmt.go
get called indirectly, by way of fmt format strings,
so there's nowhere natural to add a parameter.
Since there are only a few fmtmode modes,
use named types to distinguish between modes.
For example, fmtNodeErr, fmtNodeDbg, and fmtNodeTypeId
are all gc.Node, but they print in different modes.
Varying the type allows us to thread mode through fmt.
Handle fmtpkgpfx by converting it to a printing mode,
FTypeIdName, and using the same type-based approach.
To avoid a loss of readability and danger of bugs
from introducing conversions at all call sites,
instead add a helper that systematically modifies the args.
The only remaining gc/fmt.go global is dumpdepth.
Since that is used for debugging only,
it that can be handled with a global mutex,
or some similarly basic, if inefficient, protection.
Passes toolstash -cmp. No compiler performance impact.
For future reference, other options for threading state
that were considered and rejected:
* Wrapping values in structs, such as:
type fmtNode struct {
n *Node
mode fmtMode
}
This reduces the proliferation of types, and supports
easily adding extra local parameters.
However, putting such a struct into an interface{} allocates.
This is unacceptable in this particular area of code.
* Passing state via precision, such as:
fmt.Fprintf("%*v", mode, n)
where mode is the state encoded as an integer.
This avoids extra allocations, but it is out of keeping
with the intended semantics of precision, and is less readable.
* Modify the fmt package to support setting/getting context
via fmt.State. Unavailable due to Go 1 compatibility,
and probably the wrong solution anyway.
* Give up on package fmt. This would be a huge readability
regression and cause high code churn.
* Attempt a de-novo rewrite that circumvents these problems.
Too high a risk of bugs, with insufficient reward for the effort,
particularly since long term plans call for elimination
of gc.Node.
Change-Id: Iea2440d5a34a938e64273707de27e3a897cb41d1
Reviewed-on: https://go-review.googlesource.com/38147
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Currently the conversion from constant divides to multiplies is mostly
done during the walk pass. This is suboptimal because SSA can
determine that the value being divided by is constant more often
(e.g. after inlining).
Change-Id: If1a9b993edd71be37396b9167f77da271966f85f
Reviewed-on: https://go-review.googlesource.com/37015
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
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>
- use syntax.Pos in syntax.Error (rather than line, col)
- use syntax.Pos in syntax.PragmaHandler (rather than just line)
- update uses
- better documentation in various places
Also:
- make Pos methods use Pos receiver (rather than *Pos)
Reviewed in and cherry-picked from https://go-review.googlesource.com/#/c/33891/.
With minor adjustments to noder.go to make merge compile.
Change-Id: I5507cea6c2be46a7677087c1aeb69382d31033eb
Reviewed-on: https://go-review.googlesource.com/34236
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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>
This is a step toward chosing a different position representation.
By introducing an explicit type, it will be easier to make the
transition step-wise while ensuring everything keeps running.
This has been reviewed via https://go-review.googlesource.com/#/c/34025/.
Change-Id: Ibceddcd62d8f346321ac3250e3940e9c436ed684
Reviewed-on: https://go-review.googlesource.com/34132
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Lazar <lazard@golang.org>
This allows use of newer math/big (and later debug/pe)
without maintaining a vendored copy somewhere in cmd.
Use for math/big, deleting cmd/compile/internal/big.
Change-Id: I2bffa7a9ef115015be29fafdb02acc3e7a665d11
Reviewed-on: https://go-review.googlesource.com/31010
Reviewed-by: Minux Ma <minux@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
