While we're at it, add tests for EncodedLen and DecodedLen.
Fixes#14803.
Change-Id: I200c72cf11c51669b8d9f70c6e57ece359f7ae61
Reviewed-on: https://go-review.googlesource.com/20649
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
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>
On my Mac I am in group 5000 which apparently has no name
(I suspect because it is an LDAP group and I cannot reach the
LDAP server). Do not make the test fail in that case.
Fixes#14806
Change-Id: I56b11a8e86b048abfb00812eaad37802fd2adcc5
Reviewed-on: https://go-review.googlesource.com/20710
Reviewed-by: Brad Fitzpatrick <bradfitz@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>
CL 14603 attempted to preserve the callee-save registers for
the darwin/arm runtime initialization routine, but I believe it
wasn't sufficient and resulted in the crash reported in issue
Saving and restoring the registers on the stack the same way
linux/arm does seems more obvious and fixes#14778, so do that.
Even though #14778 is not reproducible on darwin/arm64, I applied
a similar change there, and to linux/arm64 which obeys the same
calling convention.
Finally, this CL is a candidate for a 1.6 minor release for the same
reason CL 14603 was in a 1.5 minor release (as CL 16968). It is
small and only touches the iOS platforms and gomobile on darwin/arm
is currently useless without it.
Fixes#14778Fixes#12590 (again)
Change-Id: I7401daf0bbd7c579a7e84761384a7b763651752a
Reviewed-on: https://go-review.googlesource.com/20621
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: Elias Naur <elias.naur@gmail.com>
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>
Treat the verb %X in the same special way as %q, %s and %x
are for arrays and slices with byte type elements.
Modify input for tests so the result of %x and %X is distinct.
Change-Id: I38d227755e98c7fad5e4adc2f603c6873aa910fd
Reviewed-on: https://go-review.googlesource.com/20516
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@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>
In particular, write down the rules for stack ownership because the
details of this are about to get very important with concurrent stack
shrinking. (Interestingly, the details don't actually change, but
anything that's currently skating on thin ice is likely to fall
through.)
Fox #12967.
Change-Id: I561e2610e864295e9faba07717a934aabefcaab9
Reviewed-on: https://go-review.googlesource.com/20034
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently copystack adjusts pointers in the old stack and then copies
the adjusted stack to the new stack. In addition to being generally
confusing, this is going to make concurrent stack shrinking harder.
Switch this around so that we first copy the stack and then adjust
pointers on the new stack (never writing to the old stack).
This reprises CL 15996, but takes a different and simpler approach. CL
15996 still walked the old stack while adjusting pointers on the new
stack. In this CL, we adjust auxiliary structures before walking the
stack, so we can just walk the new stack.
For #12967.
Change-Id: I94fa86f823ba9ee478e73b2ba509eed3361c43df
Reviewed-on: https://go-review.googlesource.com/20033
Reviewed-by: Rick Hudson <rlh@golang.org>
In particular, document that *sel is on the stack no matter what.
Change-Id: I1c264215e026c27721b13eedae73ec845066cdec
Reviewed-on: https://go-review.googlesource.com/20032
Reviewed-by: Rick Hudson <rlh@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>
And fix the wrong comment.
Initially found this because the comment was wrong about the possible
values. Then noticed that there doesn't seem to be any reason to use
uintptr over SelectDir.
Change-Id: I4f9f9640e49d89e558ed00bd99e57dab890785f5
Reviewed-on: https://go-review.googlesource.com/20655
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Only compute the number of maximum allowed elements per slice once.
Special case newcap computation for slices with byte sized elements.
name old time/op new time/op delta
GrowSliceBytes-2 61.1ns ± 1% 43.4ns ± 1% -29.00% (p=0.000 n=20+20)
GrowSliceInts-2 85.9ns ± 1% 75.7ns ± 1% -11.80% (p=0.000 n=20+20)
Change-Id: I5d9c0d5987cdd108ac29dc32e31912dcefa2324d
Reviewed-on: https://go-review.googlesource.com/20653
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It was failing like "unknown groupid ᎈ|" instead of "unknown groupid
5000" due to the conversion from int to string.
Updates #14806
Change-Id: I83e4b478ff628ad4053573a9f32b3fadce22e847
Reviewed-on: https://go-review.googlesource.com/20642
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: 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>
