For a long time varexpr has handled ODOT incorrectly: it has always
returned false. Before https://golang.org/cl/20890 this has been
because an ODOT had a Right field with an ONAME with no Class, for which
varexpr returns false. CL 20890 preserved the behavior of varexpr for
ODOT, so that the change would pass toolstash -cmp.
This CL fixes varexpr so that ODOT can return true in some cases. This
breaks toolstash -cmp. While the changed compiler allocates temporary
variables in a different order, I have not been able to find any
examples where the generated code is different, other than using
different stack offsets and, in some cases, registers. It seems that
other parts of the compiler will force the ODOT into a temporary anyhow.
Still, this change is clearly correct, and is a minor compiler cleanup.
Change-Id: I71506877aa3c13966bb03c281aa16271ee7fe80a
Reviewed-on: https://go-review.googlesource.com/20907
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Historically ODOT and friends have been considered to cost an extra
budget point when deciding whether they should be inlined, because they
had an ONAME node that represented the name to the right of the dot.
This doesn't really make sense, as in general that symbol does not add
any extra instructions; it just affects the offset of the load or store
instruction. And the ONAME node is gone now. So, remove the extra
cost.
This does not pass toolstash -cmp, as it changes inlining decisions.
For example, mspan.init in runtime/mheap.go is now considered to be an
inlining candidate.
Change-Id: I5ad27f08c66fd5daa4c8472dd0795df989183f5e
Reviewed-on: https://go-review.googlesource.com/20891
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reduces size of archives in pkg/linux_amd64 by 3% from 41.5MB to 40.2MB
Change-Id: Id64ca7995de8dd84c9e7ce1985730927cf4bfd66
Reviewed-on: https://go-review.googlesource.com/20912
Reviewed-by: David Crawshaw <crawshaw@golang.org>
In golang.org/cl/20602, I changed the semantics of Eqtype to stop
checking the receiver parameters for type equality, and pushed this
responsibility to addmethod (the only Eqtype caller that cared).
However, I accidentally made the check stricter by making it start
requiring that receiver names were identical.
In general, this is a non-problem because the receiver names in export
data will always match the original source. But running
GO_GCFLAGS=-newexport ./all.bash at one point tries to load both old
and new format export data for package sync, which reveals the
problem. (See golang.org/issue/14877 for details.)
Easy fix: just check the receiver type for type equality in addmethod,
instead of the entire receiver parameter list.
Fixes#14877.
Change-Id: If10b79f66ba58a1b7774622b4fbad1916aba32f1
Reviewed-on: https://go-review.googlesource.com/20906
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Most 64-bit x86 ops can only take a signed 32-bit constant.
Clean up our rewrite rules to enforce this restriction.
Modify the assembler to fail if the offset does not fit
in the instruction.
That last check triggers a few times on weird testing code.
Suppress those errors if the compiler itself generated errors.
Fixes#14862
Change-Id: I76559af035b38483b1e59621a8029fc66b3a5d1e
Reviewed-on: https://go-review.googlesource.com/20815
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The Node type ODOT and its variants all represent a selector, with a
simple name to the right of the dot. Before this change this was
represented by using an ONAME Node in the Right field. This ONAME node
served no useful purpose. This CL changes these Node types to store the
symbol in the Sym field instead, thus not requiring allocating a Node
for each selector.
When compiling x/tools/go/types this CL eliminates nearly 5000 calls to
newname and reduces the total number of Nodes allocated by about 6.6%.
It seems to cut compilation time by 1 to 2 percent.
Getting this right was somewhat subtle, and I added two dubious changes
to produce the exact same output as before. One is to ishairy in
inl.go: the ONAME node increased the cost of ODOT and friends by 1, and
I retained that, although really ODOT is not more expensive than any
other node. The other is to varexpr in walk.go: because the ONAME in
the Right field of an ODOT has no class, varexpr would always return
false for an ODOT, although in fact for some ODOT's it seemingly ought
to return true; I added an && false for now. I will send separate CLs,
that will break toolstash -cmp, to clean these up.
This CL passes toolstash -cmp.
Change-Id: I4af8a10cc59078c436130ce472f25abc3a9b2f80
Reviewed-on: https://go-review.googlesource.com/20890
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Consider functions with an ODCLCONST for inlining and modify exprfmt to
ignore those nodes when exporting. Don't add symbols to the export list
if there is no definition. This occurs when OLITERAL symbols are looked
up via Pkglookup for non-exported symbols.
Fixes#7655
Change-Id: I1de827850f4c69e58107447314fe7433e378e069
Reviewed-on: https://go-review.googlesource.com/20773
Run-TryBot: Todd Neal <todd@tneal.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This ought to revert the bad effects of
https://go-review.googlesource.com/#/c/20775/
If you don't pass BOOT_GO_GCFLAGS, you get the
old behavior.
Tweaked to allow multiple space-separated flags in
BOOT_GO_GCFLAGS.
Change-Id: I2a22a04211b4535d1c5a8ec7a8a78cb051161c31
Reviewed-on: https://go-review.googlesource.com/20871
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Phi splitting sometimes leads to a phi with only a single predecessor.
This must be replaced with a copy to maintain a valid SSA form.
Fixes#14857
Change-Id: I5ab2423fb6c85a061928e3206b02185ea8c79cd7
Reviewed-on: https://go-review.googlesource.com/20826
Reviewed-by: Keith Randall <khr@golang.org>
This seems to help the problem reported in #14606; this
change seems to produce about a 4% improvement (mostly
for the 128-8192 shards).
Fixes#14789.
Change-Id: I1bd52c82d4ca81d9d5e9ab371fdfc860d7e8af50
Reviewed-on: https://go-review.googlesource.com/20660
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Improve the test by also translating " " to "_".
Fixes#14671.
Change-Id: Ie5997934b93c7663d7b8432244fad47bb5d3ffbe
Reviewed-on: https://go-review.googlesource.com/20714
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is intended to help debug compiler problems that pop
up in the bootstrap phase of make.bash. GO_GCFLAGS does not
normally apply there. Options-for-all phases is intended
to allow crude tracing (and full timing) by turning on timing
for all phases, not just one.
Phase names can also be specified using a regular expression,
for example
BOOT_GO_GCFLAGS=-d='ssa/~^.*scc$/off' \
GO_GCFLAGS='-d=ssa/~^.*scc$/off' ./make.bash
I just added this because it was the fastest way to get
me to a place where I could easily debug the compiler.
Change-Id: I0781f3e7c19651ae7452fa25c2d54c9a245ef62d
Reviewed-on: https://go-review.googlesource.com/20775
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The critical phase did not correctly maintain the use count
when two predecessors of a new critical block transmit the
same value.
Change-Id: Iba802c98ebb84e36a410721ec32c867140efb6d4
Reviewed-on: https://go-review.googlesource.com/20822
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Todd Neal <todd@tneal.org>
Make sure we don't generate write barriers in runtime
code that is marked to forbid write barriers.
Implement the optimization that if we're writing a sliced
slice back to the location it came from, we don't need a
write barrier.
Fixes#14784
Change-Id: I04b6a3b2ac303c19817e932a36a3b006de103aaa
Reviewed-on: https://go-review.googlesource.com/20791
Reviewed-by: Austin Clements <austin@google.com>
Reduces size of archives in pkg/linux_amd64 by 1.4MB (3.2%),
slightly improving link time.
name old s/op new s/op delta
LinkCmdGo 0.52 ± 3% 0.51 ± 2% -0.65% (p=0.000 n=98+99)
Change-Id: I7e265f4d4dd08967c5c5d55c1045e533466bbbec
Reviewed-on: https://go-review.googlesource.com/20802
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Boolean expressions involving t.Thistuple were converted to use
t.Recv(), because it's a bit clearer and will hopefully reveal cases
where we could remove redundant calls to t.Recv() (in followup CLs).
The other cases were all converted to use t.Recvs().NumFields(),
t.Params().NumFields(), or t.Results().NumFields().
Change-Id: I4df91762e7dc4b2ddae35995f8dd604a52c09b09
Reviewed-on: https://go-review.googlesource.com/20796
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
We never need a type hash for a method type, so skip trying to
overwrite Thistuple.
Change-Id: I8de6480ba5fd321dfa134facf7661461d298840e
Reviewed-on: https://go-review.googlesource.com/20795
Reviewed-by: Russ Cox <rsc@golang.org>
This is an automated rewrite of all the calls of the form:
for f, it := IterFields(t); f != nil; f = it.Next() { ... }
Followup CLs will work on cleaning up the remaining cases.
Change-Id: Ic1005ad45ae0b50c63e815e34e507e2d2644ba1a
Reviewed-on: https://go-review.googlesource.com/20794
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Analogous to the Nodes type used as a more space efficient []*Node
representation.
Passes toolstash -cmp.
Change-Id: I8341e45304777d6e4200bd36dadc935b07ccf3ff
Reviewed-on: https://go-review.googlesource.com/20793
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
For every string constant the compiler was creating 2 Sym's and 2
Node's. It would never refer to them again, but would keep them alive
in gostringpkg. This changes the code to just use obj.LSym's instead.
When compiling x/tools/go/types, this yields about a 15% reduction in
the number of calls to newname and a 3% reduction in the total number of
Node objects. Unfortunately I couldn't see any change in compile time,
but reducing memory usage is desirable anyhow.
Passes toolstash -cmp.
Change-Id: I24f1cb1e6cff0a3afba4ca66f7166874917a036b
Reviewed-on: https://go-review.googlesource.com/20792
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This should probably be considered "experimental" at this stage, but
what it needs is feedback from adventurous adopters. I think the data
structure used for describing escape reasons might be extendable to
allow a cleanup of the underlying algorithms, which suffers from
insufficiently separated concerns (the graph does not deal well with
escape level adjustments, so it is augmented by a second custom-walk
portion of the "flood" phase. It would be better to put it all,
including level adjustments, in a single graph structure, and then
simply flood the graph.
Tweaked to avoid allocations in the no-logging case.
Modified run.go to ignore lines with leading "#" in the output (since
it can never match a line), and in -update_errors to ignore leading
tabs in output lines and to normalize embedded filenames.
Currently requires -m -m because otherwise the noise/update
burden for the other escape tests is considerable.
There is a partial test. Existing escape analysis tests seem to
cover all except the panic case and what looks like it might be
unreachable code in escape analysis.
Fixes#10526.
Change-Id: I2524fdec54facae48b00b2548e25d9e46fcaf832
Reviewed-on: https://go-review.googlesource.com/18041
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
If a phi has duplicate arguments, then the new block that is constructed
to remove the critical edge can be used for all of the duplicate
arguments.
read-only data = -904 bytes (-0.058308%)
global text (code) = -2240 bytes (-0.060056%)
Total difference -3144 bytes (-0.056218%)
Change-Id: Iee3762744d6a8c9d26cdfa880bb23feb62b03c9c
Reviewed-on: https://go-review.googlesource.com/20746
Run-TryBot: Todd Neal <todd@tneal.org>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Keep track of how many uses each Value has. Each appearance in
Value.Args and in Block.Control counts once.
The number of uses of a value is generically useful to
constrain rewrite rules. For instance, we might want to
prevent merging index operations into loads if the same
index expression is used lots of times.
But I have one use in particular for which the use count is required.
We must make sure we don't combine ops with loads if the load has
more than one use. Otherwise, we may split a single load
into multiple loads and that breaks perceived behavior in
the presence of races. In particular, the load of m.state
in sync/mutex.go:Lock can't be done twice. (I have a separate
CL which triggers the mutex failure. This CL has a test which
demonstrates a similar failure.)
Change-Id: Icaafa479239f48632a069d0c3f624e6ebc6b1f0e
Reviewed-on: https://go-review.googlesource.com/20790
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Todd Neal <todd@tneal.org>
When building shared libraries, all symbols on Allsym are marked reachable.
What I didn't realize was that this includes the ".dup" symbols created when
"dupok" symbols are read from multiple package files. This breaks now because
deadcode makes some assumptions that fail for these ".dup" symbols, but in any
case was a bad idea -- I suspect this change makes libstd.so a bunch smaller,
but creating it was broken before this CL so I can't be sure.
This change simply stops adding these symbols to Allsym, which might make some
of the many iterations over Allsym the linker does a touch quicker, although
that's not the motivation here.
Add a test that no symbols called ".dup" makes it into the runtime shared
library.
Fixes#14841
Change-Id: I65dd6e88d150a770db2d01b75cfe5db5fd4f8d25
Reviewed-on: https://go-review.googlesource.com/20780
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Receiver parameters generally aren't relevant to the function
signature type. In particular:
1. When checking whether a type's method implements an interface's
method, we specifically want to ignore the receiver parameters,
because they'll be different.
2. When checking interface type equality, interface methods always
use the same "fakethis" *struct{} type as their receiver.
3. Finally, method expressions and method values degenerate into
receiver-less function types.
The only case where we care about receiver types matching is in
addmethod, which is easily handled by adding an extra Eqtype check of
the receiver parameters. Also, added a test for this, since
(surprisingly) there weren't any.
As precedence, go/types.Identical ignores receiver parameters when
comparing go/types.Signature values.
Notably, this allows us to slightly simplify the "implements"
function, which is used for checking whether type/interface t
implements interface iface. Currently, cmd/compile actually works
around Eqtype's receiver parameter checking by creating new throwaway
TFUNC Types without the receiver parameter.
(Worse, the compiler currently only provides APIs to build TFUNC Types
from Nod syntax trees, so building those throwaway types also involves
first building throwaway syntax trees.)
Passes toolstash -cmp.
Change-Id: Ib07289c66feacee284e016bc312e8c5ff674714f
Reviewed-on: https://go-review.googlesource.com/20602
Reviewed-by: Robert Griesemer <gri@golang.org>
Add a C.CBytes function to copy a Go byte slice into C memory. This
returns an unsafe.Pointer, since that is what needs to be passed to
C.free, and the data is often opaque bytes anyway.
Fixes#14838
Change-Id: Ic7bc29637eb6f1f5ee409b3898c702a59833a85a
Reviewed-on: https://go-review.googlesource.com/20762
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently we generate write barriers when the right side of an
assignment is a global function. This doesn't fall into the existing
case of storing an address of a global because we haven't lowered the
function to a pointer yet.
This write barrier is unnecessary, so eliminate it.
Fixes#13901.
Change-Id: Ibc10e00a8803db0fd75224b66ab94c3737842a79
Reviewed-on: https://go-review.googlesource.com/20772
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
c2go translated writing and advancing a pointer using slices.
Switch to something more idiomatic.
It is also more efficient, but not enough to matter.
Change-Id: I67709632ac53253615a35365824ae97bbe5458d5
Reviewed-on: https://go-review.googlesource.com/20767
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The parser code was not reachable ever since some of the lexer cleanups.
We could recognize '~' in the lexer, complain, and return a '^' instead,
but it's been a few years since Go was new and this may have been a use-
ful error. The lexer complains with "illegal character U+007E '~'" which
is good enough.
For #13244.
Change-Id: Ie3283738486eb6f8462d594f2728ac98333c0520
Reviewed-on: https://go-review.googlesource.com/20768
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Use a local variable instead.
Passes toolstash -cmp.
Change-Id: I9623a40ff0d568f11afd1279b6aaa1c33eda644c
Reviewed-on: https://go-review.googlesource.com/20730
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently the select lock order is a []*hchan. We're going to need to
refer to things other than the channel itself in lock order shortly,
so switch this to a []uint16 of indexes into the select cases. This
parallels the existing representation for the poll order.
Change-Id: I89262223fe20b4ddf5321592655ba9eac489cda1
Reviewed-on: https://go-review.googlesource.com/20036
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Given a G, there's currently no way to find the channel it's blocking
on. We'll need this information to fix a (probably theoretical) bug in
select and to implement concurrent stack shrinking, so record the
channel in the sudog.
For #12967.
Change-Id: If8fb63a140f1d07175818824d08c0ebeec2bdf66
Reviewed-on: https://go-review.googlesource.com/20035
Reviewed-by: Rick Hudson <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Step 2 of stream-lining parameter parsing
- do parameter validity checks in parser
- two passes instead of multiple (and theoretically quadratic) passes
when checking parameters
- removes the need for OKEY and some ONONAME nodes in those passes
This removes allocation of ~123K OKEY (incl. some ONONAME) nodes
out of a total of ~10M allocated nodes when running make.bash, or
a reduction of the number of alloacted nodes by ~1.2%.
Change-Id: I4a8ec578d0ee2a7b99892ac6b92e56f8e0415f03
Reviewed-on: https://go-review.googlesource.com/20748
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
They've been on for a few weeks of general use and nothing
has tripped up on them yet.
Makes the compiler ~18% faster.
Change-Id: I42d7bbc0581597f9cf4fb28989847814c81b08a2
Reviewed-on: https://go-review.googlesource.com/20741
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The obj.Fmt* values are only used by gc/fmt.go, so just move them
there. Also, add comments documenting the correspondance between
FmtFoo names and their flag characters to make understanding the
existing documentation slightly less confusing.
While here, add a new FmtFlag named type to represent these values.
Change-Id: I9631214b892557d094823f1ac575d0c43a84007b
Reviewed-on: https://go-review.googlesource.com/20717
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Change Cond implementation to use a notification list such that waiters
can first register for a notification, release the lock, then actually
wait. Signalers never have to park anymore.
This is intended to address an issue in the previous implementation
where Broadcast could fail to signal all waiters.
Results of the existing benchmark are below.
Original New Diff
BenchmarkCond1-48 2000000 745 ns/op 755 +1.3%
BenchmarkCond2-48 1000000 1545 ns/op 1532 -0.8%
BenchmarkCond4-48 300000 3833 ns/op 3896 +1.6%
BenchmarkCond8-48 200000 10049 ns/op 10257 +2.1%
BenchmarkCond16-48 100000 21123 ns/op 21236 +0.5%
BenchmarkCond32-48 30000 40393 ns/op 41097 +1.7%
Fixes#14064
Change-Id: I083466d61593a791a034df61f5305adfb8f1c7f9
Reviewed-on: https://go-review.googlesource.com/18892
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Caleb Spare <cespare@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The type information for a method includes two variants: a func
without the receiver, and a func with the receiver as the first
parameter. The former is used as part of the dynamic interface
checks, but the latter is only returned as a type in the
reflect.Method struct.
Instead of computing it at compile time, construct it at run time
with reflect.FuncOf.
Using cl/20701 as a baseline,
cmd/go: -480KB, (4.4%)
jujud: -5.6MB, (7.8%)
For #6853.
Change-Id: I1b8c73f3ab894735f53d00cb9c0b506d84d54e92
Reviewed-on: https://go-review.googlesource.com/20709
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
All of a struct's fields have to fit into memory anyway, so index them
with int instead of int64. This also makes it nicer for
cmd/compile/internal/gc to reuse the same NumFields function.
Change-Id: I210be804a0c33370ec9977414918c02c675b0fbe
Reviewed-on: https://go-review.googlesource.com/20691
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Remove method type information for pruned methods from any program
that does not reflect on methods. This can be a significant saving:
addr2line: -310KB (8.8%)
A future update might want to consider a more aggressive variant of
this: setting the Type and Func fields of reflect.Method to nil for
unexported methods. That would shrink cmd/go by 2% and jujud by 2.6%
but could be considered an API change. So this CL sticks to the
uncontroversial change.
For #6853.
Change-Id: I5d186d9f822dc118ee89dc572c4912a3b3c72577
Reviewed-on: https://go-review.googlesource.com/20701
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Make sure symbol gets carried along by load-combining rule.
Add the new load into the right block where we know that
mem is live.
Use auxInt field to carry i along instead of an explicit ADDQ.
Incorporate LEA ops into MOVBQZX and friends.
Change-Id: I587f7c6120b98fd2a0d48ddd6ddd13345d4421b4
Reviewed-on: https://go-review.googlesource.com/20732
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Todd Neal <todd@tneal.org>
Turns out there were only two call sites that expected
t.Field(t.NumFields()) to return nil.
Change-Id: I4679988d38ee9d7c9d89883537a17046717b2a77
Reviewed-on: https://go-review.googlesource.com/20731
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Symbols in the object file currently refer to each other using symbol name
and version. Referring to the same symbol many times in an object file takes
up space and causes redundant map lookups. Instead write out a list of unique
symbol references and have symbols refer to each other using indexes into this
list.
Credit to Michael Hudson-Doyle for kicking this off.
Reduces pkg/linux_amd64 size by 30% from 61MB to 43MB
name old s/op new s/op delta
LinkCmdGo 0.74 ± 3% 0.63 ± 4% -15.22% (p=0.000 n=20+20)
LinkJuju 6.38 ± 6% 5.73 ± 6% -10.16% (p=0.000 n=20+19)
Change-Id: I7e101a0c80b8e673a3ba688295e6f80ea04e1cfb
Reviewed-on: https://go-review.googlesource.com/20099
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Move the C header style architecture constants to the per arch Main
methods.
Change-Id: Ie7ff39baa275ceaa6680e7d16441ca9f0aa12597
Reviewed-on: https://go-review.googlesource.com/20722
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Partial automatic cleanup driven by Dominik Honnef's unused tool.
As _lookup now only has one caller, merge it into the caller and remove
the conditional create logic.
Change-Id: I2ea354d9d4b32a19905271eca74725231b6d8a93
Reviewed-on: https://go-review.googlesource.com/20589
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is a band-aid, but it fixes the problem
until a deeper fix is in place.
Testing with genpkg -n 50000, I see:
Before:
154.67 real 184.66 user 3.15 sys
After:
61.82 real 96.99 user 2.17 sys
Fixes#14781.
Change-Id: I24c7822d60c289bdd6a18a7840b984954c95f7d4
Reviewed-on: https://go-review.googlesource.com/20696
Reviewed-by: Robert Griesemer <gri@golang.org>
Change-Id: I928f51a1fe4830a81d4f5d3eb572785e06a75b77
Reviewed-on: https://go-review.googlesource.com/20581
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Another step towards hooking up exported inlined function bodies.
Change-Id: Ib8094b03ac7970fee0e51b5826b5f8aa232e23fb
Reviewed-on: https://go-review.googlesource.com/20605
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
Currently, the only use for this is on the Left side of OKEY nodes
within struct literals. esc and fmt only care so they can recognize
that the ONAME nodes are actually field names, which need special
handling.
sinit additionally needs to know the field's offset within the struct,
which we can provide via Xoffset.
Passes toolstash/buildall.
Change-Id: I362d965e161f4d80fcd9c9bae0dfacc657dc0b29
Reviewed-on: https://go-review.googlesource.com/20676
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Switch TSTRUCT and TINTER to use Fields instead of Type, which wrings
out the remaining few direct uses of the latter.
Preparation for converting fields to use a separate "Field" type.
Passes toolstash/buildall.
Change-Id: I5a2ea7e159d0dde1be2c9afafc10a8f739d95743
Reviewed-on: https://go-review.googlesource.com/20675
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
String symbols' names used to appear in the final binary.
Using a string's contents as it's symbol's name
was a thus a bad idea if the string's name was long.
Recent improvements by crawshaw have changed that.
Instead of placing long strings behind opaque names
in local packages, place them in the global string
package and make them content-addressable.
Symbol names still occur in the object files,
so use a hash to avoid needless length there.
Reduces the size of cmd/go by 30k.
Change-Id: Ifdbbaf47bf44352418c90ddd903d5106e48db4f1
Reviewed-on: https://go-review.googlesource.com/20524
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
decompose-builtin pass requires an opt pass, but -N disables
late-opt, the only opt pass (out of two) that happens
after decompose-builtin. This CL enables both 'opt' and 'late opt'
passes. The extra compile time for 'late opt' in negligible
since most rewrites were already done in the first 'opt'
(also measured before). We should put some effort in splitting the
generic rules into required and optional.
Also update generic.rules comments about lowering
of StringMake and SliceMake.
Tested with GO_GCFLAGS=-N ./all.bash
Change-Id: I92999681aaa02587b6dc6e32ce997a91f1fc9499
Reviewed-on: https://go-review.googlesource.com/20682
Run-TryBot: Alexandru Moșoi <alexandru@mosoi.ro>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Parts of the SSA compiler in package gc contain amd64-specific code,
most notably Prog generation. Move this code into package amd64, so that
other architectures can be added more easily.
In package gc, this change is just moving code. There are no functional
changes or even any larger structural changes beyond changing function
names (mostly for export).
In the cmd/compile/internal/ssa/gen tool, more information is included
in arch to remove the AMD64-specific behavior in the main portion of the
tool. The generated opGen.go is identical.
Change-Id: I8eb37c6e6df6de1b65fa7dab6f3bc32c29daf643
Reviewed-on: https://go-review.googlesource.com/20609
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Line numbers are always int32, so the Warnl function should take the
line number as an int32 as well. This matches gc.Warnl and removes
a cast every place it's used.
Change-Id: I5d6201e640d52ec390eb7174f8fd8c438d4efe58
Reviewed-on: https://go-review.googlesource.com/20662
Run-TryBot: Todd Neal <todd@tneal.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Just recognize "DATA" as a special pseudo op word in the assembler
directly.
Change-Id: I508e111fd71f561efa600ad69567a7089a57adb2
Reviewed-on: https://go-review.googlesource.com/20648
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
The only remaining place that generated ADATA
Prog was the assembler. Stop, and delete some
now-dead code.
Passes toolstash -cmp.
Change-Id: I26578ff1b4868e98562b44f69d909c083e96f8d5
Reviewed-on: https://go-review.googlesource.com/20646
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Instead of generating ADATA instructions for
static data, write that static data directly
into the linker sym.
This is considerably more efficient.
The assembler still generates
ADATA instructions, so the ADATA machinery
cannot be dismantled yet. (Future work.)
Skipping ADATA has a significant impact
compiling the unicode package, which has lots
of static data.
name old time/op new time/op delta
Unicode 227ms ±10% 192ms ± 4% -15.61% (p=0.000 n=29+30)
name old alloc/op new alloc/op delta
Unicode 51.0MB ± 0% 45.8MB ± 0% -10.29% (p=0.000 n=30+30)
name old allocs/op new allocs/op delta
Unicode 610k ± 0% 578k ± 0% -5.29% (p=0.000 n=30+30)
This does not pass toolstash -cmp, because
this changes the order in which some relocations
get added, and thus it changes the output from
the compiler. It is not worth the execution time
to sort the relocs in the normal case.
However, compiling with -S -v generates identical
output if (1) you suppress printing of ADATA progs
in flushplist and (2) you suppress printing of
cpu timing. It is reasonable to suppress printing
the ADATA progs, since the data itself is dumped
later. I am therefore fairly confident that all
changes are superficial and non-functional.
Fixes#14786, although there's more to do
in general.
Change-Id: I8dfabe7b423b31a30e516cfdf005b62a2e9ccd82
Reviewed-on: https://go-review.googlesource.com/20645
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
* Shaves about 10k from pkg/tools/linux_amd64.
* Was suggested by drchase before
* Found by looking at ssa output of #14758
Change-Id: If2c4ddf3b2603d4dfd8fb4d9199b9a3dcb05b17d
Reviewed-on: https://go-review.googlesource.com/20570
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Alexandru Moșoi <alexandru@mosoi.ro>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This makes the output of compiling with -S more
stable in the face of unimportant variation in the
order in which relocs are generated.
It is also more pleasant to read the relocs when
they are sorted.
Also, do some minor cleanup.
For #14786
Change-Id: Id92020b13fd21777dfb5b29c2722c3b2eb27001b
Reviewed-on: https://go-review.googlesource.com/20641
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Split the auxFloat type into 32/64 bit versions and perform checking for
exactly representable float32 values. Perform const folding on
float32/64. Comment out some const negation rules that the frontend
already performs.
Change-Id: Ib3f8d59fa8b30e50fe0267786cfb3c50a06169d2
Reviewed-on: https://go-review.googlesource.com/20568
Run-TryBot: Todd Neal <todd@tneal.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
