The counter part, writeInt in cmd/internal/obj, writes int64s.
So the reader side should also read int64s. This may cause a
larger range of values being accepted, some of which should
not be that large. This is probably ok: for example, for
size/index/length, the very large value (due to corruption)
may be well past the end and causes other errors. And we did
not do much bound check anyway.
One exmaple where this matters is ARM32's object file. For one
type of relocation it encodes the instruction into Reloc.Add
field (which itself may be problematic and worth fix) and the
instruction encoding overflows int32, causing ARM32 object
file being rejected by goobj (and so objdump and nm) before.
Unskip ARM32 object file tests in goobj, nm, and objdump.
Updates #19811.
Change-Id: Ia46c2b68df5f1c5204d6509ceab6416ad6372315
Reviewed-on: https://go-review.googlesource.com/69010
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Recurse into structs/arrays of one element when
assigning names.
Test incorporated into existing end-to-end debugger test,
hand-verified that it fails without this CL.
Fixes#19868
Revives CL 40010
Old-Change-Id: I0266e58af975fb64cfa17922be383b70f0a7ea96
Change-Id: I122ac2375931477769ec8d763607c1ec42d78a7f
Reviewed-on: https://go-review.googlesource.com/71731
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Dsymutil, an utility used on macOS when externally linking executables,
does not support base address selector entries in debug_ranges.
To work around this deficiency this commit removes base address
selectors from debug_ranges and emits instead a list composed only of
compile unit relative addresses.
A new type of relocation is introduced, R_ADDRCUOFF, similar to
R_ADDROFF, that relocates an address to its offset from the low_pc of
the symbol's compile unit.
Fixes#21945
Change-Id: Ie991f9bc1afda2b49ac5d734eb41c37d3a37e554
Reviewed-on: https://go-review.googlesource.com/72371
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
After this CL, "go vet" can be guaranteed to have complete type information
about the packages being checked, even if cgo or swig is in use,
which will in turn make it reasonable for vet checks to insist on type
information. It also fixes vet's understanding of unusual import paths
like relative paths and vendored packages.
For now "go tool vet" will continue to cope without type information,
but the eventual plan is for "go tool vet" to query the go command for
what it needs, and also to be able to query alternate build systems
like bazel. But that's future work.
Fixes#4889.
Fixes#12556 (if not already fixed).
Fixes#15182.
Fixes#16086.
Fixes#17571.
Change-Id: I932626ee7da649b302cd269b82eb6fe5d7b9f0f2
Reviewed-on: https://go-review.googlesource.com/74750
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This CL adds support for accepting package config from
the go command. Paired with CL 74356 this lets us make
sure vet has complete information about package sources.
This fixes many issues (see CL 74356 for the list), including
mishandling of cgo and vendoring.
Change-Id: Ia4a1dce6f9b1b0a8ef5fdf9005a20a8b294969f1
Reviewed-on: https://go-review.googlesource.com/74355
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
commentText is only called if g != nil in ParseGo, so the check inside
commentText is redundant and can be deleted.
Change-Id: I130c18b738527c96bc59950b354a50b9e23f92e9
Reviewed-on: https://go-review.googlesource.com/74871
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is basically a mini-bootstrap, to reach a fixed point.
Change-Id: I88abad3d3ac961c3d11a48cb64d625d458684ef7
Reviewed-on: https://go-review.googlesource.com/74792
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Otherwise the new numbered directories like b028/ appear in the objects,
and they can change from run to run.
Fixes#22514.
Change-Id: I8d0cf65f3622e48b2547d5757febe0ee1301e2ed
Reviewed-on: https://go-review.googlesource.com/74791
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This makes 'go install cmd/compile' in one directory produce
a different binary from running it in another directory,
which is problematic for reproducible builds.
Change-Id: If26685d2e45d2695413b472142b49694716575fa
Reviewed-on: https://go-review.googlesource.com/74790
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Previously some of the AuxInt are uint32, which may not fit into
int32. This CL convert them to int32. This does not change the
generated code, but make ssacheck happy.
Pass "toolstash -cmp" for std cmd on ARM.
Fixes#22499.
Change-Id: Ib072d3c14962388bfeb0766c861995d00b4fa7c4
Reviewed-on: https://go-review.googlesource.com/74770
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
All of these had a return or break in the else body, so flipping the
condition means we can unindent and simplify.
Change-Id: If93e97504480d18a0dac3f2c8ffe57ab8bcb929c
Reviewed-on: https://go-review.googlesource.com/74190
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Previously, anytime we exported a function or method declaration
(which includes methods for every type transitively exported), we
included the inline function bodies, if any. However, in many cases,
it's impossible (or at least very unlikely) for the importing package
to call the method.
For example:
package p
type T int
func (t T) M() { t.u() }
func (t T) u() {}
func (t T) v() {}
T.M and T.u are inlineable, and they're both reachable through calls
to T.M, which is exported. However, t.v is also inlineable, but cannot
be reached.
Exception: if p.T is embedded in another type q.U, p.T.v will be
promoted to q.U.v, and the generated wrapper function could have
inlined the call to p.T.v. However, in practice, this doesn't happen,
and a missed inlining opportunity doesn't affect correctness.
To implement this, this CL introduces an extra flood fill pass before
exporting to mark inline bodies that are actually reachable, so the
exporter can skip over methods like t.v.
This reduces Kubernetes build time (as measured by "time go build -a
k8s.io/kubernetes/cmd/...") on an HP Z620 measurably:
== before ==
real 0m44.658s
user 11m19.136s
sys 0m53.844s
== after ==
real 0m41.702s
user 10m29.732s
sys 0m50.908s
It also significantly cuts down the cost of enabling mid-stack
inlining (-l=4):
== before (-l=4) ==
real 1m19.236s
user 20m6.528s
sys 1m17.328s
== after (-l=4) ==
real 0m59.100s
user 13m12.808s
sys 0m58.776s
Updates #19348.
Change-Id: Iade58233ca42af823a1630517a53848b5d3c7a7e
Reviewed-on: https://go-review.googlesource.com/74110
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
On PPC64 (and a few other architectures), accessing global
requires multiple instructions and use of temp register.
The compiler emits a single MOV prog, and the assembler
expands it to multiple instructions. If globals are accessed
multiple times, each time it generates a reload of the temp
register. As this is done by the assembler, the compiler
cannot optimize it.
This CL makes the compiler not fold address of global into load
and store. If a global is accessed multiple times, or multiple
fields of a struct are accessed, the compiler can CSE the
address. Currently, this doesn't help the case where different
globals are accessed, even though they may be close to each
other in the address space (which we don't know at compile time).
It helps a little bit in go1 benchmark:
name old time/op new time/op delta
BinaryTree17-2 4.84s ± 1% 4.84s ± 1% ~ (p=0.796 n=10+10)
Fannkuch11-2 4.10s ± 0% 4.08s ± 0% -0.58% (p=0.000 n=9+8)
FmtFprintfEmpty-2 97.9ns ± 1% 96.8ns ± 1% -1.08% (p=0.000 n=10+10)
FmtFprintfString-2 147ns ± 0% 147ns ± 1% ~ (p=0.129 n=9+10)
FmtFprintfInt-2 152ns ± 0% 152ns ± 0% ~ (p=0.294 n=10+8)
FmtFprintfIntInt-2 218ns ± 1% 217ns ± 0% -0.64% (p=0.000 n=10+8)
FmtFprintfPrefixedInt-2 263ns ± 1% 256ns ± 0% -2.77% (p=0.000 n=10+8)
FmtFprintfFloat-2 375ns ± 1% 368ns ± 0% -1.95% (p=0.000 n=10+7)
FmtManyArgs-2 849ns ± 0% 850ns ± 0% ~ (p=0.621 n=8+9)
GobDecode-2 12.3ms ± 1% 12.2ms ± 1% -0.94% (p=0.003 n=10+10)
GobEncode-2 10.3ms ± 1% 10.5ms ± 1% +2.03% (p=0.000 n=10+10)
Gzip-2 414ms ± 1% 414ms ± 0% ~ (p=0.842 n=9+10)
Gunzip-2 66.3ms ± 0% 66.4ms ± 0% ~ (p=0.077 n=9+9)
HTTPClientServer-2 66.3µs ± 5% 66.4µs ± 1% ~ (p=0.661 n=10+9)
JSONEncode-2 23.9ms ± 1% 23.9ms ± 1% ~ (p=0.905 n=10+9)
JSONDecode-2 119ms ± 1% 116ms ± 0% -2.65% (p=0.000 n=10+10)
Mandelbrot200-2 5.11ms ± 0% 4.92ms ± 0% -3.71% (p=0.000 n=10+10)
GoParse-2 5.81ms ± 1% 5.84ms ± 1% ~ (p=0.052 n=10+10)
RegexpMatchEasy0_32-2 315ns ± 0% 317ns ± 0% +0.67% (p=0.000 n=10+10)
RegexpMatchEasy0_1K-2 658ns ± 0% 638ns ± 0% -3.01% (p=0.000 n=9+9)
RegexpMatchEasy1_32-2 315ns ± 1% 317ns ± 0% +0.56% (p=0.000 n=9+9)
RegexpMatchEasy1_1K-2 935ns ± 0% 926ns ± 0% -0.96% (p=0.000 n=9+9)
RegexpMatchMedium_32-2 394ns ± 0% 396ns ± 1% +0.46% (p=0.001 n=10+10)
RegexpMatchMedium_1K-2 65.1µs ± 0% 64.5µs ± 0% -0.90% (p=0.000 n=9+9)
RegexpMatchHard_32-2 3.16µs ± 0% 3.17µs ± 0% +0.35% (p=0.000 n=10+9)
RegexpMatchHard_1K-2 89.4µs ± 0% 89.3µs ± 0% ~ (p=0.136 n=9+9)
Revcomp-2 703ms ± 2% 694ms ± 2% -1.41% (p=0.009 n=10+10)
Template-2 107ms ± 1% 107ms ± 1% ~ (p=0.053 n=9+10)
TimeParse-2 526ns ± 0% 524ns ± 0% -0.34% (p=0.002 n=9+9)
TimeFormat-2 534ns ± 0% 504ns ± 1% -5.51% (p=0.000 n=10+10)
[Geo mean] 93.8µs 93.1µs -0.70%
It also helps in the case mentioned in issue #17110, main.main
in package math's test. Now it generates 4 loads of R31 instead
of 10, for the same piece of code.
This causes a slight increase of binary size: cmd/go increases
0.66%.
If this is a good idea, we should do it on other architectures
where accessing global is expensive.
Updates #17110.
Change-Id: I2687af6eafc04f2a57c19781ec300c33567094b6
Reviewed-on: https://go-review.googlesource.com/68250
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
The new RoundToEven function can be implemented as a single FIDBR
instruction on s390x.
name old time/op new time/op delta
RoundToEven 5.32ns ± 1% 0.86ns ± 1% -83.86% (p=0.000 n=10+10)
Change-Id: Iaf597e57a0d1085961701e3c75ff4f6f6dcebb5f
Reviewed-on: https://go-review.googlesource.com/74350
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Noted in CL 73212 review by crawshaw.
Neglected to update CL 73212 before submitting.
Also fix printing of target goos/goarch for cross-compile build.
Change-Id: If702f23071a4456810f1de6abb9115b38933c5c1
Reviewed-on: https://go-review.googlesource.com/74631
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Every time I see an error that begins `missing argument for Fprintf("%s")`
my mental type-checker goes off, since obviously "%s" is not a valid first
argument to Fprintf. Writing Printf("%s") to report an error in Printf("hello %s")
is almost as confusing.
This CL rewords the errors reported by vet's printf check to be more
consistent with each other, avoid placing context like "in printf call"
in the middle of the message, and to avoid the imprecisions above by
not quoting the format string at all.
Before:
bad.go:9: no formatting directive in Printf call
bad.go:10: missing argument for Printf("%s"): format reads arg 1, have only 0 args
bad.go:11: wrong number of args for format in Printf call: 1 needed but 2 args
bad.go:12: bad syntax for printf argument index: [1]
bad.go:13: index value [0] for Printf("%[0]s"); indexes start at 1
bad.go:14: missing argument for Printf("%[2]s"): format reads arg 2, have only 1 args
bad.go:15: bad syntax for printf argument index: [abc]
bad.go:16: unrecognized printf verb 'z'
bad.go:17: arg "hello" for * in printf format not of type int
bad.go:18: arg fmt.Sprint in printf call is a function value, not a function call
bad.go:19: arg fmt.Sprint in Print call is a function value, not a function call
bad.go:20: arg "world" for printf verb %d of wrong type: string
bad.go:21: missing argument for Printf("%q"): format reads arg 2, have only 1 args
bad.go:22: first argument to Print is os.Stderr
bad.go:23: Println call ends with newline
bad.go:32: arg r in Sprint call causes recursive call to String method
bad.go:34: arg r for printf causes recursive call to String method
After:
bad.go:9: Printf call has arguments but no formatting directives
bad.go:10: Printf format %s reads arg #1, but have only 0 args
bad.go:11: Printf call needs 1 args but has 2 args
bad.go:12: Printf format %[1 is missing closing ]
bad.go:13: Printf format has invalid argument index [0]
bad.go:14: Printf format has invalid argument index [2]
bad.go:15: Printf format has invalid argument index [abc]
bad.go:16: Printf format %.234z has unknown verb z
bad.go:17: Printf format %.*s uses non-int "hello" as argument of *
bad.go:18: Printf format %s arg fmt.Sprint is a func value, not called
bad.go:19: Print arg fmt.Sprint is a func value, not called
bad.go:20: Printf format %d has arg "world" of wrong type string
bad.go:21: Printf format %q reads arg #2, but have only 1 args
bad.go:22: Print does not take io.Writer but has first arg os.Stderr
bad.go:23: Println args end with redundant newline
bad.go:32: Sprint arg r causes recursive call to String method
bad.go:34: Sprintf format %s with arg r causes recursive String method call
Change-Id: I5719f0fb9f2cd84df8ad4c7754ab9b79c691b060
Reviewed-on: https://go-review.googlesource.com/74352
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
The compiler's instrumentation pass has some out-of-date comments
about the write barrier and some confusing comments about
typedslicecopy. Update these comments and add a comment to
typedslicecopy explaining why it's manually instrumented while none of
the other operations are.
Change-Id: I024e5361d53f1c3c122db0c85155368a30cabd6b
Reviewed-on: https://go-review.googlesource.com/74430
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Hide in the source code instead of in the separate whitelist.
Removes the only printf false positive in the standard library.
Change-Id: I99285e67588c7c93bd56d59ee768a03be7c301e7
Reviewed-on: https://go-review.googlesource.com/74590
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
The httpresponse.go module wants to be able to tell if a particular type t
is net/http.Response (and also net/http.Client). It does this by importing
net/http, looking up Response, and then comparing that saved type against
each t.
Instead of doing an eager import of net/http, wait until we have a type t
to ask a question about, and then just look to see if that t is http.Response.
This kind of lazy check does not require assuming that net/http is available
or will be important (perhaps the check is disabled in this run, or perhaps
other conditions that lead to the comparison are not satisfied).
Not loading these kinds of types at startup time will scale better.
Change-Id: Ibb00623901a96e725a4ff6f231e6d15127979dfd
Reviewed-on: https://go-review.googlesource.com/74353
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The signal-to-noise ratio is too low.
Stop printing the name of every package.
Can still get the old output with make.bash -v.
Change-Id: Ib2c82e037166e6d2ddc31ae2a4d29af5becce574
Reviewed-on: https://go-review.googlesource.com/74351
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This cuts 23 seconds from all.bash on my MacBook Pro.
Change-Id: Ibc4d7c01660b9e9ebd088dd55ba993f0d7ec6aa3
Reviewed-on: https://go-review.googlesource.com/73991
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We can't make all.bash faster if we can't measure it.
Measure it.
Change-Id: Ia5da791d4cfbfa1fd9a8e905b3188f63819ade73
Reviewed-on: https://go-review.googlesource.com/73990
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This CL changes the go command to base all its rebuilding decisions
on the content of the files being processed and not their file system
modification times. It also eliminates the special handling of release
toolchains, which were previously considered always up-to-date
because modification time order could not be trusted when unpacking
a pre-built release.
The go command previously tracked "build IDs" as a backup to
modification times, to catch changes not reflected in modification times.
For example, if you remove one .go file in a package with multiple .go
files, there is no modification time remaining in the system that indicates
that the installed package is out of date. The old build ID was the hash
of a list of file names and a few other factors, expected to change if
those factors changed.
This CL moves to using this kind of build ID as the only way to
detect staleness, making sure that the build ID hash includes all
possible factors that need to influence the rebuild decision.
One such factor is the compiler flags. As of this CL, if you run
go build -gcflags -N cmd/gofmt
you will get a gofmt where every package is built with -N,
regardless of what may or may not be installed already.
Another such factor is the linker flags. As of this CL, if you run
go install myprog
go install -ldflags=-s myprog
the second go install will now correctly build a new myprog with
the updated linker flags. (Previously the installed myprog appeared
up-to-date, because the ldflags were not included in the build ID.)
Because we have more precise information we can also validate whether
the target of a "go test -c" operation is already the right binary and
therefore can avoid a rebuild.
This CL sets us up for having a more general build artifact cache,
maybe even a step toward not having a pkg directory with .a files,
but this CL does not take that step. For now the result of go install
is the same as it ever was; we just do a better job of what needs to
be installed.
This CL does slow down builds a small amount by reading all the
dependent source files in full. (The go command already read the
beginning of every dependent source file to discover build tags
and imports.) On my MacBook Pro, before this CL all.bash takes
3m58s, while after this CL and a few optimizations stacked above it
all.bash takes 4m28s. Given that CL 73850 cut 1m43s off the all.bash
time earlier today, we can afford adding 30s back for now.
More optimizations are planned that should make the go command
more efficient than it was even before this CL.
Fixes#15799.
Fixes#18369.
Fixes#19340.
Fixes#21477.
Change-Id: I10d7ca0e31ca3f58aabb9b1f11e2e3d9d18f0bc9
Reviewed-on: https://go-review.googlesource.com/73212
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
In the new content-based staleness world, setting -gcflags like this
recompiles all the packages involved in running the program, not just
the "stale" ones. So go run -gcflags=-d=ssa/check/on recompiles
runtime with those flags too, which is not what the test is trying
to check.
Change-Id: I4dbd5bf2970c3a622c01de84bd8aa9d5e9ec5239
Reviewed-on: https://go-review.googlesource.com/74570
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
If the go install doesn't use the same flags as the main build
it can overwrite the installed standard library, leading to
flakiness and slow future tests.
Force uses of 'go install' etc to propagate $GO_GCFLAGS
or disable them entirely, to avoid problems.
As I understand it, the main place this happens is the ssacheck builder.
If there are other uses that need to run some of the now-disabled
tests we can reenable fixed tests in followup CLs.
Change-Id: Ib860a253539f402f8a96a3c00ec34f0bbf137c9a
Reviewed-on: https://go-review.googlesource.com/74470
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Adds the following s390x test under mask (immediate) instructions:
TMHH
TMHL
TMLH
TMLL
These are useful for testing bits and are already used in the math package.
Change-Id: Idffb3f83b238dba76ac1e42ac6b0bf7f1d11bea2
Reviewed-on: https://go-review.googlesource.com/41092
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This change adds three new instructions:
- LPDFR: load positive (math.Abs(x))
- LNDFR: load negative (-math.Abs(x))
- CPSDR: copy sign (math.Copysign(x, y))
By making use of GPR <-> FPR moves we can now compile math.Abs and
math.Copysign to these instructions using SSA rules.
This CL also adds new rules to merge address generation into combined
load operations. This makes GPR <-> FPR move matching more reliable.
name old time/op new time/op delta
Copysign 1.85ns ± 0% 1.40ns ± 1% -24.65% (p=0.000 n=8+10)
Abs 1.58ns ± 1% 0.73ns ± 1% -53.64% (p=0.000 n=10+10)
The geo mean improvement for all math package benchmarks was 4.6%.
Change-Id: I0cec35c5c1b3fb45243bf666b56b57faca981bc9
Reviewed-on: https://go-review.googlesource.com/73950
Run-TryBot: Michael Munday <mike.munday@ibm.com>
Reviewed-by: Keith Randall <khr@golang.org>
When compiling a package that defines a type T with method T.M, we
already compile and emit the wrapper method (*T).M. There's no need
for every package that uses T to do the same.
Change-Id: I3ca2659029907570f8b98d66111686435fad7ed0
Reviewed-on: https://go-review.googlesource.com/74412
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
KeepAlive needs to introduce a use of the spill of the
value it is keeping alive. Without that, we don't guarantee
that the spill dominates the KeepAlive.
This bug was probably introduced with the code to move spills
down to the dominator of the restores, instead of always spilling
just after the value itself (CL 34822).
Fixes#22458.
Change-Id: I94955a21960448ffdacc4df775fe1213967b1d4c
Reviewed-on: https://go-review.googlesource.com/74210
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
If the compiler has a non-devel version it will report that version
to the go command for use as the "compiler ID" instead of using
the content ID of the binary. This in turn allows the go command
to see the compiled-for-amd64 arm compiler and the compiled-for-arm
arm compiler as having the same ID, so that packages cross-compiled
from amd64 look up-to-date when copied to the arm system
during the linux-arm buildlets and trybots.
Change-Id: I76cbf129303941f8e31bdb100e263478159ddaa5
Reviewed-on: https://go-review.googlesource.com/74360
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This implements runtime support for buffered write barriers on amd64.
The buffered write barrier has a fast path that simply enqueues
pointers in a per-P buffer. Unlike the current write barrier, this
fast path is *not* a normal Go call and does not require the compiler
to spill general-purpose registers or put arguments on the stack. When
the buffer fills up, the write barrier takes the slow path, which
spills all general purpose registers and flushes the buffer. We don't
allow safe-points or stack splits while this frame is active, so it
doesn't matter that we have no type information for the spilled
registers in this frame.
One minor complication is cgocheck=2 mode, which uses the write
barrier to detect Go pointers being written to non-Go memory. We
obviously can't buffer this, so instead we set the buffer to its
minimum size, forcing the write barrier into the slow path on every
call. For this specific case, we pass additional information as
arguments to the flush function. This also requires enabling the cgo
write barrier slightly later during runtime initialization, after Ps
(and the per-P write barrier buffers) have been initialized.
The code in this CL is not yet active. The next CL will modify the
compiler to generate calls to the new write barrier.
This reduces the average cost of the write barrier by roughly a factor
of 4, which will pay for the cost of having it enabled more of the
time after we make the GC pacer less aggressive. (Benchmarks will be
in the next CL.)
Updates #14951.
Updates #22460.
Change-Id: I396b5b0e2c5e5c4acfd761a3235fd15abadc6cb1
Reviewed-on: https://go-review.googlesource.com/73711
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
This adds support for math Abs, Copysign to be instrinsics on ppc64x.
New instruction FCPSGN is added to generate fcpsgn. Some new
rules are added to improve the int<->float conversions that are
generated mainly due to the Float64bits and Float64frombits in
the math package. PPC64.rules is also modified as suggested
in the review for CL 63290.
Improvements:
benchmark old ns/op new ns/op delta
BenchmarkAbs-16 1.12 0.69 -38.39%
BenchmarkCopysign-16 1.30 0.93 -28.46%
BenchmarkNextafter32-16 9.34 8.05 -13.81%
BenchmarkFrexp-16 8.81 7.60 -13.73%
Others that used Copysign also saw smaller improvements.
I attempted to make this work using rules since that
seems to be preferred, but due to the use of Float64bits and
Float64frombits in these functions, several rules had to be added and
even then not all cases were matched. Using rules became too
complicated and seemed too fragile for these.
Updates #21390
Change-Id: Ia265da9a18355e08000818a4fba1a40e9e031995
Reviewed-on: https://go-review.googlesource.com/67130
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Keith Randall <khr@golang.org>
This pragma is not actually honored by the compiler.
The tests implicitly relied on the inliner being unable
to inline closures with captured variables, which will
soon change.
Fixes#22208
Change-Id: I13abc9c930b9156d43ec216f8efb768952a29439
Reviewed-on: https://go-review.googlesource.com/73211
Reviewed-by: Michael Munday <mike.munday@ibm.com>
If runtime.GOROOT() and the os.Executable method for finding GOROOT
find the same directory but with different spellings, prefer the spelling
returned by runtime.GOROOT().
This avoids an inconsistency if "pwd" returns one spelling but a
different spelling is used in $PATH (and therefore in os.Executable()).
make.bash runs with GOROOT=$(cd .. && pwd); the goal is to allow
the resulting toolchain to use that default setting (unless moved)
even if the directory spelling is different in $PATH.
Change-Id: If96b28b9e8697f4888f153a400b40bbf58a9128b
Reviewed-on: https://go-review.googlesource.com/74250
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Currently copy and append for types containing only scalars and
notinheap pointers still get compiled to have write barriers, even
though those write barriers are unnecessary. Fix these to use
HasHeapPointer instead of just Haspointer so that they elide write
barriers when possible.
This fixes the unnecessary write barrier in runtime.recordspan when it
grows the h.allspans slice. This is important because recordspan gets
called (*very* indirectly) from (*gcWork).tryGet, which is
go:nowritebarrierrec. Unfortunately, the compiler's analysis has no
hope of seeing this because it goes through the indirect call
fixalloc.first, but I saw it happen.
Change-Id: Ieba3abc555a45f573705eab780debcfe5c4f5dd1
Reviewed-on: https://go-review.googlesource.com/73413
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Currently (*Type).HasHeapPointer only ignores pointers go:notinheap
types if the type itself is a pointer to a go:notinheap type. However,
if it's some other type that contains pointers where all of those
pointers are go:notinheap, it will conservatively return true. As a
result, we'll use write barriers where they aren't needed, for example
calling typedmemmove instead of just memmove on structs that contain
only go:notinheap pointers.
Fix this by making HasHeapPointer walk the whole type looking for
pointers that aren't marked go:notinheap.
Change-Id: Ib8c6abf6f7a20f34969d1d402c5498e0b990be59
Reviewed-on: https://go-review.googlesource.com/73412
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Most write barrier calls are inserted by SSA, but copy and append are
lowered to runtime.typedslicecopy during walk. Fix these to set
Func.WBPos and emit the "write barrier" warning, as done for the write
barriers inserted by SSA. As part of this, we refactor setting WBPos
and emitting this warning into the frontend so it can be shared by
both walk and SSA.
Change-Id: I5fe9997d9bdb55e03e01dd58aee28908c35f606b
Reviewed-on: https://go-review.googlesource.com/73411
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The current go:nowritebarrierrec checker has two problems that limit
its coverage:
1. It doesn't understand that systemstack calls its argument, which
means there are several cases where we fail to detect prohibited write
barriers.
2. It only observes calls in the AST, so calls constructed during
lowering by SSA aren't followed.
This CL completely rewrites this checker to address these issues.
The current checker runs entirely after walk and uses visitBottomUp,
which introduces several problems for checking across systemstack.
First, visitBottomUp itself doesn't understand systemstack calls, so
the callee may be ordered after the caller, causing the checker to
fail to propagate constraints. Second, many systemstack calls are
passed a closure, which is quite difficult to resolve back to the
function definition after transformclosure and walk have run. Third,
visitBottomUp works exclusively on the AST, so it can't observe calls
created by SSA.
To address these problems, this commit splits the check into two
phases and rewrites it to use a call graph generated during SSA
lowering. The first phase runs before transformclosure/walk and simply
records systemstack arguments when they're easy to get. Then, it
modifies genssa to record static call edges at the point where we're
lowering to Progs (which is the latest point at which position
information is conveniently available). Finally, the second phase runs
after all functions have been lowered and uses a direct BFS walk of
the call graph (combining systemstack calls with static calls) to find
prohibited write barriers and construct nice error messages.
Fixes#22384.
For #22460.
Change-Id: I39668f7f2366ab3c1ab1a71eaf25484d25349540
Reviewed-on: https://go-review.googlesource.com/72773
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
On linux/amd64, for now.
Updates #16706
Change-Id: Ib8c89b6edc73fb88042c06873ff815d387491504
Reviewed-on: https://go-review.googlesource.com/69117
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is ugly but needed on the builders, because they do not set
PWD/GOROOT consistently, and the new content-based staleness
understands that the setting of GOROOT influences the content in
the linker outputs.
Change-Id: I0606f2c70b719619b188864ad3ae1b34432140cb
Reviewed-on: https://go-review.googlesource.com/74070
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
New relocation flavor R_DWARFFILEREF, to be applied to DWARF attribute
values that correspond to file references (ex: DW_AT_decl_file,
DW_AT_call_file). The LSym for this relocation is the file itself; the
linker replaces the relocation target with the index of the specified
file in the line table's file section.
Note: for testing purposes this patch changes the DWARF function
subprogram DIE abbrev to include DW_AT_decl_file (allowed by DWARF
but not especially useful) so as to have a way to test this
functionality. This attribute will be removed once there are other
file reference attributes (coming as part of inlining support).
Change-Id: Icf676beb60fcc33f06d78e747ef717532daaa3ba
Reviewed-on: https://go-review.googlesource.com/73330
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The compiler depends on the way heap and sort break ties
in some cases. Instead of trying to find them all, bundle
those packages into the bootstrap compiler builds.
The overall goal is that Go1.4 building cmd/compile during the
bootstrap process produces a semantically equivalent compiler
to cmd/compile compiling itself. After this CL, that property is true,
at least for the compiler compiling itself and the other tools.
A test for this property will be in CL 73212.
Change-Id: Icc1ba7cbe828f5673e8198ebacb18c7c01f3a735
Reviewed-on: https://go-review.googlesource.com/73952
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Don't pass -gdwarf-2 to the external linker when external linkage is
requested. The Go compiler is now emitting DWARF version 4, so this
doesn't seem needed any more.
Fixes#22455
Change-Id: Ic4122c55e946619a266430f2d26f06d6803dd232
Reviewed-on: https://go-review.googlesource.com/73672
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
env.MkEnv was computing the full gcc command line to report as
$GOGCCFLAGS in "go env" output, which meant running gcc (or clang)
multiple times to discern which flags are available.
We also set $GOGCCFLAGS in the environment, but nothing actually uses
that as far as I can tell - it was always intended only for debugging.
Move GOGCCFLAGS to env.ExtraEnvVars, which displayed in "go env"
output but not set in child processes and not computed nearly as
often.
The effect is that trivial commands like "go help" or "go env GOARCH"
or "go tool -n compile" now run in about 0.01s instead of 0.1s,
because they no longer run gcc 4 times each.
go test -short cmd/go drops from 81s to 44s (and needs more trimming).
The $GOROOT/test suite drops from 92s to 33s, because the number of
gcc invocation drops from 13,336 to 0.
Overall, all.bash drops from 5m53s to 4m07s wall time.
Change-Id: Ia85abc89e1e2bb126b933aff3bf7c5f6c0984cd5
Reviewed-on: https://go-review.googlesource.com/73850
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
1. Move AXXX constants (A-enumeration) from "a.out.go" to "aenum.go"
2. Move VEX-encoded optabs from "asm6.go" to "vex_optabs.go"
Also run "go generate" over aenum.go. This explains diff in "anames.go".
Initialization of opindex is split into 2 loops:
one for `vexOptab`, second for `optab`.
Rationale:
when VEX instructions are generated with current structure,
asm6.go is modified, which can lead to merge conflicts and
larger diffs than desired. Same for a.out.go.
This change makes x86avxgen usage possible:
https://go-review.googlesource.com/c/arch/+/66972
Change-Id: Id9eefcf5ccf0a89440e5d01bcb80926a8163b41d
Reviewed-on: https://go-review.googlesource.com/70630
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Both the linker and the plugin package were inconsistent
about when they applied the path encoding defined in
objabi.PathToPrefix. As a result, only some symbols from
a package path that required encoding were being found.
So always encoding the path.
Fixes#22295
Change-Id: Ife86c79ca20b2e9307008ed83885e193d32b7dc4
Reviewed-on: https://go-review.googlesource.com/72390
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
