No semantic change, just clarifying a bit by choosing better words
in a couple of places.
Change-Id: I4496062ee7909baf83d4d22d25e13ef93b358b4b
Reviewed-on: https://go-review.googlesource.com/55255
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts commit f612cd704a.
Reason for revert: We thought the original change had broken the
linux/amd64 and linux/386 builders, but it turned out to be a problem
with the build infrastructure, not the change.
Change-Id: Ic3318a63464fcba8d845ac04494115a7ba620364
Reviewed-on: https://go-review.googlesource.com/55050
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
$ go tool -h says:
For more about each tool command, see 'go tool command -h'.
but it was suggested to change the suggestion to say:
see 'go doc command'
In #18313.
That would work for every tool except dist, which has no doc.go.
This change adds a doc.go file to cmd/dist.
Updates #18313
Change-Id: If67a21934b87647a69359d9c14d8de3775c587b7
Reviewed-on: https://go-review.googlesource.com/54351
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently all trace slices get shifted to start at time 0. This makes
it very difficult to find specific points in time unless they fall in
the first slice.
For example, right now when you click "View trace
(6.005646218s-8.155419698s)" on the trace tool's main page, the trace
view puts the first event in that slice at time 0. If you're looking
for something that happened at time 7s, you have to look at time
0.9943537s in the trace view. And if you want to subtract times taken
from different slices, you have to figure out what those time really
correspond to.
Fix this by telling the trace viewer not to shift the times when it
imports the trace. In the above example, this makes the view of that
second trace slice start at time 6.005646218s, so you don't have to do
any gymnastics to find or calculate times in later slices.
Change-Id: I04e0afda60f5573fdd8ad96238c24013297ef263
Reviewed-on: https://go-review.googlesource.com/54633
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
This updates the HTML served for the trace viewer to follow the latest
revision of the example from the upstream tracing project.
The main thing this adds is CSS for the trace viewer (which was
actually in the example at the originally referenced revision, so I'm
not sure why it got dropped). In particular, this expands the trace
viewer to use the entire browser client area, which fixes several
problems with the current page:
1. The details pane gets cut off at a strange place and can get a
scroll bar even if there's plenty of room below it on the page. This
fixes the bottom of the details pane to the bottom of the window.
2. If the track view is very tall (lots of procs), there's no way to
view the top tracks and the details pane at the same time. This fixes
this problem by limiting the height of the track view to something
less than the height of the window so it gets a scroll bar of its own
if necessary.
3. Dragging the divider between the track pane and the details pane
actually moves the bottom of the details pane without moving the
divider. Fixing the height of the trace viewer fixes this problem.
Change-Id: Ia811e72a7413417ca21c45e932c9db2724974633
Reviewed-on: https://go-review.googlesource.com/54632
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
This implements trunc, floor, and ceil in the math package
as intrinsics on ppc64x. Significant improvement mainly due
to avoiding call overhead of args and return value.
BenchmarkCeil-16 5.95 0.69 -88.40%
BenchmarkFloor-16 5.95 0.69 -88.40%
BenchmarkTrunc-16 5.82 0.69 -88.14%
Updates #21390
Change-Id: I951e182694f6e0c431da79c577272b81fb0ebad0
Reviewed-on: https://go-review.googlesource.com/54654
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
Reviewed-by: David Chase <drchase@google.com>
Change-Id: I8d295ea32bf56adc42171947133f3e16a88664c6
Reviewed-on: https://go-review.googlesource.com/54911
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Change-Id: I84b0e1d86728a76bc6a87fee4accf6fc43d87006
Reviewed-on: https://go-review.googlesource.com/54814
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reverts commit f0b3626904.
Reason for revert: this change caused the runtime tests on all linux/amd64 and linux/386 builders to timeout
Change-Id: Idf8cfdfc84540e21e8da403e74df5596a1d9327b
Reviewed-on: https://go-review.googlesource.com/54490
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Mostly node and position parameters that are no longer used.
Also remove an unnecessary node variable while at it.
Found with github.com/mvdan/unparam.
Change-Id: I88f9bd5d20bfc5b0f6f63ea81869daa246175061
Reviewed-on: https://go-review.googlesource.com/54130
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
If we've already imported a named type, then there's no need to
process its associated methods except to validate that the signature
matches the existing known method.
However, the current import code still creates a new function node for
each method, saves its inline body (if any), and adds the node to the
global importlist. Because of this, the duplicate methods are never
garbage collected.
This CL changes the compiler to avoid amassing uncollectable garbage
or performing any unnecessary processing.
This is particularly noticeable for protobuf-heavy code. For the
motivating Go package, this CL reduced compile max-RSS from ~12GB to
~3GB and compile time from ~65s to ~50s.
Passes toolstash -cmp for std, cmd, and k8s.io/kubernetes/cmd/....
Change-Id: Ib53ba9f2ad3212995671cf6ba220ee8a56d8d009
Reviewed-on: https://go-review.googlesource.com/51331
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This change enables buildmode c-shared on ppc64le.
A bug was fixed in runtime/rt0_linux_ppc64le.s that was necessary to
make this work. In _rt0_ppc64le_linux_lib, there is code to store
the value of r2 onto the caller's stack. However, if this file
is compiled using a build mode that maintains the TOC address in
r2, then instructions will be inserted at the beginning of this
function to generate the r2 value for the callee, not the caller.
That means the r2 value for the callee is stored onto the caller's
stack. If caller and callee don't have the same r2 values, then
the caller will restore the wrong r2 value after it returns. This
situation can happen when using dlopen since the caller of this
function will be in ld64.so and will definitely have a different
TOC.
Updates #20756
Change-Id: I6e165e0d0716e73721bbbcc520e8302e4856e3ba
Reviewed-on: https://go-review.googlesource.com/53890
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Otherwise, vet might have false positives when "C" is a variable and
we're just using a method on it. Or when an import was renamed to "C".
Add test files for both of these cases.
Fixes#20655.
Change-Id: I55fb93119444a67fcf7891ad92653678cbd4670e
Reviewed-on: https://go-review.googlesource.com/45551
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Rob Pike <r@golang.org>
These rules trigger a few times during make.bash.
When we eliminate boundedness checks from walk.go
we'll rely on them more heavily.
Updates #19692
Change-Id: I268c36ae2f1401c68dd685b15f2d30f5d6971176
Reviewed-on: https://go-review.googlesource.com/43775
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
gc.Sysfunc must not be called concurrently.
We set up runtime routines used by the backend
prior to doing any backend compilation.
I missed the 387 ones; fix that.
Sysfunc should have been unexported during 1.9.
I will rectify that in a subsequent CL.
Fixes#21352
Change-Id: I8386eaa1e05879c25c672b9c9fc693c938e9aeb6
Reviewed-on: https://go-review.googlesource.com/54090
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Avelino <t@avelino.xxx>
TryBot-Result: Gobot Gobot <gobot@golang.org>
ADDSDmem comment said f32 (likely a copy-paste mistake).
Also swap ADDSSmem and ADDSDmem positions in the list to uniform the
list order.
Fixes#21225
Change-Id: I26bb116900c1cf4c4e6faeef613d7318c9c85b98
Reviewed-on: https://go-review.googlesource.com/52071
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
Reviewed-by: Keith Randall <khr@golang.org>
The vet tool only reports a type checking error when invoked with -v.
Don't let that by itself cause vet to exit with an error exit status.
Updates #21188
Change-Id: I172c13d46c35d49e229e96e833683d8c82a77de7
Reviewed-on: https://go-review.googlesource.com/52851
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Rob Pike <r@golang.org>
Fixes#21205
Change-Id: I81b001eb42cbf2a5d5b7b82eb63548b22f501be5
Reviewed-on: https://go-review.googlesource.com/52110
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
For address of an auto or arg, on all non-x86 architectures
the assembler backend encodes the actual SP offset in the
instruction but leaves the offset in Prog unchanged. When the
assembly is printed in compile -S, it shows an offset
relative to pseudo FP/SP with an actual hardware SP base
register (e.g. R13 on ARM). This is confusing. Unset the
base register if it is indeed SP, so the assembly output is
consistent. If the base register isn't SP, it should be an
error and the error output contains the actual base register.
For address loading instructions, the base register isn't set
in the compiler on non-x86 architectures. Set it. Normally it
is SP and will be unset in the change mentioned above for
printing. If it is not, it will be an error and the error
output contains the actual base register.
No change in generated binary, only printed assembly. Passes
"go build -a -toolexec 'toolstash -cmp' std cmd" on all
architectures.
Fixes#21064.
Change-Id: Ifafe8d5f9b437efbe824b63b3cbc2f5f6cdc1fd5
Reviewed-on: https://go-review.googlesource.com/49432
Run-TryBot: Cherry Zhang <cherryyz@google.com>
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>
The linker is pretty good at combining a bunch of symbols into a
section, so let it do .debug_ranges the normal way. Along the way,
remove a bunch of globals that were only used by one function that would
only be called once per invocation.
Change-Id: I1a528a438b193c41e7c444e8830516b07f11affc
Reviewed-on: https://go-review.googlesource.com/43890
Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When the compiler decomposes a user variable, track its origin so that
it can be recomposed during DWARF generation.
Change-Id: Ia71c7f8e7f4d65f0652f1c97b0dda5d9cad41936
Reviewed-on: https://go-review.googlesource.com/50878
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Fix an oversight in decompose that caused floats to be missing from the
Names list.
Change-Id: I5db9c9498e9a4421742389eb929752fdac873b38
Reviewed-on: https://go-review.googlesource.com/50877
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
When we start tracking the mapping from Value to Prog, valueProgs will
be confusing. Disambiguate.
Change-Id: Ib3b302fedb7eb0ff1bde789d70a11656d82f0897
Reviewed-on: https://go-review.googlesource.com/50876
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
After we track decomposition, offset could mean stack offset or offset
in recomposed variable. Disambiguate.
Change-Id: I4d810b8c0dcac7a4ec25ac1e52898f55477025df
Reviewed-on: https://go-review.googlesource.com/50875
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
It is possible to have an unexported name with a nil package,
for an embedded field whose type is a pointer to an unexported type.
We must encode that fact in the type..namedata symbol name,
to avoid incorrectly merging an unexported name with an exported name.
Fixes#21120
Change-Id: I2e3879d77fa15c05ad92e0bf8e55f74082db5111
Reviewed-on: https://go-review.googlesource.com/50710
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This avoids an error from clang when using -nopie during compilation,
and permits us to check that the entire build succeeds.
Updates #21042
Change-Id: I2e6c7d5c97a85c223ed3288622bbb58ce33b8774
Reviewed-on: https://go-review.googlesource.com/50874
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fixes TestGoBuildUmask when the user has a POSIX ACL on the Go source tree.
Fixes#17909.
Change-Id: I5bc19099af8353afd41071258f4f317612b4c8c1
Reviewed-on: https://go-review.googlesource.com/50370
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Load/store-merging and move optimizations can result in unaligned
memory accesses. This is fine so long as the load/store instruction
used does not take a relative offset. In the SSA rules this means we
must not merge (MOVDaddr (SB)) ops into loads/stores unless we can
guarantee the alignment of the target.
Fixes#21048.
Change-Id: I70f13a62a148d5f0a56e704e8f76e36b4a4226d9
Reviewed-on: https://go-review.googlesource.com/49250
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Currently, Windows stacks are either 128kB or 2MB depending on whether
the binary uses cgo. This is because we assume that Go system stacks
and the small amount of C code invoked by the standard library can
operate within smaller stacks, but general Windows C code assumes
larger stacks.
However, it's easy to call into arbitrary C code using the syscall
package on Windows without ever importing cgo into a binary. Such
binaries need larger system stacks even though they don't use cgo.
Fix this on 64-bit by increasing the system stack size to 2MB always.
This only costs address space, which is free enough on 64-bit to not
worry about. We keep (for now) the existing heuristic on 32-bit, where
address space comes at more of a premium.
Updates #20975.
Change-Id: Iaaaa9a2fcbadc825cddc797aaaea8d34ef8debf2
Reviewed-on: https://go-review.googlesource.com/49331
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Framepointer is the default now. Only print an X: list
if the settings are _not_ the default.
Before:
$ go tool compile -V
compile version devel +a5f30d9508 Sun Jul 16 14:43:48 2017 -0400 X:framepointer
$ go1.8 tool compile -V
compile version go1.8 X:framepointer
$
After:
$ go tool compile -V
compile version devel +a5f30d9508 Sun Jul 16 14:43:48 2017 -0400
$ go1.9 tool compile -V # imagined
compile version go1.9
$
Perpetuates #18317.
Change-Id: I981ba5c62be32e650a166fc9740703122595639b
Reviewed-on: https://go-review.googlesource.com/49252
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fails on iOS because CC_FOR_TARGET points to clangwrap.sh in the
original GOROOT. We could fix that but it doesn't seem worth it.
Fails on Android with "exec format error". I'm not sure why but I
doubt it is interesting.
Fails on Plan 9 because the original GOROOT is being preserved in some
unknown way. This is issue #21016.
Updates #21016
Change-Id: I4e7115d734fc7bf21e5a2ba18fb6ad0bfa31c735
Reviewed-on: https://go-review.googlesource.com/48650
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David du Colombier <0intro@gmail.com>
Reviewed-by: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is necessary to make a relocated GOROOT work correctly.
Fixes#20997
Change-Id: I18624bd2e109721066cd9e4a887a12583ab79f5d
Reviewed-on: https://go-review.googlesource.com/48550
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Previous code failed to account for particular control flow
involving nested loops when updating phi function inputs.
Fix involves:
1) remove incorrect shortcut
2) generate a "better" order for children in dominator tree
3) note inner-loop updates and check before applying
outer-loop updates.
Fixes#20675.
Change-Id: I2fe21470604b5c259e777ad8b15de95f7706894d
Reviewed-on: https://go-review.googlesource.com/45791
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
CL 44352 changed the behavior of SIGINT, which can break tests that
themselves use SIGINT. I think we can only implement this if the
testing package has a way to know whether the code under test is using
SIGINT, but os/signal does not provide an API for that. Roll back for
1.9 and think about this again for 1.10.
Updates #19397
Change-Id: I021c314db2b9d0a80d0088b120a6ade685459990
Reviewed-on: https://go-review.googlesource.com/48370
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When a local variable is moved to the heap the declaration position
should be preserved so that later on we can assign it to the correct
DW_TAG_lexical_block.
Fixes#20959
Change-Id: I3700ef53c68ccd506d0633f11374ad88a52b2898
Reviewed-on: https://go-review.googlesource.com/47852
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
ld.addpltsym adds an R_X86_64_JMP_SLOT dynamic relocation to .rela.plt
and uses Addaddrplus to reference the GOT in Elf64_Rela.r_offset.
Addaddrplus results in an R_ADDR relocation, which here we transform
into an R_X86_64_64 dynamic relocation. This is wrong for several
reasons:
1. .rela.plt is not a writable, relro section. It is mapped read-only,
causing the dynamic linker to segfault when it tried to handle the
relocation. This was the immediate cause of internal PIE cgo
crashes.
2. Relocations targetting other reloc sections are, as far as I can
tell, undefined behavior in the ELF spec and are unlikely to be a
good idea.
3. Even if the relocation did work, it isn't what we want. The
relocation, if successfully handled, would have put an absolute
address as the JMP_SLOT offset, but it should be the offset from the
beginning of the binary, just like any other relocation. What we want
is a statically resolved R_ADDR relocation, just as is used below for
the R_X86_64_64 relocation.
Skipping the .rela.plt allows reloc() to handle these R_ADDR
relocations.
With this CL, internal PIE cgo binaries work.
Updates #18968
Change-Id: Ie74e6fe249e88150baa0e340b1cb128cf7f28673
Reviewed-on: https://go-review.googlesource.com/47837
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts commit 168eb9cf33.
CL 47831 fixes the issue with plugins on ARMv5, so we can re-enable the test.
Updates #19674.
Change-Id: Idcb29f93ffb0460413f1fab5bb82fa2605795038
Reviewed-on: https://go-review.googlesource.com/47834
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The text before CL 45816 was:
-timeout t
If a test runs longer than t, panic.
The default is 10 minutes (10m).
CL 45816 was supposed to be about clarifying test vs test binary,
and it did add the clarification of referring to "duration d",
but it also introduced incorrect text about timeout 0.
The new text in this CL preserves the good change and
eliminates the incorrect one:
-timeout d
If a test binary runs longer than duration d, panic.
The default is 10 minutes (10m).
For #14780.
Change-Id: I4f79d6e48ed9295bc9f34a36aa90d3b03b40d7f5
Reviewed-on: https://go-review.googlesource.com/47571
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Wrong instructions "MOVW 8(F0), R1" and "MOVW R0<<0(F1), R1"
are silently accepted, and all Fx are treated as Rx.
The patch checks all those illegal base registers.
fixes#20724
Change-Id: I05d41bb43fe774b023205163b7daf4a846e9dc88
Reviewed-on: https://go-review.googlesource.com/46132
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The current code calculates register number incorrectly.
The fix corrects the register number calculation.
Add cases created by decoder to test assembler.
Fixes#20697Fixes#20723
Change-Id: I73ac153df9ea9f51c43a5104828d7a5389551c92
Reviewed-on: https://go-review.googlesource.com/45850
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
clang can emit some dwarf.VoidType which are wrapped by multiple
dwarf.TypedefType. We need to unwrap those before further processing.
Fixes#20129
Change-Id: I671ce6aef2dc7b55f1a02aec5f9789ac1b369643
Reviewed-on: https://go-review.googlesource.com/44772
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If the LHS is unassignable, there's no point in trying to make sure
the RHS can be assigned to it or making sure they're realizable
types. This is consistent with go/types.
In particular, this prevents "1 = 2" from causing a panic when "1"
still ends up with the type "untyped int", which is not realizable.
Fixes#20813.
Change-Id: I4710bdaac2e375ef12ec29b888b8ac84fb640e56
Reviewed-on: https://go-review.googlesource.com/46835
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
After https://golang.org/cl/46421 is landed.
helpdoc.go should be updated that -buildmode=c-shared
requires only one main package.
Fixes#15082
Change-Id: I30a0ee956e6c4655c975ecdaa905887bd474952c
Reviewed-on: https://go-review.googlesource.com/46810
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fixes crash when printing a related error message later on.
Fixes#20789.
Change-Id: I6d2c35aafcaeda26a211fc6c8b7dfe4a095a3efe
Reviewed-on: https://go-review.googlesource.com/46713
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
"MULBB R1, R2, R3" is encoded to 0xe163f182, which should be
0xe1630182.
This patch fix it.
fix#20764
Change-Id: I9d3c3ffa40ecde86638e5e083eacc67578caebf4
Reviewed-on: https://go-review.googlesource.com/46491
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
If GOROOT_FINAL was set during the build, the default GOROOT
will not be testGOROOT. Determine the default GOROOT by reading
the right source file instead of guessing. (GOROOT_FINAL may no
longer be set when the test is actually run.)
Also refactor a bit.
Fixes#20284.
Change-Id: I2274595a235bee10c3f3a5ffecf4bb976f4d9982
Reviewed-on: https://go-review.googlesource.com/46428
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Maybe there are no Go files at all.
Maybe they are all excluded by build constraints.
Maybe there are only test Go files.
Be specific.
Fixes#17008.
Fixes parts of #20760.
Change-Id: If6ac82ba0ed437772e76e06763263747d3bc4f65
Reviewed-on: https://go-review.googlesource.com/46427
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We can't follow symlinks for fear of directory cycles and other problems,
but we can at least notice potentially-relevant symlinks that are being
ignored and report them.
Fixes#17662.
Change-Id: I1fce00bd5b80ea8df45dac8b61bfa08076ec5f4b
Reviewed-on: https://go-review.googlesource.com/46425
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We already detect this collision when both imports are used
anywhere in a single program. Also detect it when they are in
different targets being processed together.
Fixes#20264.
Change-Id: I5d3c822aae136053fbcb5ed167e1d67f9b847a0f
Reviewed-on: https://go-review.googlesource.com/46424
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
"MOVBS.U R0<<0(R1), R2" is assembled to 0xe19120d0 (ldrsb r2, [r1, r0]),
but it is expected to be 0xe11120d0 (ldrsb r2, [r1, -r0]).
This patch fixes it and adds more encoding tests.
fixes#20701
Change-Id: Ic1fb46438d71a978dbef06d97494a70c95fcbf3a
Reviewed-on: https://go-review.googlesource.com/45996
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Avoids confusing errors from the GNU assembler
processing Go assembly source code.
Fixes#19448.
Change-Id: Ic2c68b2521847cca5a3d078a092e5c60ec340840
Reviewed-on: https://go-review.googlesource.com/46423
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The current behavior is to filter out the non-main packages silently,
which is confusing if there are only non-main packages.
Instead, report an error unless it's used with a single main package.
To be clear, I don't really know what I'm doing.
It might be that multiple main packages are allowed, or even
that we do want the filtering, but all.bash passes with this change,
so I am taking that as a sign that we don't need that extra flexibility.
Fixes#15082.
Change-Id: I984d0f444a01c0ee0c3cd6646a75527ea99a9ebe
Reviewed-on: https://go-review.googlesource.com/46421
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This makes custom import path checks work even when the
custom import metadata directs checking out a subtree
of the subversion repository.
(Git and Mercurial allow no such thing, so they are unaffected.)
Fixes#20731.
Change-Id: I635f3a2037d69a87c6dac7b08b0a0d8266abd250
Reviewed-on: https://go-review.googlesource.com/46417
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Loops of the form "for i,e := range" needed to have their
condition rotated to the "bottom" for the preemptible loops
GOEXPERIMENT, but this caused a performance regression
because it degraded bounds check removal. For now, make
the loop rotation/guarding conditional on the experiment.
Fixes#20711.
Updates #10958.
Change-Id: Icfba14cb3b13a910c349df8f84838cf4d9d20cf6
Reviewed-on: https://go-review.googlesource.com/46410
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This reverts commit 3d13b5e00c.
Reason for revert: the new TestHttpsInsecure test breaks two darwin builders, the android builders, and one plan9 builder.
Change-Id: I09158e7d1bd2b3ffda57e7f2350f34eb9b62e784
Reviewed-on: https://go-review.googlesource.com/46158
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If you have BenchmarkX1 with sub-benchmark Y
and you have BenchmarkX2 with no sub-benchmarks,
then
go test -bench=X/Y
runs BenchmarkX1 once with b.N=1 (to find out about Y)
and then not again, because it has sub-benchmarks,
but arguably also because we're interested in Y.
In contrast, it runs BenchmarkX2 in full, even though clearly
that is not relevant to the match X/Y. We do have to run X2
once with b.N=1 to probe for having X2/Y, but we should not
run it with larger b.N.
Fixes#20589.
Change-Id: Ib86907e844f34dcaac6cd05757f57db1019201d0
Reviewed-on: https://go-review.googlesource.com/46031
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
This doesn't change the existing restriction with disallows
spaces in import paths (as found in an import declaration).
It simply permits packages to be under a directory name that
may contain spaces.
Verified manually that it works. This could use a test, but the
change is trivial. We also can't use the existing test framework
(under test/) because the way those tests are run with test/run.go,
the mechanims for compiling a directory, even if it contains blanks
it its name, does't produce compiler paths with blanks
(the compilation is local).
Fixes#20306.
Change-Id: I6cbffb86c3394347897c3c94b110da0aadc5bfdf
Reviewed-on: https://go-review.googlesource.com/46001
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Make it clearer that -test=X/Y runs all the tests matching X,
even if they don't have sub-tests matching Y.
Fixes#20589.
Change-Id: Ic27e89e748d60f67b50c68445ec0480066bdf207
Reviewed-on: https://go-review.googlesource.com/46030
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The existing docs states that, get looks for a branch or tag
that matches the locally installed version of Go.
First, this is only working for "go1", so it could be confusing.
Second, "If no such version exists it retrieves the most recent
version of the package". It's more the default branch, by git defaults,
rather than most recent version.
This should address the potential unclear parts.
Fixes#20320
Change-Id: Id7d727d88dc350c9902974b64fa28c3766f7e245
Reviewed-on: https://go-review.googlesource.com/45890
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Before CL 36170, we identified all function bodies that needed to be
exported before writing any export data.
With CL 36170, we started identifying additional functions while
exporting function bodies. As a consequence, we cannot use a
range-based for loop for iterating over function bodies anymore.
Fixes#18895.
Change-Id: I9cbefa8d311ca8c9898c8272b2ac365976b02396
Reviewed-on: https://go-review.googlesource.com/45817
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
These are used by DIV[U] and MOD[U] assembly instructions.
Add a test in the stdlib so we actually exercise linking
to these routines.
Update #19507
Change-Id: I0d8e19a53e3744abc0c661ea95486f94ec67585e
Reviewed-on: https://go-review.googlesource.com/45703
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The existing code used Type.String() to obtain the name of a type;
specifically type reflect.Method in this case. However, Type.String()
formatting is intended for error messages and uses the format
pkgpath.name instead of pkgname.name if a package (in this case
package reflect) is imported multiple times. As a result, the
reflect.Method type detection failed under peculiar circumstances
(see the included test case).
Thanks to https://github.com/ericlagergren for tracking down
an easy way to make the bug disappear (which in turn directly
led to the underlying cause).
Fixes#19028.
Change-Id: I1b9c5dfd183260a9be74969fe916a94146fc36da
Reviewed-on: https://go-review.googlesource.com/45777
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The check that the go tool version matched the go compiler version was
too aggressive and didn't cover the bootstrapping case with make.bash.
We never noticed because we never had a VERSION file in place.
Repro:
$ echo "go1.9beta1" > $GOROOT/VERSION
$ cd $GOROOT/src
$ ./make.bash
No test, because x/build/cmd/release catches it.
Updates #19064Fixes#20674
Change-Id: Ibdd7a92377f4cc77d71ed548f02d48bde6550f67
Reviewed-on: https://go-review.googlesource.com/45778
Reviewed-by: Chris Broadfoot <cbro@golang.org>
Also add runtime· prefixes to the code that is still used.
Fixes#19507
Change-Id: Ib6da6b2a9e398061d3f93958ee1258295b6cc33b
Reviewed-on: https://go-review.googlesource.com/45699
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
With current GCC a macro that refers to another macro can report an
error on the macro definition line, with a note on the use.
When cgo is trying to decide which line an error refers to,
it is looking at the uses. So if we see an error on a line that we
don't recognize followed by a note on a line that we do recognize,
treat the note as an error.
Fixes#20125.
Change-Id: I389cd0eb7d56ad2d54bef70e278d9f76c4d36448
Reviewed-on: https://go-review.googlesource.com/44290
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Hiroshi Ioka <hirochachacha@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When using golang on ppc64le there have been issues
when building executables that generate extremely large text
sections. This is due to the call instruction and the limitation
on the offset field, which is smaller than most platforms. If the
size of the call target offset is too big for the offset field in
the call instruction, then link errors can occur.
The original solution to this problem in golang was to split the
text section when it became too large, allowing the external (GNU)
linker to insert the necessary stub to handle the long call. That
worked fine until the another size limit for the program size was hit,
where a plt_branch was created instead of a long branch. In that case
the plt_branch code sequence expects r2 to contain the address of the
TOC, but when golang creates dynamic executables by default
(-buildmode=exe) r2 does not always contain the address of the TOC
and as a result when building programs that reach this extremely
large size, a runtime SEGV or SIGILL can occur due to branching to a bad
address.
When using internal linking, trampolines are generated to handle the
long calls but the text sections are not split. With this change,
text sections will still be split approrpriately with external linking
but if the buildmode being used does not maintain r2 as the TOC
addresses, then trampolines will be created for those calls.
Fixes#20497
Change-Id: If5400b0f86c2c08e106b332be6db0b259b07d93d
Reviewed-on: https://go-review.googlesource.com/45130
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The compiler now also prints column information - make sure we use
the correct regexp for compiler error cleanups. Accept both, error
positions with columns and without, since column printing may be
disabled with -gcflags=-C.
Fixes#20628.
Change-Id: I46dc921dd5c29d7b8172cd19a3df57951f60d889
Reviewed-on: https://go-review.googlesource.com/45612
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Otherwise the packages will not pick up the way that -installsuffix is
changed by -buildmode and -race.
Fixes#20512.
Change-Id: Ieca32c958a531beb6331dff30d7e9a4998867dbb
Reviewed-on: https://go-review.googlesource.com/44630
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
"MOVW FPSR, g" should be assembled to 0xeef1aa10, but actually
0xee30a110 (RFS). "MOVW g, FPSR" should be 0xeee1aa10, but actually
0xee20a110 (WFS). They should be updated to VFP forms, since the ARM
back end doesn't support non-VFP floating points.
The patch fixes them and adds more assembly encoding tests.
fixes#20643
Change-Id: I3b29490337c6e8d891b400fcedc8b0a87b82b527
Reviewed-on: https://go-review.googlesource.com/45276
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Done with grep & interactive search & replace, to double-check
replacements. Not many remained after CL 20022.
Fixes#18572
Change-Id: Idbe90ba3b584f9b9661d2bbd141607daaadfa41a
Reviewed-on: https://go-review.googlesource.com/45270
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
This results in names to unexported fields like
net.(*Dialer)."".deadline instead of net.(*Dialer).deadline.
Fixes#18419.
Change-Id: I0415c68b77cc16125c2401320f56308060ac3f25
Reviewed-on: https://go-review.googlesource.com/44070
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
"MOVW R1, CPSR" is assembled to 0xe129f001, which should be 0xe12cf001.
"MOVW $255, CPSR" is assembled to 0xe329f0ff, which should be 0xe32cf0ff.
This patch fixes them and adds more assembly encoding tests.
fix#20626
Change-Id: Iefc945879ea774edf40438ce39f52c144e1501a1
Reviewed-on: https://go-review.googlesource.com/45170
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Because of parallel tests, which have stalled executions, the RUN
output of a test can be much earlier than its completion output resulting
in hard-to-read verbose output.
The tests are displayed in the order in which the output shows
that they began, to make it easy to line up with the "RUN" output.
Similarly, the definitions of when tests begin and complete is
determined by when RUN and FAIL/SKIP/PASS are output since the
focus of this code is on enhancing readability.
Fixes#19397
Change-Id: I4d0ca3fd268b620484e7a190117f79a33b3dc461
Reviewed-on: https://go-review.googlesource.com/44352
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
No test because as far as I can tell, there aren't existing tests for
these.
Fixes#18383
Change-Id: I06eaef05777a1474886167e3797c5bcd93189d1b
Reviewed-on: https://go-review.googlesource.com/45156
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TestBuildIDContainsArchModeEnv fails on arm64 because defaultGO386 has
different value from x86 (amd64/386). On arm64 defaultGO386 = '387' but
on x86 defaultGO386 = 'sse2'. The difference will make the test succeed
on x86 while fail on arm64 since it generates the same build ID.
Fix it by explicitly setting GO386 instead of using default value
Fixes#20608
Change-Id: I864b0e47325942d9513516bdf47f60391d18c0d0
Reviewed-on: https://go-review.googlesource.com/45112
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Prep work for issues mentioned below. No semantic or functionality change.
For #11945.
For #17446.
Change-Id: Ia1bb2b87647a6daa47f7863c0eb42cf5e1d35a7c
Reviewed-on: https://go-review.googlesource.com/45076
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
We insert guard instructions after each "dangerous" instruction
to make NaCl's validator happy. This happens before asmout. If
in asmout an instruction is split to two dangerous instructions,
but only one guard instruction is inserted, the validation fails.
Therefore don't split instructions on NaCl.
Fixes#20595.
Change-Id: Ie34f209bc7d907d6d16ecef6721f88420981ac01
Reviewed-on: https://go-review.googlesource.com/45021
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This allows reading from package storage systems that may not
preserve the .a suffix (used with -importcfg).
Fixes#20579 (combined with CLs earlier in stack).
Change-Id: If2fc6a3d01bd0170a757e1f2ba9a22a4d9be7dbf
Reviewed-on: https://go-review.googlesource.com/44853
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Adds the ability to specify the file location of each imported package,
like in the -importcfg added to cmd/compile in a related CL.
In effect, -importcfg is a generalization of and supersedes -installsuffix
and -L. Of course, those flags will continue to be supported, for
compatibility with other tools.
Having this flag in Go 1.9 will let us try some experiments involving
package management without needing guinea pigs to build a custom
Go toolchain.
This flag also helps with #14271 at some later point.
For #20579.
Change-Id: Ie4c171bcd3aa2faa446ac340e36516f2f9853882
Reviewed-on: https://go-review.googlesource.com/44851
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Allows reading -importmap options from a file instead of putting
them all on the command line, and adds the ability to specify the
file location of specific packages. In effect, -importcfg is a generalization
of and supersedes -importmap, -importsuffix, and -I.
Of course, those flags will continue to be supported,
for compatibility with other tools.
Having this flag in Go 1.9 will let us try some experiments involving
package management without needing guinea pigs to build a
custom Go toolchain.
This flag also helps with #14271 at some later point.
For #20579.
Change-Id: If005dbc2b01d8fd16cbfd3687dfbe82499f4bc56
Reviewed-on: https://go-review.googlesource.com/44850
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The MOVFL instruction (which external PPC64 docs call mtcrf) can take
either a CR register or a constant. It doesn't make sense to specify
both, as the CR register implies the constant value. Specifying either
a register or a constant is enforced by the implementation in the
asmout method (case 69).
However, the optab was providing a form that specified both a constant
and a CR register, and was not providing a form that specified only a
constant. This CL fixes the optab table to provide a form that takes
only a constant.
No test because I don't know where to write it. The next CL in this
series will use the new instruction format.
Change-Id: I8bb5d3ed60f483b54c341ce613931e126f7d7be6
Reviewed-on: https://go-review.googlesource.com/44732
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Programs built from N libraries required O(N²) time to do the
deduplication checks, even if there were never any duplicates.
In most programs N is small enough not to worry, but this may
affect large programs.
Noticed by inspection, not any specific bug report.
Fixes#20578.
Change-Id: Ic4108f1058be39da990a79b1e0b8ce95fde44cef
Reviewed-on: https://go-review.googlesource.com/44852
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Otherwise removing a .f file won't trigger a rebuild.
Noticed by inspection while working on the code.
I don't have a good way to write a test for this,
and I expect the code to change significantly in the next
release, but might as well get it right for Go 1.9.
Change-Id: I3f6f9f71b3a7d4f0be49a47419dac59899959e7c
Reviewed-on: https://go-review.googlesource.com/44855
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
There are two issues in constant decomposition.
1. A typo in "func immrot2s" blocks "case 107" of []optab be triggered.
2. Though "ADD $0xffff, R0, R0" is decomposed to "ADD $0xff00, R0, R0" and
"ADD $0x00ff, R0, R0" as expected, "ADD $0xffff, R0" still uses the
constant pool, which should be the same as "ADD $0xffff, R0, R0".
This patch fixes them and adds more instruction encoding tests.
fix#20516
Change-Id: Icd7bdfa1946b29db15580dcb429111266f1384c6
Reviewed-on: https://go-review.googlesource.com/44335
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Fixes#17625
Change-Id: I09319b888b547c631a50dbfab9255cc08e8a2426
Reviewed-on: https://go-review.googlesource.com/40691
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I learned from CL 41770 that valState’s size
matters to compiler performance.
Encode that knowledge in a test.
Change-Id: I7c0fde6a4cf460017619dbcce1c1ddaa7af10239
Reviewed-on: https://go-review.googlesource.com/44737
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
For test.go:
package main
import (
"C"
"fmt"
)
func main() {
fmt.Println("Hello, world!")
C.no_such_f()
}
Before:
could not determine kind of name for C.no_such_f
After:
./test.go:10:2: could not determine kind of name for C.no_such_f
Fixes#18452
Change-Id: I49c136b7fa60fab25d2d5b905d440fe4d106e565
Reviewed-on: https://go-review.googlesource.com/34783
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Defaulting total to 1 for each function, adds up to the
counting error. testing/cover.go already does this once.
Fixes#20515
Change-Id: I0e3f524c2ccb628eb9a8f0a1f81c22365c24cf9a
Reviewed-on: https://go-review.googlesource.com/44337
Run-TryBot: Dhananjay Nakrani <dhananjayn@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Arguments to inlined calls are hidden from setPos as follows:
args := as.Rlist
as.Rlist.Set(nil)
// setPos...
as.Rlist.Set(args.Slice())
Previously, this code had no effect since the value of as was
overwritten by the assignment in the retvars loop.
Fixes#19799.
Change-Id: Iaf97259f82fdba8b236136337cc42b2774c7fef5
Reviewed-on: https://go-review.googlesource.com/44351
Run-TryBot: David Lazar <lazard@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The test no longer passes as of https://golang.org/cl/43777, which
intentionally disables the feature that this test was checking for.
Nobody noticed since the test is not run in -short mode.
Updates #20435.
Change-Id: I64f37fd94c01f22ead88470b6d9bfe8a151ddb1c
Reviewed-on: https://go-review.googlesource.com/44650
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Apply the fix in CL 44355 to MIPS.
ARM64 has these rules but commented out for performance reason.
Fix the commented rules, in case they are enabled in the future.
Enhance the test so it triggers the failure on ARM and MIPS without
the fix.
Updates #20530.
Change-Id: I82d77448e3939a545fe519d0a29a164f8fa5417c
Reviewed-on: https://go-review.googlesource.com/44430
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This should help narrowing down the possible cause of #20514.
Updates #20514.
Change-Id: Ie997400c9749aace7783bd585b23dbb4cefc181d
Reviewed-on: https://go-review.googlesource.com/44375
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Replacing byteload-of-bytestore-of-x with x is incorrect
when x contains a larger-than-byte value (and so on for
16 and 32-bit load/store pairs). Replace "x" with the
appropriate zero/sign extension of x, which if unnecessary
will be repaired by other rules.
Made logic for arm match x86 and amd64; yields minor extra
optimization, plus I am (much) more confident it's correct,
despite inability to reproduce bug on arm.
Ppc64 lacks this optimization, hence lacks this problem.
See related https://golang.org/cl/37154/Fixes#20530.
Change-Id: I6af9cac2ad43bee99cafdcb04725ce7e55a43323
Reviewed-on: https://go-review.googlesource.com/44355
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Replaced ../gc/reflect.c with
cmd/compile/internal/gc/reflect.go.
Fixes#20525
Change-Id: Ibafd36ea446ace5c677df27873a4bbf716a0a9bc
Reviewed-on: https://go-review.googlesource.com/44376
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Vet returns with a nonzero exit for all possible messages in the
buildtag check. However for this file:
//+buildlinux
package main
vet returns a zero exit status:
$ go vet main.go
demo.go:1: possible malformed +build comment
$ echo $?
0
This CL sets the exit status to non zero for the remaining messages in
the buildtag check.
Change-Id: Ia2c35ebc3ec5ac311d2a0295b5b9fdd997a85726
Reviewed-on: https://go-review.googlesource.com/44371
Reviewed-by: Rob Pike <r@golang.org>
In preparation for CL 41770, upgrade .debug_info to DWARF4, and emit
DW_AT_frame_base on subprograms. This should make no semantic
difference.
Also fix a long-standing bug/inconsistency in puttattr: it didn't
add the addend to ref_addrs. Previously this didn't matter because it
was only used for types, but now it's used for section offsets into
symbols that have multiple entries.
RELNOTE=yes
Change-Id: Ib10654ac92edfa29c5167c44133648151d70cf76
Reviewed-on: https://go-review.googlesource.com/44210
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
It is expected to test assembly code for ARMv5, ARMv6 and ARMv7
in cmd/asm/internal/asm/endtoend_test.go. But actually the loop
in "func TestARMEndToEnd(t *testing.T)" runs three times all
for ARMv5.
This patch fixes that bug and adds a new armv6.s which is only tested
with GOARM=6.
fixes#20465
Change-Id: I5dbf00809a47ace2c195335e2c9bdd768479aada
Reviewed-on: https://go-review.googlesource.com/43930
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
"ADDF F0, R1, F2" is silently accepted by the arm assembler and
assembled to the same binary code of "ADDF F0, F1, F2". So does
"CMPF F0, R1".
"ABSF F0, F1, F2" is also silently accepted and assembled to a
different instruction.
This patch reports those illegal forms and adds test cases.
fix#20464
Change-Id: I88b80dc29de24c6266ac7bf7bce1578c5adbc68c
Reviewed-on: https://go-review.googlesource.com/43931
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
We weren't setting r0 to 0, as required by our generated code.
Before this patch, the misc/cgo/testcarchive tests failed on ppc64le.
After this patch, they work, so enable them.
Change-Id: I53b16746961da9f7c34f59030a1e40953c9c1e05
Reviewed-on: https://go-review.googlesource.com/44093
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Test that we really do move spills down to the dominator of
all the uses.
Also add a test where go1.8 would have moved the spill out of
the loop into two exit points, but go1.9 doesn't move the spill.
This is a case where the 1.9 spill moving code does not subsume
the 1.8 spill moving code.
Maybe we resurrect moving-spills-out-of-loops CL to fix this one.
(I suspect it wouldn't be worth the effort, but would be happy
to hear evidence otherwise.)
Update #20472
Change-Id: I7dbf8d65e7f4d675d14e5ecf502887cebda35d2a
Reviewed-on: https://go-review.googlesource.com/44038
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This is one hurdle to building Go on Android; the runtime does
not build properly because *_linux.go files are excluded from
the "Building go_bootstrap" step when GOOS=android.
There are other hurdles; this is the first one.
Change-Id: I766e4bbf6ffc0d273888913f2516cf3e995a1786
Reviewed-on: https://go-review.googlesource.com/38308
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Current code cannot handle string #define macros if those macros are
defined via other macros. This CL solve the issue.
Updates #18720
Change-Id: Ibed0773d10db3d545bb246b97e81c0d19e3af3d5
Reviewed-on: https://go-review.googlesource.com/41312
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Now:
$ GOARCH=arm GOARM=5 go install -x cmd/go
... followed by:
$ GOARCH=arm GOARM= go install -x cmd/go
... actually does work. Previously the second "go install" would reuse
the cached binaries from the GOARM=5 command and not rebuild.
(Or vice versa from GOARM= to GOARM=5)
And do the same for GO386.
Fixes#9737
Change-Id: I9630aab34d06465d5033e6743dfe6592c8247aa0
Reviewed-on: https://go-review.googlesource.com/43855
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Due to the fact that -cover injects additional code to the original
source, tests run with -cover will often have incorrect line numbers.
Also includes docs for -list regexp missed by ba8ff87
Updates #6329
Change-Id: I87f0618ac31e96071bca61055cc17c0cbdee208a
Reviewed-on: https://go-review.googlesource.com/38640
Reviewed-by: Rob Pike <r@golang.org>
Many instructions can not have a .S suffix, such as MULS, SWI, CLZ,
CMP, STREX and others. And so do .P and .W suffixes. Even wrong
assembly code is generated for some instructions with invalid
suffixes.
This patch tries to simplify .S/.W/.P checks. And a wrong assembly
test for arm is added.
fixes#20377
Change-Id: Iba1c99d9e6b7b16a749b4d93ca2102e17c5822fe
Reviewed-on: https://go-review.googlesource.com/43561
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Instead of just printing the value, print the original node to make the
error more human-friendly. Also print the value if its string form is
different than the original node, to make sure it's obvious what value
was duplicated.
This means that "case '@', '@':", which used to print:
duplicate case 64 in switch
Will now print:
duplicate case '@' (value 64) in switch
Factor this logic out into its own function to reuse it in range cases
and any other place where we might want to print a node and its value in
the future.
Also needed to split the errorcheck files because expression switch case
duplicates are now detected earlier, so they stop the compiler before it
gets to generating the AST and detecting the type switch case
duplicates.
Fixes#20112.
Change-Id: I9009b50dec0d0e705e5de9c9ccb08f1dce8a5a99
Reviewed-on: https://go-review.googlesource.com/41852
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TestScopeRanges has been added in CL 40095. This
test is failing on Plan 9 because executables don't
have a DWARF symbol table.
Fixes#20418.
Change-Id: I6dd3baa636998134ccd042203c8b5c3199a4d6e1
Reviewed-on: https://go-review.googlesource.com/43670
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Change compiler and linker to emit DWARF lexical blocks in .debug_info
section when compiling with -N -l.
Version of debug_info is updated from DWARF v2 to DWARF v3 since
version 2 does not allow lexical blocks with discontinuous PC ranges.
Remaining open problems:
- scope information is removed from inlined functions
- variables records do not have DW_AT_start_scope attributes so a
variable will shadow other variables with the same name as soon as its
containing scope begins, even before its declaration.
Updates #6913.
Updates #12899.
Change-Id: Idc6808788512ea20e7e45bcf782453acb416fb49
Reviewed-on: https://go-review.googlesource.com/40095
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The writebarrier pass processes WB ops from beginning to end,
replacing them by other values.
But it also checks whether there are more ops to process
by walking from beginning to end.
This is quadratic, so walk from end to beginning instead.
This speeds up compiling the code in issue 13554:
name old time/op new time/op delta
Pkg 11.9s ± 2% 8.3s ± 3% -29.88% (p=0.000 n=18+17)
Updates #13554
Passes toolstash-check.
Change-Id: I5f8a872ddc4b783540220d89ea2ee188a6d2b2ff
Reviewed-on: https://go-review.googlesource.com/43571
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
SWI only support "SWI $imm", but currently "SWI (Reg)" is also
accepted. This patch fixes it.
And more instruction tests are added to cmd/asm/internal/asm/testdata/arm.s
fixes#20375
Change-Id: Id437d853924a403e41da9b6cbddd20d994b624ff
Reviewed-on: https://go-review.googlesource.com/43552
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Second attempt to fix#14710.
CL 35272 already tried to fix this issue. But CL 35272 assumed
that runtime.epclntab type is STEXT, while it is actually SRODATA.
This CL uses Symbol.Sect.Seg to determine if symbol is part
of Segtext or Segdata.
Fixes#14710
Change-Id: Ic6b6f657555c87a64d2bc36cc4c07ab0591d00c4
Reviewed-on: https://go-review.googlesource.com/42390
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
fuseBlockPlain was accidentally quadratic.
If you had plain blocks b1 -> b2 -> b3 -> b4,
each containing single values v1, v2, v3, and v4 respectively,
fuseBlockPlain would move v1 from b1 to b2 to b3 to b4,
then v2 from b2 to b3 to b4, etc.
There are two obvious fixes.
* Look for runs of blocks in fuseBlockPlain
and handle them in a single go.
* Fuse from end to beginning; any given value in a run
of blocks to fuse then moves only once.
The latter is much simpler, so that's what this CL does.
Somewhat surprisingly, this change does not pass toolstash-check.
The resulting set of blocks is the same,
and the values in them are the same,
but the order of values in them differ,
and that order of values (while arbitrary)
is enough to change the compiler's output.
This may be due to #20178; deadstore is the next pass after fuse.
Adding basic sorting to the beginning of deadstore
is enough to make this CL pass toolstash-check:
for _, b := range f.Blocks {
obj.SortSlice(b.Values, func(i, j int) bool { return b.Values[i].ID < b.Values[j].ID })
}
Happily, this CL appears to result in better code on average,
if only by accident. It cuts 4k off of cmd/go; go1 benchmarks
are noisy as always but don't regress (numbers below).
No impact on the standard compilebench benchmarks.
For the code in #13554, this speeds up compilation dramatically:
name old time/op new time/op delta
Pkg 53.1s ± 2% 12.8s ± 3% -75.92% (p=0.008 n=5+5)
name old user-time/op new user-time/op delta
Pkg 55.0s ± 2% 14.9s ± 3% -73.00% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
Pkg 2.04GB ± 0% 2.04GB ± 0% +0.18% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
Pkg 6.21M ± 0% 6.21M ± 0% ~ (p=0.222 n=5+5)
name old object-bytes new object-bytes delta
Pkg 28.4M ± 0% 28.4M ± 0% +0.00% (p=0.008 n=5+5)
name old export-bytes new export-bytes delta
Pkg 208 ± 0% 208 ± 0% ~ (all equal)
Updates #13554
go1 benchmarks:
name old time/op new time/op delta
BinaryTree17-8 2.29s ± 2% 2.26s ± 2% -1.43% (p=0.000 n=48+50)
Fannkuch11-8 2.74s ± 2% 2.79s ± 2% +1.63% (p=0.000 n=50+49)
FmtFprintfEmpty-8 36.6ns ± 3% 34.6ns ± 4% -5.29% (p=0.000 n=49+50)
FmtFprintfString-8 58.3ns ± 3% 59.1ns ± 3% +1.35% (p=0.000 n=50+49)
FmtFprintfInt-8 62.4ns ± 2% 63.2ns ± 3% +1.19% (p=0.000 n=49+49)
FmtFprintfIntInt-8 95.1ns ± 2% 96.7ns ± 3% +1.61% (p=0.000 n=49+50)
FmtFprintfPrefixedInt-8 118ns ± 3% 113ns ± 2% -4.00% (p=0.000 n=50+49)
FmtFprintfFloat-8 191ns ± 2% 192ns ± 2% +0.40% (p=0.034 n=50+50)
FmtManyArgs-8 419ns ± 2% 420ns ± 2% ~ (p=0.228 n=49+49)
GobDecode-8 5.26ms ± 3% 5.19ms ± 2% -1.33% (p=0.000 n=50+49)
GobEncode-8 4.12ms ± 2% 4.15ms ± 3% +0.68% (p=0.007 n=49+50)
Gzip-8 198ms ± 2% 197ms ± 2% -0.50% (p=0.018 n=48+48)
Gunzip-8 31.9ms ± 3% 31.8ms ± 3% -0.47% (p=0.024 n=50+50)
HTTPClientServer-8 64.4µs ± 0% 64.0µs ± 0% -0.55% (p=0.000 n=43+46)
JSONEncode-8 10.6ms ± 2% 10.6ms ± 3% ~ (p=0.543 n=49+49)
JSONDecode-8 43.3ms ± 3% 43.1ms ± 2% ~ (p=0.079 n=50+50)
Mandelbrot200-8 3.70ms ± 2% 3.70ms ± 2% ~ (p=0.553 n=47+50)
GoParse-8 2.70ms ± 2% 2.71ms ± 3% ~ (p=0.843 n=49+50)
RegexpMatchEasy0_32-8 70.5ns ± 4% 70.4ns ± 4% ~ (p=0.867 n=48+50)
RegexpMatchEasy0_1K-8 162ns ± 3% 162ns ± 2% ~ (p=0.739 n=48+48)
RegexpMatchEasy1_32-8 66.1ns ± 5% 66.2ns ± 4% ~ (p=0.970 n=50+50)
RegexpMatchEasy1_1K-8 297ns ± 7% 296ns ± 7% ~ (p=0.406 n=50+50)
RegexpMatchMedium_32-8 105ns ± 5% 105ns ± 5% ~ (p=0.702 n=50+50)
RegexpMatchMedium_1K-8 32.3µs ± 4% 32.2µs ± 3% ~ (p=0.614 n=49+49)
RegexpMatchHard_32-8 1.75µs ±18% 1.74µs ±12% ~ (p=0.738 n=50+48)
RegexpMatchHard_1K-8 52.2µs ±14% 51.3µs ±13% ~ (p=0.230 n=50+50)
Revcomp-8 366ms ± 3% 367ms ± 3% ~ (p=0.745 n=49+49)
Template-8 48.5ms ± 4% 48.5ms ± 4% ~ (p=0.824 n=50+48)
TimeParse-8 263ns ± 2% 256ns ± 2% -2.98% (p=0.000 n=48+49)
TimeFormat-8 265ns ± 3% 262ns ± 3% -1.35% (p=0.000 n=48+49)
[Geo mean] 41.1µs 40.9µs -0.48%
Change-Id: Ib35fa15b54282abb39c077d150beee27f610891a
Reviewed-on: https://go-review.googlesource.com/43570
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
When the race detector is enabled,
the compiler randomizes the order in which functions are compiled,
in an attempt to shake out bugs.
But we never re-seed the rand source, so every execution is identical.
Fix that to get more coverage.
Change-Id: If5cdde03ef4f1bab5f45e07f03fb6614945481d7
Reviewed-on: https://go-review.googlesource.com/43572
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently, cgo converts integer macros into int64 if it's possible.
As a result, some macros which satisfy
math.MaxInt64 < x <= math.MaxUint64
will lose their original values.
This CL introduces the new probe to check signs,
so we can handle signed ints and unsigned ints separately.
Fixes#20369
Change-Id: I002ba452a82514b3a87440960473676f842cc9ee
Reviewed-on: https://go-review.googlesource.com/43476
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The generated file runtime/internal/sys/zversion.go is deleted by
`go tool cmd dist clean` as part of running clean.bash. Don't treat
a missing file as a reason to stop running the go tool; just treat
is as meaning that runtime/internal/sys is stale.
No test because I don't particularly want to clobber $GOROOT.
Fixes#20385.
Change-Id: I5251a99542cc93c33f627f133d7118df56e18af1
Reviewed-on: https://go-review.googlesource.com/43559
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
LLV and SCV are 64-bit load-linked and store-conditional. They
were used in runtime as #define WORD. Change them to normal
instruction form.
NOOP is hardware no-op. It was written as WORD $0. Make a name
for it for better disassembly output.
Fixes#12561.
Fixes#18238.
Change-Id: I82c667ce756fa83ef37b034b641e8c4366335e83
Reviewed-on: https://go-review.googlesource.com/40297
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Noticed while looking at #20356.
Cuts 160k (1%) off of the cmd/compile binary.
Change-Id: If2397bc6971d6be9be6975048adecb0b5efa6d66
Reviewed-on: https://go-review.googlesource.com/43501
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Previous CL (cmd/internal/objabi: shrink SymType down to a uint8) shrinks
SymType down to a uint8 but forgot making according change in goobj.
Fixes#20296
Also add a test to catch such Goobj format inconsistency bug
Change-Id: Ib43dd7122cfcacf611a643814e95f8c5a924941f
Reviewed-on: https://go-review.googlesource.com/42971
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Attaching positions to SB, SP, initial mem can result in
less-good line-numbering when compiled for debugging.
This "fix" also removes source position from a zero-valued
struct (but not from its fields) and from a zero-length
array constant.
This may be a general problem for constants in entry blocks.
Fixes#20367.
Change-Id: I7e9df3341be2e2f60f127d35bb31e43cdcfce9a1
Reviewed-on: https://go-review.googlesource.com/43531
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Enhance the one-live-memory-at-a-time check to run during many
more phases of the SSA backend. Also make it work in an interblock
fashion.
Change types.IsMemory to return true for tuples containing a memory type.
Fix trim pass to build the merged phi correctly. Doesn't affect
code but allows the check to pass after trim runs.
Switch the AddTuple* ops to take the memory-containing tuple argument second.
Update #20335
Change-Id: I5b03ef3606b75a9e4f765276bb8b183cdc172b43
Reviewed-on: https://go-review.googlesource.com/43495
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Adjust finddebugruntimepath to look for runtime/debug.go file
instead of runtime/runtime.go. This actually finds runtime.GOMAXPROCS
in every Go executable (including windows).
I also included "-Wl,-T,fix_debug_gdb_scripts.ld" parameter to gcc
invocation on windows to work around gcc bug (see #20183 for details).
This CL only fixes windows -buildmode=exe, buildmode=c-archive
is still broken.
Thanks to Egon Elbre and Nick Clifton for investigation.
Fixes#20183Fixes#20218
Change-Id: I5369a4db3913226aef3d9bd6317446856b0a1c34
Reviewed-on: https://go-review.googlesource.com/43331
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Make yellow the last highlight color rather than the first.
Yellow is also the color that Chrome uses to highlight
search results, which can be confusing.
Also, when Night Shift is on on macOS,
yellow highlighting is completely invisible.
I suppose should be sleeping instead.
Also, remove a completed TODO.
Change-Id: I0eb4439272fad9ccb5fe8e2cf409fdd5dc15b26e
Reviewed-on: https://go-review.googlesource.com/43463
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When compiling concurrently, we walk all functions before compiling
any of them. Walking functions can cause variables to switch from
being non-addrtaken to addrtaken, e.g. to prepare for a runtime call.
Typechecking propagates addrtaken-ness of closure variables to
their outer variables, so that capturevars can decide whether to
pass the variable's value or a pointer to it.
When all functions are compiled immediately, as long as the containing
function is compiled prior to the closure, this propagation has no effect.
When compilation is deferred, though, in rare cases, this results in
a change in the addrtaken-ness of a variable in the outer function,
which in turn changes the compiler's output.
(This is rare because in a great many cases, a temporary has been
introduced, insulating the outer variable from modification.)
But concurrent compilation must generate identical results.
To fix this, track whether capturevars has run.
If it has, there is no need to update outer variables
when closure variables change.
Capturevars always runs before any functions are walked or compiled.
The remainder of the changes in this CL are to support the test.
In particular, -d=compilelater forces the compiler to walk all
functions before compiling any of them, despite being non-concurrent.
This is useful because -live is fundamentally incompatible with
concurrent compilation, but we want -c=1 to have no behavior changes.
Fixes#20250
Change-Id: I89bcb54268a41e8588af1ac8cc37fbef856a90c2
Reviewed-on: https://go-review.googlesource.com/42853
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
On Windows the drive letter is sometime "c:" and sometimes "C:".
Fixes#20336.
Change-Id: I38c86999af9522c51470d60016729d41cfec6b25
Reviewed-on: https://go-review.googlesource.com/43390
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
TestCgoContainsSpace builds a small program which mimics $CC.
Usually, $CC attempts to compile a trivial code to detect its own
supported flags (i.e. "-no-pie", which must be passed on some systems),
however the mimic didn't consider these cases.
This CL solve the issue.
Also, use the same name as $CC, it may solve other potential problems.
Fixes#20324
Change-Id: I7a00ac016a5fd0667540f2a715371f8152edc395
Reviewed-on: https://go-review.googlesource.com/43330
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Tuple ops are weird. They are essentially a pair of ops,
one which consumes a mem and one which generates a mem (the Select1).
The schedule pass didn't handle these quite right.
Fix the scheduler to include both parts of the paired op in
the store chain. That makes sure that loads are correctly ordered
with respect to the first of the pair.
Add a check for the ssacheck builder, that there is only one
live store at a time. I thought we already had such a check, but
apparently not...
Fixes#20335
Change-Id: I59eb3446a329100af38d22820b1ca2190ca46a78
Reviewed-on: https://go-review.googlesource.com/43294
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The writebarrier test has to change.
Now that T23 composite literals are passed to the backend,
they get SSA'd, so writes to their fields are treated separately,
so the relevant part of the first write to t23 is now a dead store.
Preserve the intent of the test by splitting it up into two functions.
Reduces code size a bit:
name old object-bytes new object-bytes delta
Template 386k ± 0% 386k ± 0% ~ (all equal)
Unicode 202k ± 0% 202k ± 0% ~ (all equal)
GoTypes 1.16M ± 0% 1.16M ± 0% ~ (all equal)
Compiler 3.92M ± 0% 3.91M ± 0% -0.19% (p=0.008 n=5+5)
SSA 7.91M ± 0% 7.91M ± 0% ~ (all equal)
Flate 228k ± 0% 228k ± 0% -0.05% (p=0.008 n=5+5)
GoParser 283k ± 0% 283k ± 0% ~ (all equal)
Reflect 952k ± 0% 952k ± 0% -0.06% (p=0.008 n=5+5)
Tar 188k ± 0% 188k ± 0% -0.09% (p=0.008 n=5+5)
XML 406k ± 0% 406k ± 0% -0.02% (p=0.008 n=5+5)
[Geo mean] 649k 648k -0.04%
Fixes#18872
Change-Id: Ifeed0f71f13849732999aa731cc2bf40c0f0e32a
Reviewed-on: https://go-review.googlesource.com/43154
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reuse block head or preceding instruction's line number for
register allocator's spill, fill, copy, rematerialization
instructionsl; and also for phi, and for no-src-pos
instructions. Assembler creates same line number tables
for copy-predecessor-line and for no-src-pos,
but copy-predecessor produces better-looking assembly
language output with -S and with GOSSAFUNC, and does not
require changes to tests of existing assembly language.
Split "copyInto" into two cases, one for register allocation,
one for otherwise. This caused the test score line change
count to increase by one, which may reflect legitimately
useful information preserved. Without any special treatment
for copyInto, the change count increases by 21 more, from
51 to 72 (i.e., quite a lot).
There is a test; using two naive "scores" for line number
churn, the old numbering is 2x or 4x worse.
Fixes#18902.
Change-Id: I0a0a69659d30ee4e5d10116a0dd2b8c5df8457b1
Reviewed-on: https://go-review.googlesource.com/36207
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Fixes#7906
Change-Id: Ibcf9cd670593241921ab3c426ff7357f799ebc3e
Reviewed-on: https://go-review.googlesource.com/43072
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Implements detection of x86 cpu features that
are used in the go standard library.
Changes all standard library packages to use the new cpu package
instead of using runtime internal variables to check x86 cpu features.
Updates: #15403
Change-Id: I2999a10cb4d9ec4863ffbed72f4e021a1dbc4bb9
Reviewed-on: https://go-review.googlesource.com/41476
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Eliminates stores of values that have just been loaded from the same
location. Handles the common case where there are up to 3 intermediate
stores to non-overlapping struct fields.
For example the loads and stores of x.a, x.b and x.d in the following
function are now removed:
type T struct {
a, b, c, d int
}
func f(x *T) {
y := *x
y.c += 8
*x = y
}
Before this CL (s390x):
TEXT "".f(SB)
MOVD "".x(R15), R5
MOVD (R5), R1
MOVD 8(R5), R2
MOVD 16(R5), R0
MOVD 24(R5), R4
ADD $8, R0, R3
STMG R1, R4, (R5)
RET
After this CL (s390x):
TEXT "".f(SB)
MOVD "".x(R15), R1
MOVD 16(R1), R0
ADD $8, R0, R0
MOVD R0, 16(R1)
RET
In total these rules are triggered ~5091 times during all.bash,
which is broken down as:
Intermediate stores | Triggered
--------------------+----------
0 | 1434
1 | 2508
2 | 888
3 | 261
--------------------+----------
Change-Id: Ia4721ae40146aceec1fdd3e65b0e9283770bfba5
Reviewed-on: https://go-review.googlesource.com/38793
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The files PPC64.rules and rewritePPC64.go were out of sync due to
conflicts between CL 41630 and CL 42145 (i.e. running 'go run *.go'
in the gen directory resulted in unexpected changes).
Change-Id: I1d409656b66afeab6cb9c6df9b3dcab7859caa75
Reviewed-on: https://go-review.googlesource.com/43091
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
This can make life easier for Delve (and other debuggers),
and can help them with bug reports.
Sample producer field (from objdump):
<48> DW_AT_producer : Go cmd/compile devel +8a59dbf41a Mon May 8 16:02:44 2017 -0400
Change-Id: I0605843c959b53a60a25a3b870aa8755bf5d5b13
Reviewed-on: https://go-review.googlesource.com/33588
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This adds math/bits intrinsics for OnesCount, Len, TrailingZeros on
ppc64x.
benchmark old ns/op new ns/op delta
BenchmarkLeadingZeros-16 4.26 1.71 -59.86%
BenchmarkLeadingZeros16-16 3.04 1.83 -39.80%
BenchmarkLeadingZeros32-16 3.31 1.82 -45.02%
BenchmarkLeadingZeros64-16 3.69 1.71 -53.66%
BenchmarkTrailingZeros-16 2.55 1.62 -36.47%
BenchmarkTrailingZeros32-16 2.55 1.77 -30.59%
BenchmarkTrailingZeros64-16 2.78 1.62 -41.73%
BenchmarkOnesCount-16 3.19 0.93 -70.85%
BenchmarkOnesCount32-16 2.55 1.18 -53.73%
BenchmarkOnesCount64-16 3.22 0.93 -71.12%
Update #18616
I also made a change to bits_test.go because when debugging some failures
the output was not quite providing the right argument information.
Change-Id: Ia58d31d1777cf4582a4505f85b11a1202ca07d3e
Reviewed-on: https://go-review.googlesource.com/41630
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
Reviewed-by: Keith Randall <khr@golang.org>
Also change type from map[*types.Type]bool to map[*types.Type]struct{}.
This is basically a clean-up.
Change-Id: I167583eff0fa1070a7522647219476033b52b840
Reviewed-on: https://go-review.googlesource.com/41859
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
OpVarXXX Values don't generate instructions,
so there's no reason not to duplicate them,
and duplicating them generates better code
(fewer branches).
This requires changing the start/end accounting
to correctly handle the case in which we have run
of Values beginning with an OpVarXXX, e.g.
OpVarDef, OpZeroWB, OpMoveWB.
In that case, the sequence of values should begin
at the OpZeroWB, not the OpVarDef.
This also lays the groundwork for experimenting
with allowing duplication of some scalar stores.
Shrinks function text sizes a tiny amount:
name old object-bytes new object-bytes delta
Template 381k ± 0% 381k ± 0% -0.01% (p=0.008 n=5+5)
Unicode 203k ± 0% 203k ± 0% -0.04% (p=0.008 n=5+5)
GoTypes 1.17M ± 0% 1.17M ± 0% -0.01% (p=0.008 n=5+5)
SSA 8.24M ± 0% 8.24M ± 0% -0.00% (p=0.008 n=5+5)
Flate 230k ± 0% 230k ± 0% ~ (all equal)
GoParser 286k ± 0% 286k ± 0% ~ (all equal)
Reflect 1.00M ± 0% 1.00M ± 0% ~ (all equal)
Tar 189k ± 0% 189k ± 0% ~ (all equal)
XML 415k ± 0% 415k ± 0% -0.01% (p=0.008 n=5+5)
Updates #19838
Change-Id: Ic5ef30855919f1468066eba08ae5c4bd9a01db27
Reviewed-on: https://go-review.googlesource.com/42011
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
In PPC64 ELF files, the st_other field indicates the number of
prologue instructions between the global and local entry points.
We add the instructions in the compiler and assembler if -shared is used.
We were assuming that the instructions were present when building a
c-archive or PIE or doing dynamic linking, on the assumption that those
are the cases where the go tool would be building with -shared.
That assumption fails when using some other tool, such as Bazel,
that does not necessarily use -shared in exactly the same way.
This CL records in the object file whether a symbol was compiled
with -shared (this will be the same for all symbols in a given compilation)
and uses that information when setting the st_other field.
Fixes#20290.
Change-Id: Ib2b77e16aef38824871102e3c244fcf04a86c6ea
Reviewed-on: https://go-review.googlesource.com/43051
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.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>
These collectively fire a few hundred times during make.bash,
mostly rewriting XOR SETNE -> SETEQ.
Fixes#17905.
Change-Id: Ic5eb241ee93ed67099da3de11f59e4df9fab64a3
Reviewed-on: https://go-review.googlesource.com/42491
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
CL 39915 introduced sorting of signats by ShortString
for reproducible builds. But ShortString treats types
byte and uint8 identically; same for rune and uint32.
CL 39915 attempted to compensate for this by only
adding the underlying type (uint8) to signats in addsignat.
This only works for byte and uint8. For e.g. *byte and *uint,
both get added, and their sort order is random,
leading to non-reproducible builds.
One fix would be to add yet another type printing mode
that doesn't eliminate byte and rune, and use it
for sorting signats. But the formatting routines
are complicated enough as it is.
Instead, just sort first by ShortString and then by String.
We can't just use String, because ShortString makes distinctions
that String doesn't. ShortString is really preferred here;
String is serving only as a backstop for handling of bytes and runes.
The long series of types in the test helps increase the odds of
failure, allowing a smaller number of iterations in the test.
On my machine, a full test takes 700ms.
Passes toolstash-check.
Updates #19961Fixes#20272
name old alloc/op new alloc/op delta
Template 37.9MB ± 0% 37.9MB ± 0% +0.12% (p=0.032 n=5+5)
Unicode 28.9MB ± 0% 28.9MB ± 0% ~ (p=0.841 n=5+5)
GoTypes 110MB ± 0% 110MB ± 0% ~ (p=0.841 n=5+5)
Compiler 463MB ± 0% 463MB ± 0% ~ (p=0.056 n=5+5)
SSA 1.11GB ± 0% 1.11GB ± 0% +0.02% (p=0.016 n=5+5)
Flate 24.7MB ± 0% 24.8MB ± 0% +0.14% (p=0.032 n=5+5)
GoParser 31.1MB ± 0% 31.1MB ± 0% ~ (p=0.421 n=5+5)
Reflect 73.9MB ± 0% 73.9MB ± 0% ~ (p=1.000 n=5+5)
Tar 25.8MB ± 0% 25.8MB ± 0% +0.15% (p=0.016 n=5+5)
XML 41.2MB ± 0% 41.2MB ± 0% ~ (p=0.310 n=5+5)
[Geo mean] 72.0MB 72.0MB +0.07%
name old allocs/op new allocs/op delta
Template 384k ± 0% 385k ± 1% ~ (p=0.056 n=5+5)
Unicode 343k ± 0% 344k ± 0% ~ (p=0.548 n=5+5)
GoTypes 1.16M ± 0% 1.16M ± 0% ~ (p=0.421 n=5+5)
Compiler 4.43M ± 0% 4.44M ± 0% +0.26% (p=0.032 n=5+5)
SSA 9.86M ± 0% 9.87M ± 0% +0.10% (p=0.032 n=5+5)
Flate 237k ± 1% 238k ± 0% +0.49% (p=0.032 n=5+5)
GoParser 319k ± 1% 320k ± 1% ~ (p=0.151 n=5+5)
Reflect 957k ± 0% 957k ± 0% ~ (p=1.000 n=5+5)
Tar 251k ± 0% 252k ± 1% +0.49% (p=0.016 n=5+5)
XML 399k ± 0% 401k ± 1% ~ (p=0.310 n=5+5)
[Geo mean] 739k 741k +0.26%
Change-Id: Ic27995a8d374d012b8aca14546b1df9d28d30df7
Reviewed-on: https://go-review.googlesource.com/42955
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
If there were more unused imports than
the maximum default number of errors to report,
the set of reported imports was non-deterministic.
Fix by accumulating and sorting them prior to output.
Fixes#20298
Change-Id: Ib3d5a15fd7dc40009523fcdc1b93ddc62a1b05f2
Reviewed-on: https://go-review.googlesource.com/42954
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
ARM64 assembler backend only accepts loads and stores with small
or aligned offset. The compiler therefore can only fold small or
aligned offsets into loads and stores. For locals and args, their
offsets to SP are not known until very late, and the compiler
makes conservative decision not folding some of them. However,
in most cases, the offset is indeed small or aligned, and can
be folded into load and store (but actually not).
This CL adds support of loads and stores with large and unaligned
offsets. When the offset doesn't fit into the instruction, it
uses two instructions and (for very large offset) the constant
pool. This way, the compiler doesn't need to be conservative,
and can simply fold the offset.
To make it work, the assembler's optab matching rules need to be
changed. Before, MOVD accepts C_UAUTO32K which matches multiple
of 8 between 0 and 32K, and also C_UAUTO16K, which may not be
multiple of 8 and does not fit into MOVD instruction. The
assembler errors in the latter case. This change makes it only
matches multiple of 8 (or offsets within ±256, which also fits
in instruction), and uses the large-or-unaligned-offset rule
for things doesn't fit (without error). Other sized move rules
are changed similarly.
Class C_UAUTO64K and C_UOREG64K are removed, as they are never
used.
In shared library, load/store of global is rewritten to using
GOT and temp register, which conflicts with the use of temp
register for assembling large offset. So the folding is disabled
for globals in shared library mode.
Reduce cmd/go binary size by 2%.
name old time/op new time/op delta
BinaryTree17-8 8.67s ± 0% 8.61s ± 0% -0.60% (p=0.000 n=9+10)
Fannkuch11-8 6.24s ± 0% 6.19s ± 0% -0.83% (p=0.000 n=10+9)
FmtFprintfEmpty-8 116ns ± 0% 116ns ± 0% ~ (all equal)
FmtFprintfString-8 196ns ± 0% 192ns ± 0% -1.89% (p=0.000 n=10+10)
FmtFprintfInt-8 199ns ± 0% 198ns ± 0% -0.35% (p=0.001 n=9+10)
FmtFprintfIntInt-8 294ns ± 0% 293ns ± 0% -0.34% (p=0.000 n=8+8)
FmtFprintfPrefixedInt-8 318ns ± 1% 318ns ± 1% ~ (p=1.000 n=10+10)
FmtFprintfFloat-8 537ns ± 0% 531ns ± 0% -1.17% (p=0.000 n=9+10)
FmtManyArgs-8 1.19µs ± 1% 1.18µs ± 1% -1.41% (p=0.001 n=10+10)
GobDecode-8 17.2ms ± 1% 17.3ms ± 2% ~ (p=0.165 n=10+10)
GobEncode-8 14.7ms ± 1% 14.7ms ± 2% ~ (p=0.631 n=10+10)
Gzip-8 837ms ± 0% 836ms ± 0% -0.14% (p=0.006 n=9+10)
Gunzip-8 141ms ± 0% 139ms ± 0% -1.24% (p=0.000 n=9+10)
HTTPClientServer-8 256µs ± 1% 253µs ± 1% -1.35% (p=0.000 n=10+10)
JSONEncode-8 40.1ms ± 1% 41.3ms ± 1% +3.06% (p=0.000 n=10+9)
JSONDecode-8 157ms ± 1% 156ms ± 1% -0.83% (p=0.001 n=9+8)
Mandelbrot200-8 8.94ms ± 0% 8.94ms ± 0% +0.02% (p=0.000 n=9+9)
GoParse-8 8.69ms ± 0% 8.54ms ± 1% -1.69% (p=0.000 n=8+10)
RegexpMatchEasy0_32-8 227ns ± 1% 228ns ± 1% +0.48% (p=0.016 n=10+9)
RegexpMatchEasy0_1K-8 1.92µs ± 0% 1.63µs ± 0% -15.08% (p=0.000 n=10+9)
RegexpMatchEasy1_32-8 256ns ± 0% 251ns ± 0% -2.19% (p=0.000 n=10+9)
RegexpMatchEasy1_1K-8 2.38µs ± 0% 2.09µs ± 0% -12.49% (p=0.000 n=10+9)
RegexpMatchMedium_32-8 352ns ± 0% 354ns ± 0% +0.39% (p=0.002 n=10+9)
RegexpMatchMedium_1K-8 106µs ± 0% 106µs ± 0% -0.05% (p=0.005 n=10+9)
RegexpMatchHard_32-8 5.92µs ± 0% 5.89µs ± 0% -0.40% (p=0.000 n=9+8)
RegexpMatchHard_1K-8 180µs ± 0% 179µs ± 0% -0.14% (p=0.000 n=10+9)
Revcomp-8 1.20s ± 0% 1.13s ± 0% -6.29% (p=0.000 n=9+8)
Template-8 159ms ± 1% 154ms ± 1% -3.14% (p=0.000 n=9+10)
TimeParse-8 800ns ± 3% 769ns ± 1% -3.91% (p=0.000 n=10+10)
TimeFormat-8 826ns ± 2% 817ns ± 2% -1.04% (p=0.050 n=10+10)
[Geo mean] 145µs 143µs -1.79%
Change-Id: I5fc42087cee9b54ea414f8ef6d6d020b80eb5985
Reviewed-on: https://go-review.googlesource.com/42172
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
It can be disabled by setting the environment variable
GO19CONCURRENTCOMPILATION=0, or with -gcflags=-c=1.
Fixes#15756.
Change-Id: I7acbf16330512b62ee14ecbab1f46b53ec5a67b6
Reviewed-on: https://go-review.googlesource.com/41820
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This permits the user to override the code generation flag when they
know better. This is always a good policy for all flags automatically
inserted by the build system.
Doing this now so that I can write a test for #20290.
Update #20290
Change-Id: I5c6708a277238d571b8d037993a5a59e2a442e98
Reviewed-on: https://go-review.googlesource.com/42952
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also switch "stating" to "statting" to describe applying os.Stat to
a resource; the former is more confusable than the latter.
Change-Id: I9d8e3506bd383f8f1479c05948c03b8c633dc4af
Reviewed-on: https://go-review.googlesource.com/42855
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
MOVSD is properly handled but its encoding test wasn't enabled. Enable
it.
For reference this was found with a little tool I wrote [1] to explore
which instructions are missing or not tested in the go obj package and
assembler:
"which SSE2 instructions aren't tested? And don't list instructions
which can take MMX operands"
$ x86db-gogen list --extension SSE2 --not-tested --not-mmx
CLFLUSH mem [m: np 0f ae /7] WILLAMETTE,SSE2
MOVSD xmmreg,xmmreg [rm: f2 0f 10 /r] WILLAMETTE,SSE2
MOVSD xmmreg,xmmreg [mr: f2 0f 11 /r] WILLAMETTE,SSE2
MOVSD mem64,xmmreg [mr: f2 0f 11 /r] WILLAMETTE,SSE2
MOVSD xmmreg,mem64 [rm: f2 0f 10 /r] WILLAMETTE,SSE2
(CLFLUSH was introduced with SSE2, but has its own CPUID bit)
[1] https://github.com/dlespiau/x86db
Change-Id: Ic3af3028cb8d4f02e53fdebb9b30fb311f4ee454
Reviewed-on: https://go-review.googlesource.com/42814
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
gcc on windows writes invalid location for .debug_gdb_scripts
which causes the executable loading to fail.
Fixes#20183
Change-Id: I5134013bc926b44a55b528f66ab79555855d1f4d
Reviewed-on: https://go-review.googlesource.com/42651
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
As discussion in issue #19141, the addend should be the third
argument of MULA. This patch fixes it in both the front end
and the back end of the assembler. And also tests are added to
the encoding test.
Fixes#19141
Change-Id: Idbc6f338b8fdfcad97a135f27a98c5b375b27d43
Reviewed-on: https://go-review.googlesource.com/42028
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Generated hash and eq routines don't need nil checks.
Prior to this CL, this was accomplished by
temporarily incrementing the global variable disable_checknil.
However, that increment lasted only the lifetime of the
call to funccompile. After CL 41503, funccompile may
do nothing but enqueue the function for compilation,
resulting in nil checks being generated.
Fix this by adding an explicit flag to a function
indicating whether nil checks should be disabled
for that function.
While we're here, allow concurrent compilation
with the -w and -W flags, since that was needed
to investigate this issue.
Fixes#20242
Change-Id: Ib9140c22c49e9a09e62fa3cf350f5d3eff18e2bd
Reviewed-on: https://go-review.googlesource.com/42591
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Marvin Stenger <marvin.stenger94@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
The Power processor manual states that "Branches not from the last instruction
of an aligned quadword and not to the first instruction of an aligned quadword
cause inefficiencies in the IBuffer". This changes the function alignment from 8
to 16 bytes to comply with that.
Fixes#18963
Change-Id: Ibce9bf8302110a86c6ab05948569af9ffdfcf4bb
Reviewed-on: https://go-review.googlesource.com/36390
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
The external darwin linker has been printing:
ld: warning: -read_only_relocs cannot be used with x86_64
for a long time. Now that it is printed by CL 33301, we may as
well get rid of it.
Fixes#20246
Change-Id: I1147cf1ff197fdfda228a1349f13627bcf9fc72f
Reviewed-on: https://go-review.googlesource.com/42730
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Todd Neal <todd@tneal.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
For .bss section symbol ldelf does not set P (raw symbol data).
Make ldpe do the same.
Change-Id: Ib3d558456f505ee568d0972465fa9b08b5794a87
Reviewed-on: https://go-review.googlesource.com/42631
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If we've already complained about a type T,
don't complain again about further expressions
involving it.
Fixes#20245 and hopefully all of its ilk.
Change-Id: Ic0abe8235d52e8a7ac40e3615aea8f3a54fd7cec
Reviewed-on: https://go-review.googlesource.com/42690
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
We already set it for mips32 objects. The native ELF linker warns when
linking PIC objects with non-PIC objects. Our objects are PIC, but we
were not marking them as such.
Fixes#20243.
Change-Id: Ifab131200b263e4c72cf81f7b131a65ac02a13a9
Reviewed-on: https://go-review.googlesource.com/42710
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change passes runtime.Version from the go tool to the compiler.
If the versions do not match, the compilation fails.
The result is a go tool from one GOROOT will complain loudly if it
is invoked with a different GOROOT value.
Only release versions are checked, so that when developing Go
you can still use "go install cmd/go" and "go install cmd/compile"
separately.
Fixes#19064
Change-Id: I17e184d07d3c1092b1d9af53ba55ed3ecf67791d
Reviewed-on: https://go-review.googlesource.com/42595
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Before this change, building a GOROOT using make.bash, and then
moving the entire to a new path confused the go tool. Correct
operation of the go tool under these conditions required either
running make.bash again (not always possible if the new location
was owned by a different system user) or setting the GOROOT
environment variable. Setting GOROOT is unfortunate and
discouraged, as it makes it too easy to use the go tool from
one GOROOT and the compiler from another GOROOT.
With this change, the go tool finds its GOROOT relative to its
own location, using os.Executable. It checks it is in a GOROOT
by searching for the GOROOT/pkg/tool directory, to avoid two
plausible situations:
ln -s $GOROOT/bin/go /usr/local/bin/go
and
PATH=$HOME/bin:$PATH
GOPATH=$HOME
ln -s $GOROOT/bin/go $HOME/bin/go
Additionally, if the current executable path is not in a GOROOT,
the tool will follow any symlinks for the executable and check
to see if its original path is a GOROOT.
Fixes#18678
Change-Id: I151d7d449d213164f98193cc176b616849e6332c
Reviewed-on: https://go-review.googlesource.com/42533
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Compile:
package p
var f = func(...A)
Before this CL:
x.go:3:13: type %!v(PANIC=runtime error: invalid memory address or nil pointer dereference) is not an expression
x.go:3:17: undefined: A
After this CL:
x.go:3:13: type func(...<T>) is not an expression
x.go:3:17: undefined: A
Found with go-fuzz.
Fixes#20233
Change-Id: Ibb232b3954c4091071440eba48b44c4022a8083f
Reviewed-on: https://go-review.googlesource.com/42610
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
When compiling the program:
package p
func _(){
*;:=
}
Before:
x.go:4:3: syntax error: unexpected semicolon, expecting expression
x.go:4:4: non-name *%!v(PANIC=runtime error: invalid memory address or nil pointer dereference) on left side of :=
x.go:5:1: syntax error: unexpected }, expecting expression
After:
x.go:4:3: syntax error: unexpected semicolon, expecting expression
x.go:4:4: non-name *<N> on left side of :=
x.go:5:1: syntax error: unexpected }, expecting expression
No test because:
(1) we don't have a good mechanism to check for the
absence of the string "PANIC" in an error message
(2) the string "*<N>", while better, is itself ugly enough
that I don't want to actively check for it
(3) the bug isn't very important, the kind of thing only fuzzers encounter
(4) the fix is obvious and trivial
Fixes#20220
Change-Id: I35faa986b60b671414ee999d6264b06937f250e3
Reviewed-on: https://go-review.googlesource.com/42498
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL modifies how MOV[DWHB] instructions that store a constant to
memory are assembled to avoid them clobbering the condition code
(flags). It also modifies zeroAuto to use MOVD instructions instead of
CLEAR (which is assembled as XC).
MOV[DWHB]storeconst ops also no longer clobbers flags.
Note: this CL modifies the assembler so that it can no longer handle
immediates outside the range of an int16 or offsets from SB, which
reflects what the machine instructions support. The compiler doesn't
need this capability any more and I don't think this affects any existing
assembly, but it is easy to workaround if it does.
Fixes#20187.
Change-Id: Ie54947ff38367bd6a19962bf1a6d0296a4accffb
Reviewed-on: https://go-review.googlesource.com/42179
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
5 shards, each of which spins up NumCPU processes,
each of which is running at GOMAXPROCS=NumCPU,
is too much for one machine. It makes my laptop unusable.
It might also be in part responsible for test flakes
that require a moderately responsive system,
like #18589 (backedge scheduling) and #19276 (locklinear).
It's possible that Go should be a better neighbor in general;
that's #17969. In the meantime, fix this corner of the world.
Builders snapshot the world and run shards on different
machines, so keeping sharding high for them is good.
This is a partial reversion of CL 18199.
Fixes#20141.
Change-Id: I123cf9436f4f4da3550372896265c38117b78071
Reviewed-on: https://go-review.googlesource.com/42431
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Prior to this CL, the compiler and assembler
were sloppy about the LSym.Type for LSyms
containing static data.
The linker then fixed this up, converting
Sxxx and SBSS to SDATA, and SNOPTRBSS to SNOPTRDATA
if it noticed that the symbol had associated data.
It is preferable to just get this right in cmd/compile
and cmd/asm, because it removes an unnecessary traversal
of the symbol table from the linker (see #14624).
Do this by touching up the LSym.Type fixes in
LSym.prepwrite and Link.Globl.
I have confirmed by instrumenting the linker
that the now-eliminated code paths were unreached.
And an additional check in the object file writing code
will help preserve that invariant.
There was a case in the Windows linker,
with internal linking and cgo,
where we were generating SNOPTRBSS symbols with data.
For now, convert those at the site at which they occur
into SNOPTRDATA, just like they were.
Does not pass toolstash-check,
but does generate identical linked binaries.
No compiler performance changes.
Change-Id: I77b071ab103685ff8e042cee9abb864385488872
Reviewed-on: https://go-review.googlesource.com/40864
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
DUFFZERO on 386 is not marked as clobbering flags, but rewriteToUseGot rewrote
"ADUFFZERO $offset" to "MOVL runtime.duffxxx@GOT, CX; ADDL $offset, CX; CALL CX"
which does. Luckily the fix is easier than figuring out what the problem was:
replace the ADDL $offset, CX with LEAL $offset(CX), CX.
On amd64 DUFFZERO clobbers flags, on arm, arm64 and ppc64 ADD does not clobber
flags and s390x does not use the duff functions, so I'm fairly confident this
is the only fix required.
I don't know how to write a test though.
Change-Id: I69b0958f5f45771d61db5f5ecb4ded94e8960d4d
Reviewed-on: https://go-review.googlesource.com/41821
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
ANDPS, like all others PS (Packed Single precision floats) instructions,
need Ym: they don't use the 0x66 prefix.
From the manual:
NP 0F 54 /r ANDPS xmm1, xmm2/m128
NP meaning, quoting the manual:
NP - Indicates the use of 66/F2/F3 prefixes (beyond those already part
of the instructions opcode) are not allowed with the instruction.
And indeed, the same instruction prefixed by 0x66 is ANDPD.
Updates #14069
Change-Id: If312a6f1e77113ab8c0febe66bdb1b4171e41e0a
Reviewed-on: https://go-review.googlesource.com/42090
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We generate code that calls each user init function one at a time.
When there are lots of user init functions,
usually due to generated code, like test/rotate* or
github.com/juju/govmomi/vim25/types,
we can end up with a giant function,
which can be slow to compile.
This CL puts in an escape valve.
When there are more than 500 functions, instead of doing:
init.0()
init.1()
// ...
we construct a static array of functions:
var fns = [...]func(){init.0, init.1, ... }
and call them in a loop.
This generates marginally bigger, marginally worse code,
so we restrict it to cases in which it might start to matter.
500 was selected as a mostly arbitrary threshold for "lots".
Each call uses two Progs, one for PCDATA and one for the call,
so at 500 calls we use ~1000 Progs.
At concurrency==8, we get a Prog cache of about
1000 Progs per worker.
So a threshold of 500 should more or less avoid
exhausting the Prog cache in most cases.
Change-Id: I276b887173ddbf65b2164ec9f9b5eb04d8c753c2
Reviewed-on: https://go-review.googlesource.com/41500
Reviewed-by: Keith Randall <khr@golang.org>
Noticed while adding to the bitset implementation
in cmd/compile/internal/gc.
The (Com (Const)) optimizations were already present
in the AMD64 lowered optimizations.
They trigger 118, 44, 262, and 108 times
respectively for int sizes 8, 16, 32, and 64
in a run of make.bash.
The (Or (And)) optimization is new.
It triggers 3 times for int size 8
and once for int size 64 during make.bash,
in packages internal/poll, reflect,
encoding/asn1, and go/types,
so there is a bit of natural test coverage.
Change-Id: I44072864ff88831d5ec7dce37c516d29df056e98
Reviewed-on: https://go-review.googlesource.com/41758
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Per gri's suggestion on CL 41623,
add a comment to trackAllTypes
about the trade-offs of enabling it.
Change-Id: Iec42b0da7933543200729003d1b2c6e0d9dcc5f0
Reviewed-on: https://go-review.googlesource.com/42186
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The generated test cases had their arguments reversed, putting them back
in order makes those tests pass.
CMPPS SRC, DEST, CC
Change-Id: Ie15021edc533d5681a6a78d10d88b665e3de9017
Reviewed-on: https://go-review.googlesource.com/42097
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I want to move the SHIDDEN type bit into Attribute, but AttrHidden is already
there and means something completely different, so rename it. (I'll give the
SHIDDEN bit a better name when it moves too).
Change-Id: I075403d9542b7626d4c1f6db9094329c4181aad3
Reviewed-on: https://go-review.googlesource.com/42024
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Without this, T can sneak through to the backend
with its width unknown.
Fixes#20174
Change-Id: I9b21e0e2641f75e360cc5e45dcb4eefe8255b675
Reviewed-on: https://go-review.googlesource.com/42175
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(ADDconst [c] x) && !isARMImmRot(uint32(c)) && isARMImmRot(uint32(-c)) -> (SUBconst [int64(int32(-c))] x)
(SUBconst [c] x) && !isARMImmRot(uint32(c)) && isARMImmRot(uint32(-c)) -> (ADDconst [int64(int32(-c))] x)
Currently
a = a + 0xfffffff1 is compiled to （variable a is in R0)
MVN $14, R11
ADD R11, R0, R0
After applying the above 2 rules, it becomes
SUB $15, R0, R0
(BICconst [c] (BICconst [d] x)) -> (BICconst [int64(int32(c|d))] x)
This rule also optimizes the generated ARM code.
The other rules are added to avoid to generate less optimized ARM code
when substitutions ADD->SUB happen.
Change-Id: I3ead9aae2b446b674e2ab42d37259d38ceb93a4d
Reviewed-on: https://go-review.googlesource.com/41679
Reviewed-by: Keith Randall <khr@golang.org>
Instead of playing whack-a-mole finding all
the non-dowidth'd expressions that can sneak
out of the frontend and then deciding on
just the right place to handle them,
use a big hammer.
Fixes#20152
Change-Id: Id452d9e8c4e9585216bd8bf0e0004c85aba4f9f7
Reviewed-on: https://go-review.googlesource.com/42021
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The code in #20162 contains an embedded interface.
It didn't get dowidth'd by the frontend,
and during DWARF generation, ngotype asked
for a string description of it,
which triggered a request for the number of fields
in the interface, which triggered a dowidth,
which is disallowed in the backend.
The other changes in this CL are to support the test.
Fixes#20162
Change-Id: I4d0be5bd949c361d4cdc89a8ed28b10977e40cf9
Reviewed-on: https://go-review.googlesource.com/42131
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Now that it only takes small values.
Change-Id: I08086d392529d8775b470d65afc2475f8d0e7f4a
Reviewed-on: https://go-review.googlesource.com/42030
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Using 'go build -tags "foo,bar"' might seem to work when you wanted
-tags "foo bar", since they make up a single tag that doesn't exist and
the build is unaffected.
Instead, error on any tag that contains a comma.
Fixes#18800.
Change-Id: I6641e03e2ae121c8878d6301c4311aef97026b73
Reviewed-on: https://go-review.googlesource.com/41951
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
More specifically, allow Unicode letters in the directories of GitHub
repositories, which can occur and don't have a valid reason to be
disallowed by go get.
Do so by using a predefined character class, the Unicode character
property class \p{L} that describes the Unicode characters that are
letters:
http://www.regular-expressions.info/unicode.html#category
Since it's not possible to create GitHub usernames or repositories
containing Unicode letters at this time, those parts of the import path
are still restricted to ASCII letters only.
Fix name of tested func in t.Errorf messages.
Fixes#18660.
Change-Id: Ia0ef4742bfd8317d989ef1eb1d7065e382852fe2
Reviewed-on: https://go-review.googlesource.com/41822
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Use it to ensure that dowidth is not called
from the backend on a type whose size
has not yet been calculated.
This is an alternative to CL 42016.
Change-Id: I8c7b4410ee4c2a68573102f6b9b635f4fdcf392e
Reviewed-on: https://go-review.googlesource.com/42018
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL mainly moves some work to the switch on w.Op,
to make a follow-up change simpler and clearer.
Updates #19838
Change-Id: I86f3181c380dd60960afcc24224f655276b8956c
Reviewed-on: https://go-review.googlesource.com/42010
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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>
Type.Size and Type.Alignment are for the front end:
They calculate size and alignment if needed.
Type.MustSize and Type.MustAlignment are for the back end:
They call Fatal if size and alignment are not already calculated.
Most uses are of MustSize and MustAlignment,
but that's because the back end is newer,
and this API was added to support it.
This CL was mostly generated with sed and selective reversion.
The only mildly interesting bit is the change of the ssa.Type interface
and the supporting ssa dummy types.
Follow-up to review feedback on CL 41970.
Passes toolstash-check.
Change-Id: I0d9b9505e57453dae8fb6a236a07a7a02abd459e
Reviewed-on: https://go-review.googlesource.com/42016
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
