If GODEBUG=gocacheverify=1, then instead of using the cache to
avoid computations, the go command will do the computations and
double-check that they match any existing cache entries.
This is handled entirely in the cache implementation; there's no
complexity added to any of the cache usage sites.
(As of this CL there aren't any cache usage sites, but soon there will be.)
Also change GOCMDDEBUGHASH to the more usual GODEBUG=gocachehash=1.
Change-Id: I574f181e06b5299b1d9c6d402e40c57a0e064e74
Reviewed-on: https://go-review.googlesource.com/75294
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This lets users see the effective GOCACHE setting.
Change-Id: I0b6dd2945d54611be89ed68fe2fd99110b9a25f6
Reviewed-on: https://go-review.googlesource.com/75293
Reviewed-by: David Crawshaw <crawshaw@golang.org>
These are convenience function for small cached items.
Change-Id: Iba92b7826a9fd6979e627687f2ce72d4b4799385
Reviewed-on: https://go-review.googlesource.com/75292
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Use a build cache separate from the default user cache,
one that will be wiped out during startup, so that make.bash
continues to start from a clean slate.
Change-Id: I38733991015c66efb89fc170c71701b1dd9de28d
Reviewed-on: https://go-review.googlesource.com/75291
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The cache is stored in $GOCACHE, which is printed by go env and
defaults to a subdirectory named "go-build" in the standard user cache
directory for the host operating system.
This CL only implements the cache. Future CLs will store data in it.
Change-Id: I0b4965a9e50f852e17e44ec3d6dafe05b58f0d22
Reviewed-on: https://go-review.googlesource.com/68116
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The marshal method allows the hash's internal state to be serialized and
unmarshaled at a later time, without having the re-write the entire stream
of data that was already written to the hash.
Fixes#20573
Change-Id: I40bbb84702ac4b7c5662f99bf943cdf4081203e5
Reviewed-on: https://go-review.googlesource.com/66710
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
...because that's an illegal addressing mode.
I double-checked handling of this code, and 387 is the only
place where this check is missing.
Fixes#22429
Change-Id: I2284fe729ea86251c6af2f04076ddf7a5e66367c
Reviewed-on: https://go-review.googlesource.com/73551
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Not perfect, but it's a start.
Change-Id: I3283385223a39ea73567b0130290bfe4de69d018
Reviewed-on: https://go-review.googlesource.com/73552
Reviewed-by: Robert Griesemer <gri@golang.org>
CL 40112 intended to allow full flag processing in go vet, but missed
vet's -shift flag; this corrects the omission.
Fixes#22442
Change-Id: I47525018306bd8b9aa452fb378d0d45319f8cf11
Reviewed-on: https://go-review.googlesource.com/73553
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This test was previously disabled when external linking was
unsupported on ppc64le. It should still be disabled on ppc64
since there is no cgo or external linking there, but I removed
the if test for GOARCH=ppc64 since the initial test for cgo
enabled will cause it to be skipped on ppc64.
Fixes#22360
Change-Id: I5a0e3e4a1bd71ac7bf0ed0c792f7b78fb4a5e100
Reviewed-on: https://go-review.googlesource.com/73510
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The new pagezero_size introduced by CL 72730 breaks
on 32-bit systems, since it is 2³². Restrict the change to
darwin/arm64, since it is intended for iOS only.
We could plausibly allow GOARCH=amd64 as well, but
without a compelling reason, changing the zero page size
doesn't seem worth the risk.
Change-Id: I5d6adcbaff8d0e5b169ff13512f188332cc7ed9a
Reviewed-on: https://go-review.googlesource.com/73250
Run-TryBot: Russ Cox <rsc@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This CL expands the job of "dist bootstrap" to be "finish make.bash".
I need to change that logic in upcoming CLs related to cmd/go
changes, and I'd rather not change it in three places in three different
shell script languages.
Change-Id: I545dc215e408289e4d0b28f7c2ffcd849d89ad3b
Reviewed-on: https://go-review.googlesource.com/72870
Reviewed-by: David Crawshaw <crawshaw@golang.org>
There is some stuff I don't understand very well involved in SSUB, better words
for the documentation gratefully accepted.
As this is the last use of a bit in SMASK, kill that off too.
Change-Id: Iddff1c9b2af02c9dfb12ac8e668d004e4642f997
Reviewed-on: https://go-review.googlesource.com/42026
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Eliminates lots of ad hoc code for recognizing the same thing in
different ways.
Passes toolstash-check.
Change-Id: Ic0bb005308e96331b4ef30f455b860e476725b61
Reviewed-on: https://go-review.googlesource.com/73190
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Instead of the hand-written control flow analysis in debug info
generation, use a reverse postorder traversal, which is basically the
same thing. It should be slightly faster.
More importantly, the previous version simply gave up in the case of
non-reducible functions, and produced output that caused a later stage
to crash. It turns out that there's a non-reducible function in
compress/flate, so that wasn't a theoretical issue.
With this change, all blocks will be visited, even for non-reducible
functions.
Change-Id: Id47536764ee93203c6b4105a1a3013fe3265aa12
Reviewed-on: https://go-review.googlesource.com/73110
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Fix a bug introduced in patch 2 of
https://go-review.googlesource.com/72630 (sense of a map
lookup test was accidentally flipped).
Change-Id: Icc6096ee50be4605fa7542b9fd855c13b8aff090
Reviewed-on: https://go-review.googlesource.com/72850
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change modifies go to create iOS arm64 binaries that pass iTunes
upload validation. Tested with xcode 9.0.1 macOS 10.13.
Fixes#22402.
Change-Id: I3f14c6ac85065e2da88d06edc8682947f6f1cd47
Reviewed-on: https://go-review.googlesource.com/72730
Reviewed-by: David Crawshaw <crawshaw@golang.org>
If n.Type==nil after typechecking, then we should have already
reported a more useful error somewhere else. Just return 0 in
evalunsafe without trying to do anything else that's likely to cause
problems.
Also, further split out issue7525.go into more test files, because
cmd/compile reports at most one typechecking loop per compilation
unit.
Fixes#22351.
Change-Id: I3ebf505f72c48fcbfef5ec915606224406026597
Reviewed-on: https://go-review.googlesource.com/72251
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Robert Griesemer <gri@golang.org>
When enhanced DWARF location list generation is enabled (via internal
option -dwarflocationlists), variable entries were missing for "large"
(non-decomposable) locals and formals. From the debugging perspective,
this makes it appear that the variable doesn't exist, which is
probably not what we want. This change insures that a formal/local DIE
is created for these vars (with correct type, line, etc) but with a
conservative ("no info") location.
Change-Id: I10b2e9a51a60c7b4c748e987cdec5f2d8b2837d5
Reviewed-on: https://go-review.googlesource.com/72630
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The addressing mode of global variable was missing, whereas the
compiler may make use of it, causing "illegal combination" error.
This CL adds support of that addressing mode.
Fixes#22390.
Change-Id: Ic8eade31aba73e6fb895f758ee7f277f8f1832ef
Reviewed-on: https://go-review.googlesource.com/72610
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
* replace a copy of IsMethod with a call of it.
* a few more switches where they simplify the code.
* prefer composite literals over "n := new(...); n.x = y; ...".
* use defers to get rid of three goto labels.
* rewrite updateHasCall into two funcs to remove gotos.
Passes toolstash-check on std cmd.
Change-Id: Icb5442a89a87319ef4b640bbc5faebf41b193ef1
Reviewed-on: https://go-review.googlesource.com/72070
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Currently, benchmarking compile performance under -l=4 is confounded
by -l=2 enabling eager typechecking of unused inline function bodies
for debugging. This isn't logically an "inlining aggressiveness"
level, so instead move this logic under the -d umbrella flag.
Change-Id: I713f68952efbe25b6941d3ebc2f3707ccbbd6240
Reviewed-on: https://go-review.googlesource.com/72253
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
For #22095
Change-Id: Idcfdfe8a94db8626392658bb93429454238f648a
Reviewed-on: https://go-review.googlesource.com/70835
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
For #22095
Change-Id: Iba3dffc782cecc15ea0e90a971a2734729984945
Reviewed-on: https://go-review.googlesource.com/70834
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
For #22095
Change-Id: Ica6b3391541fe5a0355620d7c4a5107cf53eee82
Reviewed-on: https://go-review.googlesource.com/70833
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Since inlining budget calculation is fixed in CL 70151
runtime.nextFreeFast is no longer inlineable on MIPS64x because
it does not support Ctz64 as intrinsic. Skip the test.
Updates #22239.
Change-Id: Id00d55628ddb4b48d27aebfa10377a896765d569
Reviewed-on: https://go-review.googlesource.com/72271
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This hack existed because cmd/go used to install (write) and then run
cmd/cgo in the same invocation, and writing and then running a program
is a no-no in modern multithreaded Unix programs (see #22315).
As of CL 68338, cmd/go no longer installs any programs that it then
tries to use. It never did this for any program other than cgo, and
CL 68338 removed that special case for cgo.
Now this special case, added for #3001 long ago, can be removed too.
Change-Id: I338f1f8665e9aca823e33ef7dda9d19f665e4281
Reviewed-on: https://go-review.googlesource.com/71571
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It uses the build ID, which is soon to be internal to package work.
Luckily it is also only called from package work.
Change-Id: I5e6662cfe667bdc9190f086be733105ad65a3191
Reviewed-on: https://go-review.googlesource.com/70670
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Where GCC says "unrecognized command line option", clang says "unknown
argument". This distinction usually doesn't matter because the
compiler will also exit with a non-zero status, but clang 3.4
reportedly exits with a zero status after reporting an unknown argument.
Change-Id: Ieb69ea352a8de0cd4171a1c26708dfe523421cfa
Reviewed-on: https://go-review.googlesource.com/72151
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
When traceviewer encounters a failure of json trace import
due to data error, onImportFail tried to access an error variable
which was not yet defined.
Change-Id: I431be03f179aafacaf1fd3c62a6337e8b5bd18fb
Reviewed-on: https://go-review.googlesource.com/71970
Reviewed-by: Austin Clements <austin@google.com>
The test already contained logic to do this however it did not match
the error "cannot find 'ld'" which appears to be how gcc fails when
ld.gold is missing.
Fixes#22340.
Change-Id: I841248cc489b8fa72bc00a95000ad405f9ef8a4f
Reviewed-on: https://go-review.googlesource.com/72111
Run-TryBot: Michael Munday <mike.munday@ibm.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
racewalk's "foreach" function applies a function to all of a Node's
immediate children, but with a non-idiomatic signature.
This CL reworks it to recursively iterate over the entire subtree
rooted at Node and provides a way to short-circuit iteration.
Passes toolstash -cmp for std cmd with -race.
Change-Id: I738b73953d608709802c97945b7e0f4e4940d3f4
Reviewed-on: https://go-review.googlesource.com/71911
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
On modern Unix systems it is basically impossible for a multithreaded
program to open a binary for write, close it, and then fork+exec that
same binary. So don't write the binary if we're going to fork+exec it.
This fixes the ETXTBSY flakes.
Fixes#22220.
See also #22315.
Change-Id: I6be4802fa174726ef2a93d5b2f09f708da897cdb
Reviewed-on: https://go-review.googlesource.com/71570
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When building test binaries, we build one archive with all of the test
sources and a second archive with the generated test package main and
link them together. If the test sources are themselves in package main
and the test was compiled with non-default compiler flags, then both
archives will contain a go.cuinfo.producer.main symbol, leading to a
duplicate symbol failure.
This has been causing test build failures on darwin-arm-a1428ios,
darwin-arm64-a1549ios, linux-amd64-noopt, android-arm-wiko-fever, and
android-arm64-wiko-fever since CL 71430 added this symbol. This CL
should fix the build.
Change-Id: I69051c846e7c0d97395a865a361cae07f411f9ad
Reviewed-on: https://go-review.googlesource.com/71771
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 70310 dropped TODO while moving code.
Add TODO back, so we do not forget.
Change-Id: I3599ac02743bd35fb9556fdc238e9c72cf7f718f
Reviewed-on: https://go-review.googlesource.com/71590
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This adds a whitelisted subset of compiler flags to the DW_AT_producer
DWARF attribute of each package compilation unit DIE. This is common
practice in DWARF and can help debuggers determine the quality of the
produced debugging information.
Fixes#22168.
Change-Id: I1b994ef2262aa9b88b68eb6e883695d1103acc58
Reviewed-on: https://go-review.googlesource.com/71430
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Set DW_AT_variable_parameter on DW_TAG_formal_parameters that are
actually return values. variable_parameter is supposed to indicate inout
parameters, but Go doesn't really have those, and DWARF doesn't have
explicit support for multiple return values. This seems to be the best
compromise, especially since the implementation of the two is very
similar -- both are stack slots.
Fixes#21100
Change-Id: Icebabc92b7b397e0aa00a7237478cce84ad1a670
Reviewed-on: https://go-review.googlesource.com/71670
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
More generally I'm concerned about these tests using
$GOROOT/src/cmd/go as scratch space, especially
combined wtih tg.parallel() - it's easy to believe some other
test might inadvertently also try to write x.exe about the
same time. This CL only solves the "didn't clean up x.exe"
problem and leaves for another day the "probably shouldn't
write to cmd/go at all" problem.
Fixes#22266.
Change-Id: I651534d70e2d360138e0373fb4a316081872550b
Reviewed-on: https://go-review.googlesource.com/71410
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fixed an error that occurred in atomic mode. cover adds a global
variable declaration that forces sync/atomic to be used. fixDirectives
was confused by this declaration since it has an invalid
position. These declarations are now skipped.
Fixes#22309
Change-Id: I84f5fec13ef847fca35ad49f7704fb93b60503e0
Reviewed-on: https://go-review.googlesource.com/71351
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
When an opening "{" of a block is missing and after advancing we
find a closing "}", it's likely better to assume the end of the
block. Fixed and removed TODO.
Change-Id: I20c9b4ecca798933a7cd4cbf21185bd4ca04f5f7
Reviewed-on: https://go-review.googlesource.com/71291
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Current suffix check is based on instruction, which is not very
accurate. For example, "MOVW.S R1, R2" is valid, but
"MOVW.S $0xaaaaaaaa, R1" and "MOVW.P CPSR, R9" are not.
This patch fixes the above kinds of issues by checking suffix
based on []optab. And also more test cases are added.
fixes#20509
Change-Id: Ibad91be72c78eefa719412a83b4d44370d2202a8
Reviewed-on: https://go-review.googlesource.com/70910
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Unify the 386 entry point code as much as possible.
The main function could not be unified because on Windows 386 it is
called _main. Putting main in asm_386.s caused multiple definition
errors when using the external linker.
Add the _lib entry point to various operating systems. A future CL
will enable c-archive/c-shared mode for those targets.
Fix _rt0_386_windows_lib_go--it was passing arguments as though it
were amd64.
Change-Id: Ic73f1c95cdbcbea87f633f4a29bbc218a5db4f58
Reviewed-on: https://go-review.googlesource.com/70530
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
- checking for the correct closing token leads to slightly better
behavior for some randomly bogus programs
- removed `switch` in favor of an `if` statement
Follow-up on https://go-review.googlesource.com/c/go/+/71250.
Change-Id: I47f6c47b43baf790907f55ed97a947661687a9db
Reviewed-on: https://go-review.googlesource.com/71252
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Fine-tune skipping of tokens after missing closing parentheses in lists.
Fixes#22164.
Change-Id: I575d86e21048cd40340a2c08399e8b0deec337cf
Reviewed-on: https://go-review.googlesource.com/71250
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Previously, cover printed directives (//go: comments) near the top of
the file unless they were in doc comments. However, directives
frequently apply to specific definitions, and they are not written in
doc comments to prevent godoc from printing them. Moving all
directives to the top of the file affected semantics of tests.
With this change, directives are kept together with the following
top-level declarations. Only directives that occur after all top-level
declarations are moved.
Fixes#22022
Change-Id: Ic5c61c4d3969996e4ed5abccba0989163789254c
Reviewed-on: https://go-review.googlesource.com/69630
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Use them to replace if/else chains with at least three comparisons,
where the code becomes clearly simpler.
Passes toolstash -cmp on std cmd.
Change-Id: Ic98aa3905944ddcab5aef5f9d9ba376853263d94
Reviewed-on: https://go-review.googlesource.com/70934
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The write barrier insertion has moved to the SSA backend's
writebarrier pass. There is still needwritebarrier function
left in the frontend. This function is used in two places:
- fncall, which is called in ascompatet, which is called in
walking OAS2FUNC. For OAS2FUNC, in order pass we've already
created temporaries, and there is no write barrier for the
assignments of these temporaries.
- updateHasCall, which updates the HasCall flag of a node. the
HasCall flag is then used in
- fncall, mentioned above.
- ascompatet. As mentioned above, this is an assignment to
a temporary, no write barrier.
- reorder1, which is always called with a list produced by
ascompatte, which is a list of assignments to stack, which
have no write barrier.
- vmatch1, which is called in oaslit with r.Op as OSTRUCTLIT,
OARRAYLIT, OSLICELIT, or OMAPLIT. There is no write barrier
in those literals.
Therefore, the needwritebarrier function is unnecessary. This
CL removes it.
Passes "toolstash -cmp" on std cmd.
Updates #17583.
Change-Id: I4b87ba8363d6583e4282a9e607a9ec8ce3ab124a
Reviewed-on: https://go-review.googlesource.com/43640
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
On ARM, STREX does not permit the same register used as both the
source and the destination. Reject the bad instruction.
The assembler also accepted special cases
STREX R0, (R1) as STREX R0, (R1), R0
STREX (R1), R0 as STREX R0, (R1), R0
both are illegal. Remove this special case as well.
For STREXD, check that the destination is not source, and not
source+1. Also check that the source register is even numbered,
as required by the architecture's manual.
Fixes#22268.
Change-Id: I6bfde86ae692d8f1d35bd0bd7aac0f8a11ce8e22
Reviewed-on: https://go-review.googlesource.com/71190
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
In golang.org/cl/61130, I removed the need for setting Xoffset on
OXCASE Nodes, but missed this assignment.
Passes toolstash-check.
Change-Id: I90ab05add14981b89ee18e73e1cdf2f13e9f9934
Reviewed-on: https://go-review.googlesource.com/66934
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Instead of repeating the same list parsing pattern for parenthesized
of braced comma or semicolon-separated lists, introduce a single list
parsing function that can be parametrized and which takes a closure
to parse list elements.
This ensures the same error handling and recovery logic is used across
all lists and simplifies the code.
No semantic change.
Change-Id: Ia738d354d6c2e0c3d84a5f1c7269a6eb95685edc
Reviewed-on: https://go-review.googlesource.com/70492
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
No semantic change. Move functionality not related to argument
out of the argument parsing function, and thus match parameter
parsing. Also, use a better function name.
Change-Id: Ic550875251d64e6fe1ebf91c11d33a9e4aec9fdd
Reviewed-on: https://go-review.googlesource.com/70491
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is much easier than replacing SSUB so split it out from my other CL.
Change-Id: If01e4005da5355895404456320a2156bde4ec09a
Reviewed-on: https://go-review.googlesource.com/71050
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is https://go-review.googlesource.com/42025 but with some more fixes --
hidden symbols implicitly passed "Type == 0 || Type == SXREF" checks. (This
sort of thing is part of why I wanted to make this change)
Change-Id: I2273ee98570fd7f2dd8a799c692a2083c014235e
Reviewed-on: https://go-review.googlesource.com/42330
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reduce the scope of some. Also remove vars that were simply the index or
the value in a range statement. While at it, remove a var that was
exactly the length of a slice.
Also replaced 'bad' with a more clear 'errored' of type bool, and
renamed a single-char name with a comment to a name that is
self-explanatory.
And removed a few unnecessary Index calls within loops.
Passes toolstash -cmp on std cmd.
Change-Id: I26eee5f04e8f7e5418e43e25dca34f89cca5c80a
Reviewed-on: https://go-review.googlesource.com/70930
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
After we detect errors, the AST is in a precarious state and more
likely to trip useless ICE failures. Instead let the user fix any
existing errors and see if the ICE persists. This makes Fatalf more
consistent with how panics are handled by hidePanic.
While here, also fix detection for release versions: release version
strings begin with "go" ("go1.8", "go1.9.1", etc), not "release".
Fixes#22252.
Change-Id: I1c400af62fb49dd979b96e1bf0fb295a81c8b336
Reviewed-on: https://go-review.googlesource.com/70850
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The caller's PC is always available in the frame. We can just
load it when needed, no need to spill.
Change-Id: I9c0a525903e574bb4eec9fe53cbeb8c64321166a
Reviewed-on: https://go-review.googlesource.com/70710
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
CL 69370 introduced a hasmain field to moduledata after the
modulehashes slice. However that code was relying on the zeroing
code after it to cover modulehashes if len(Shlibs) == 0. The
hasmain field gets in the way of that. So clear modulehashes
explicitly in that case.
Found when looking at #22250. Not sure if it's related.
Change-Id: I81050cb4554cd49e9f245d261ef422f97d026df4
Reviewed-on: https://go-review.googlesource.com/70730
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Some ARM64-specific instructions (such as SIMD instructions) are not supported.
This patch adds support for the following:
1. Extended register, e.g.:
ADD Rm.<ext>[<<amount], Rn, Rd
<ext> can have the following values:
UXTB, UXTH, UXTW, UXTX, SXTB, SXTH, SXTW and SXTX
2. Arrangement for SIMD instructions, e.g.:
VADDP Vm.<T>, Vn.<T>, Vd.<T>
<T> can have the following values:
B8, B16, H4, H8, S2, S4 and D2
3. Width specifier and element index for SIMD instructions, e.g.:
VMOV Vn.<T>[index], Rd // MOV(to general register)
<T> can have the following values:
S and D
4. Register List, e.g.:
VLD1 (Rn), [Vt1.<T>, Vt2.<T>, Vt3.<T>]
5. Register offset variant, e.g.:
VLD1.P (Rn)(Rm), [Vt1.<T>, Vt2.<T>] // Rm is the post-index register
6. Go assembly for ARM64 reference manual
new added instructions are required to have according explanation items in
the manual and items for existed instructions will be added incrementally
For more information about the refinement background, please refer to the
discussion (https://groups.google.com/forum/#!topic/golang-dev/rWgDxCrL4GU)
This patch only adds syntax and doesn't break any assembly that already exists.
Change-Id: I34e90b7faae032820593a0e417022c354a882008
Reviewed-on: https://go-review.googlesource.com/41654
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Excluded when -short because it still runs relatively long,
but deflaked.
Removed timeouts from normal path and ensured that they were
not needed and that reference files did not change.
Use "tbreak" instead of "break" with gdb to reduce chance
of multiple hits on main.main. (Seems not enough, but a
move in the right direction).
By default, testing ignores repeated lines that occur when
nexting. This appears to sometimes be timing-dependent and
is the observed source of flakiness in testing so far.
Note that these can also be signs of a bug in the generated
debugging output, but it is one of the less-confusing bugs
that can occur.
By default, testing with gdb uses compilation with
inlining disabled to prevent dependence on library code
(it's a bug that library code is seen while Nexting, but
the bug is current behavior).
Also by default exclude all source files outside /testdata
to prevent accidental dependence on library code. Note that
this is currently only applicable to dlv because (for the
debugging information we produce) gdb does not indicate a
change in the source file for inlined code.
Added flags -i and -r to make gdb testing compile with
inlining and be sensitive to repeats in the next stream.
This is for developer-testing and so we can describe these
problems in bug reports.
Updates #22206.
Change-Id: I9a30ebbc65aa0153fe77b1858cf19743bdc985e4
Reviewed-on: https://go-review.googlesource.com/69930
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently we look to see if the main.main symbol address is in the
module data text range. This requires access to the main.main
symbol, which usually the runtime has, but does not when building
a plugin.
To avoid a dynamic relocation to main.main (which I haven't worked
out how to have the linker generate on darwin), stop using the
symbol. Instead record a boolean in the moduledata if the module
has the main function.
Fixes#22175
Change-Id: If313a118f17ab499d0a760bbc2519771ed654530
Reviewed-on: https://go-review.googlesource.com/69370
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
For #22095
Change-Id: I8f48fce571b69a7e8edf2ad7733ffdfd38676e63
Reviewed-on: https://go-review.googlesource.com/70310
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
These functions are identical to math/bits.TrailingZeros{32,64}, which
are already intrinsified on ppc64.
Change-Id: If7ee57e7afe53154874f4b66bacdb6237806128a
Reviewed-on: https://go-review.googlesource.com/70350
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
When we split separate packages into separate compilation units, we
lost PC range information because it was no longer contiguous. This
brings it back by constructing proper per-package PC range tables.
Change-Id: Id0ab5187e08ac5d13b3d3794977bfc857a56224f
Reviewed-on: https://go-review.googlesource.com/69974
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Currently, the linker generates one huge DWARF compilation unit for
the entire Go binary. This commit creates a separate compilation unit
and line table per Go package.
We temporarily lose compilation unit PC range information, since it's
now discontiguous, so harder to emit. We'll bring it back in the next
commit.
Beyond being "more traditional", this has various technical
advantages:
* It should speed up line table lookup, since that requires a
sequential scan of the line table. With this change, a debugger can
first locate the per-package line table and then scan only that line
table.
* Once we emit compilation unit PC ranges again, this should also
speed up various other debugger reverse PC lookups.
* It puts us in a good position to move more DWARF generation into the
compiler, which could produce at least the CU header, per-function
line table fragments, and per-function frame unwinding info that the
linker could simply paste together.
* It will let us record a per-package compiler command-line flags
(#22168).
Change-Id: Ibac642890984636b3ef1d4b37fe97f4453c2cc84
Reviewed-on: https://go-review.googlesource.com/69973
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The DWARF code currently clears all section relocations every time it
creates a section. This is unnecessary and confusing, so don't do it.
This dates back to
https://codereview.appspot.com/7891044/diff/26001/src/cmd/ld/dwarf.c.
At the time, this was only done for one symbol and that symbol was
used solely for collecting relocations (which is why it made sense to
clear the relocations but not the actual data). Furthermore, DWARF
generation potentially required two passes, so it was important to
clear the state from the first pass. None of this is true now, but
this pattern had been cargo-culted all over the dwarf.go.
Change-Id: I87d4ff8ccd5c807796241559be46168ce3ccb49a
Reviewed-on: https://go-review.googlesource.com/70312
Run-TryBot: Austin Clements <austin@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The .debug_aranges section is an odd vestige of DWARF, since its
contents are easy and efficient for a debugger to reconstruct from the
attributes of the top-level compilation unit DIEs. Neither GCC nor
clang emit it by default these days. GDB and Delve ignore it entirely.
LLDB will use it if present, but is happy to construct the index from
the compilation unit attributes (and, indeed, a remarkable variety of
other ways if those aren't available either).
We're about to split up the compilation units by package, which means
they'll have discontiguous PC ranges, which is going to make
.debug_aranges harder to construct (and larger).
Rather than try to maintain this essentially unused code, let's
simplify things and remove it.
Change-Id: I8e0ccc033b583b5b8908cbb2c879b2f2d5f9a50b
Reviewed-on: https://go-review.googlesource.com/69972
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
During Mach-O linking, dsymutil takes the DWARF from individual object
files and combines it into a debug archive. Because it's content-aware,
it doesn't need our help to do its job. Nonetheless, it does try to
honor relocations that are present in its input.
When dsymutil encounters a relocation, it uses the value of that
relocation as an index into the debug map to find its final location.
When it does that, it's assuming that the value is an address in the
object file. But DWARF references are section-relative. So when it
processes a relocation for a DWARF reference, it gets confused,
and if the value happens to match the address of a function or
data symbol, it will rewrite it incorrectly.
Since the relocations don't help, and can hurt, drop them when
externally linking a Mach-O binary.
Fixes#22068
Change-Id: I8ec36da626575d9f6c8d0e7a0b76eab8ba22d62c
Reviewed-on: https://go-review.googlesource.com/68330
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The two are not meaningfully different, and it is confusing to have two.
Change-Id: Ie6a355ea4d79fb4bb79bf5124071a866038b19ba
Reviewed-on: https://go-review.googlesource.com/70211
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Previously, we were treating cross-package function calls as free for
inlining budgeting.
In theory, we should be able to recompute InlCost from the
exported/reimported function bodies. However, that process mutates the
structure of the Node AST enough that it doesn't preserve InlCost. To
avoid unexpected issues, just record and restore InlCost in the export
data.
Fixes#19261.
Change-Id: Iac2bc0d32d4f948b64524aca657051f9fc96d92d
Reviewed-on: https://go-review.googlesource.com/70151
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Noticed while reading some code that the two branches in this loop body
shared the last statements. Rewrite it in a way that they are not
duplicated.
Passes toolstash -cmp on std.
Change-Id: I3356ca9fa37c32eee496e221d7830bfc581dade1
Reviewed-on: https://go-review.googlesource.com/66470
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Calls to a closure held in a local, non-escaping,
variable can be inlined, provided the closure body
can be inlined and the variable is never written to.
The current implementation has the following limitations:
- closures with captured variables are not inlined because
doing so naively triggers invariant violation in the SSA
phase
- re-assignment check is currently approximated by checking
the Addrtaken property of the variable which should be safe
but may miss optimization opportunities if the address is
not used for a write before the invocation
Updates #15561
Change-Id: I508cad5d28f027bd7e933b1f793c14dcfef8b5a1
Reviewed-on: https://go-review.googlesource.com/65071
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hugues Bruant <hugues.bruant@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
In order to improve the line numbering for debuggers,
it's necessary to trace lines through compilation.
This makes it (much) easier to follow.
The format of the last column of the ssa.html output was
also changed to reduce the spamminess of the file name,
which is usually the same and makes it far harder to read
instructions and line numbers, and to make it wider and also
able to break words when wrapping (long path names still
can push off the end otherwise; side-to-side scrolling was
tried but was more annoying than the occasional wrapped
line).
Sample output now, where [...] is elision for sake of making
the CL character-counter happy -- and the (##) line numbers
are rendered in italics and a smaller font (11 point) under
control of a CSS class "line-number".
genssa
# /Users/drchase/[...]/ssa/testdata/hist.go
00000 (35) TEXT "".main(SB)
00001 (35) FUNCDATA $0, gclocals·7be4bb[...]1e8b(SB)
00002 (35) FUNCDATA $1, gclocals·9ab98a[...]4568(SB)
v920 00003 (36) LEAQ ""..autotmp_31-640(SP), DI
v858 00004 (36) XORPS X0, X0
v6 00005 (36) LEAQ -48(DI), DI
v6 00006 (36) DUFFZERO $277
v576 00007 (36) LEAQ ""..autotmp_31-640(SP), AX
v10 00008 (36) TESTB AX, (AX)
b1 00009 (36) JMP 10
and from an earlier phase:
b18: ← b17
v242 (47) = Copy <mem> v238
v243 (47) = VarKill <mem> {.autotmp_16} v242
v244 (48) = Addr <**bufio.Scanner> {scanner} v2
v245 (48) = Load <*bufio.Scanner> v244 v243
[...]
v279 (49) = Store <mem> {int64} v277 v276 v278
v280 (49) = Addr <*error> {.autotmp_18} v2
v281 (49) = Load <error> v280 v279
v282 (49) = Addr <*error> {err} v2
v283 (49) = VarDef <mem> {err} v279
v284 (49) = Store <mem> {error} v282 v281 v283
v285 (47) = VarKill <mem> {.autotmp_18} v284
v286 (47) = VarKill <mem> {.autotmp_17} v285
v287 (50) = Addr <*error> {err} v2
v288 (50) = Load <error> v287 v286
v289 (50) = NeqInter <bool> v288 v51
If v289 → b21 b22 (line 50)
Change-Id: I3f46310918f965761f59e6f03ea53067237c28a8
Reviewed-on: https://go-review.googlesource.com/69591
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Along the way, switch to using relocation constants from debug/elf.
For #22095
Change-Id: I1a64353619f95dde5aa39060c4b9d001af7dc1e4
Reviewed-on: https://go-review.googlesource.com/69013
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
There are two places in DWARF generation that create symbols when they
really just want to get the symbol if it exists. writeranges, in
particular, will create a DWARF range symbol for every single textp
symbol (though they won't get linked into any list, so they don't
affect the binary).
Fix these to use ROLookup instead of Lookup.
Change-Id: I401eadf22890e296bd08bccaa6ba2fd8fac800cd
Reviewed-on: https://go-review.googlesource.com/69971
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
On Windows, not closing f keeps us from being able to remove it.
Change-Id: Id4cb709b6ce0b30485b87364a9f0e6e71d2782bd
Reviewed-on: https://go-review.googlesource.com/70070
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It looks like I forgot to reenable this test when I fixed#21522.
Update deps.go and reenable.
Change-Id: I68a45df09b418f48d93d2e7ab1d274e056c192e6
Reviewed-on: https://go-review.googlesource.com/70050
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The content ID will be needed for content-based staleness
determination. It is defined as the SHA256 hash of the file
in which it appears, with occurrences of the build+content IDs
changed to zeros during the hashing operation.
Storing the content ID in the archives is a little tricky
but it means that later builds need not rehash the archives
each time they are referenced, so under the assumption
that each package is imported at least once after being
compiled, hashing at build time is a win. (Also the whole
file is more likely to be in cache at build time,
since we just wrote it.)
In my unscientific tests, the time for "go build -a std cmd"
rises from about 14.3s to 14.5s on my laptop, or under 2%.
Change-Id: Ia3d4dc657d003e8295631f73363868bd92ebf96a
Reviewed-on: https://go-review.googlesource.com/69054
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The liveness analysis no longer directly emits PCDATA. Fix stale
comments that say so.
Change-Id: Id26b112ddf4c13a12ebf766f64bf57c68fbfe3ef
Reviewed-on: https://go-review.googlesource.com/67691
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
If the stack frame is too large, abort immediately.
We used to generate code first, then abort.
In issue 22200, generating code raised a panic
so we got an ICE instead of an error message.
Change the max frame size to 1GB (from 2GB).
Stack frames between 1.1GB and 2GB didn't used to work anyway,
the pcln table generation would have failed and generated an ICE.
Fixes#22200
Change-Id: I1d918ab27ba6ebf5c87ec65d1bccf973f8c8541e
Reviewed-on: https://go-review.googlesource.com/69810
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This CL does a few things.
1. It moves the existing "read a build ID" code out of the go command
and into cmd/internal/buildid.
2. It adds new code there to "write a build ID".
3. It adds better tests.
4. It encapsulates cmd/internal/buildid into a new standalone program
"go tool buildid".
The go command is going to use the new "write a build ID" functionality
in a future CL. Adding the separate "go tool buildid" gives "go build -x"
a printable command to explain what it is doing in that new step.
(This is similar to the go command printing "go tool pack" commands
equivalent to the actions it is taking, even though it's not invoking pack
directly.) Keeping go build -x honest means that other build systems can
potentially keep up with the go command.
Change-Id: I01c0a66e30a80fa7254e3f2879283d3cd7aa03b4
Reviewed-on: https://go-review.googlesource.com/69053
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This makes it more obvious which of the two builds is failing by
putting "dbg" or "opt" directly in the test name. It also makes it
possible for them to fail independently, so a failure in "dbg" doesn't
mask a failure in "opt", and to visibly skip the opt test when run
with an unoptimized runtime.
Change-Id: I3403a7fd3c1a13ad51a938bb95dfe54c320bb58e
Reviewed-on: https://go-review.googlesource.com/69970
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Every cmd/thing is 'go tool thing' except for go and gofmt.
But the table in cmd/go enumerates all the things instead of
saying that go and gofmt are the exceptions.
Change that, so that when adding new tools it's not
necessary to update this table.
Change-Id: Ia6fef41b4d967249b19971a0d03e5acb0317ea82
Reviewed-on: https://go-review.googlesource.com/69052
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Today they happen during the build phase; they should happen
during the load phase instead, along with the C check.
Change-Id: I6074a995b8e29275549aafa574511b735642d85b
Reviewed-on: https://go-review.googlesource.com/69051
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Logically the build needs to start treating a.Package as immutable,
since we might want to build a.Package multiple ways.
Record the built target in a.built instead.
Right now a.built is predictable ahead of time, but we want to
move toward satisfying some builds from a cache directory,
in which case a.built will point into the cache directory
and not be determined until action execution time.
There is probably more to do with shared libraries, but this
does not break what's there.
Change-Id: I941988b520bee2f664fd8cabccf389e1dc29628b
Reviewed-on: https://go-review.googlesource.com/69050
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Everything got a bit tangled together in b.action1.
Try to tease things apart again.
In general this is a refactoring of the existing code, with limited
changes to the effect of the code.
The main additional change is to complete a.Deps for link actions.
That list now directly contains all the inputs the linker will attempt
to read, eliminating the need for a transitive traversal of the entire
action graph to find those. The comepleteness of a.Deps will be
important when we eventually use it to decide whether an cached
action output can be reused.
all.bash passes, but it's possible I've broken some subtety of
buildmode=shared again. Certainly that code took the longest
to get working.
Change-Id: I34e849eda446dca45a9cfce02b07bec6edb6d0d4
Reviewed-on: https://go-review.googlesource.com/69831
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
To the extent that invoking the compiler and invoking the linker
have different dependency requirements, representing both steps
by a single action node leads to confusion.
If we move to having separate .a and .x (import metadata) files
in the standard builds, then the .a is a link dependency but not
a compile dependency, and vice versa for .x.
Today, in shared library builds, the .a is a compile dependency
and a link dependency, while the .so is only a link dependency.
Also in this CL: change the gccgo link step to extract _cgo_flags
into root.Objdir, which is private to the link step, instead of into
b.WorkDir, which is shared by all the link steps that could possibly
be running in parallel. And attempt to handle the -n and -x flags
when loading _cgo_flags, instead of dying attempting to read
an archive that wasn't written.
Also in this CL: create a.Objdir before running a.Func, to avoid
duplicating the Mkdir(a.Objdir) in every a.Func.
A future CL will update the link action's Deps to be accurate.
(Right now the link steps search out the true Deps by walking
the entire action graph.)
Change-Id: I15128ce2bd064887f98abc3a4cf204241f518631
Reviewed-on: https://go-review.googlesource.com/69830
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This error was not used when using git because nested git is permitted.
Add test using Mercurial, so that at least we have a test, even though
the test is not run by default.
Fixes#22157Fixes#22201
Change-Id: If521f3c09b0754e00e56fa3cd0364764a57a43ad
Reviewed-on: https://go-review.googlesource.com/69670
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Focused on ranges, selects and switches for this one.
While at it, simplify some vars in typecheckselect.
Updates #19683.
Change-Id: Ib6aabe0f6826cb1930483aeb4bb2de1ff8052d9e
Reviewed-on: https://go-review.googlesource.com/69690
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
OTYPESW Op comment says
// List = Left.(type)
this seems to be wrong. Adding
fmt.Printf("n: %v\n", cond)
fmt.Printf(" n.List: %v\n", cond.List)
fmt.Printf(" n.Left: %v\n", cond.Left)
fmt.Printf(" n.Right: %v\n", cond.Right)
to (s *typeSwitch) walk(sw *Node), and compiling the following code
snippet
var y interface{}
switch x := y.(type) {
default:
println(x)
}
prints
n: <node TYPESW>
n.List:
n.Left: x
n.Right: y
The correct OTYPESW Node field positions are
// Left = Right.(type)
This is confirmed by the fact that, further in the code,
typeSwitch.walk() checks that Right (and not Left) is of type
interface:
cond.Right = walkexpr(cond.Right, &sw.Ninit)
if !cond.Right.Type.IsInterface() {
yyerror("type switch must be on an interface")
return
}
This patch fixes the OTYPESW comment.
Change-Id: Ief1e409cfabb7640d7f7b0d4faabbcffaf605450
Reviewed-on: https://go-review.googlesource.com/69112
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This re-enables functionality that inadvertently was disabled in the
(long) past.
Also, don't perform branch checks if we had errors in a function
to avoid spurious errors or (worst-case) crashes.
Slightly modified test/fixedbugs/issue14006.go to make sure the
test still reports invalid label errors (the surrounding function
must be syntactically correct).
Change-Id: Id5642930877d7cf3400649094ec75c753b5084b7
Reviewed-on: https://go-review.googlesource.com/69770
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change fixes the implementation of Data Cache instructions for
ppc64x, allowing non-zero hint field values.
Change-Id: I454aac9293d069a4817ee574d5809fa1799b3216
Reviewed-on: https://go-review.googlesource.com/68670
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
mkinlcall1 already guards against recursively inlining functions into
themselves, so there's no need to clear and restore fn.Func.Inl during
recursive inlining.
Passes toolstash-check.
Change-Id: I8bf0c8dea8788d94d3ea5670610b4acb1d26d2fb
Reviewed-on: https://go-review.googlesource.com/69310
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
We've supported inlining methods called as functions for a while now.
Change-Id: I53fba426e45f91d65a38f00456c2ae1527372b50
Reviewed-on: https://go-review.googlesource.com/69530
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Before, -1 meant a node being nil or not an OLITERAL, and 0 meant an
OLITERAL missing a Val.
However, the use of this value was confusing and led to some issues,
such as swt.go checking for < 0 instead of <= 0, causing panics.
We never need to differentiate these two cases, so collapse both into 0.
To make it clear that negative values can no longer happen, make Ctype
an uint8.
With this change, we can now get rid of the two n.Type == nil checks
in swt.go added to fix a couple of these panics.
Thanks to Matthew Dempsky for spotting this inconsistency.
Fixes#22001.
Change-Id: I51c65a76f38a3e16788b6a3b57932dad3436dc7e
Reviewed-on: https://go-review.googlesource.com/69510
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
ADDconst op is no longer used for addresses, as we lower Addr to
MOVDaddr. There is no rule that produces ADDconst with a non-nil
sym. So we can remove the sym aux field in ADDconst and limit its
use for adding constant (not offset to symbol).
Passes "toolstash -cmp" on std cmd for GOARCH=ppc64 and ppc64le.
Change-Id: Icee35cdb34d8d121ad7035076dfd07595c7ff809
Reviewed-on: https://go-review.googlesource.com/69450
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Add a compiler intrinsic for getcallersp. So we are able to get
rid of the argument (not done in this CL).
Change-Id: Ic38fda1c694f918328659ab44654198fb116668d
Reviewed-on: https://go-review.googlesource.com/69350
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David Chase <drchase@google.com>
CL 68490 reworked the way the panicmem function is exposed to the
C mach expection catcher. However, //go:cgo_export_static isn't enough:
the underlying assembly functions must not start with the middle dot.
Without the middle dot, the panicmem function's exported name is
not prefixed with its package; rename it to xx_cgo_panicmem to decrease
the chance of a symbol name clash.
Finally, mark the overridden C symbol weak to avoid duplicate symbol
errors from the host linker.
For the ios builders.
Change-Id: Ib87789fecec9314e398cf1bd8c04ba0b3a6642af
Reviewed-on: https://go-review.googlesource.com/69113
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Thanks to Christopher Nelson for spearheading the effort.
Fixes#11058
Change-Id: Icafabac8dc697626ff1bd943cc577b0b1cc6b349
Reviewed-on: https://go-review.googlesource.com/69091
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This will be useful for debugging but is intentionally
undocumented and not guaranteed to persist in any
particular form.
Change-Id: I60710a1e94cfc2ce31fe91fc268c51985060f8df
Reviewed-on: https://go-review.googlesource.com/69330
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The core dump reader would like a bunch of ideal int
constants to be available in dwarf.
Makes the go binary 0.9% bigger.
Update #14517
Change-Id: I00cdfc7f53bcdc56fccba576c1d33010f03bdd95
Reviewed-on: https://go-review.googlesource.com/69270
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
CL50610 broke the build for noopt (different inlining
behavior) and clang (no gdb) so it needs to catch those
cases and skip.
The run/no-run logic was slightly cleaned up,
the name of gdb on OSX was made more robust (tries gdb
first, then ggdb), and the file names were canonicalized
before loggging instead of in comparison to reduce
gratuitous noise in diffs when things aren't otherwise
equal.
This probably doesn't fix problems on Alpine, but it should
provide a cleaner and less confusing failure.
Change-Id: I26c65bff5a8d3d60f1cd6ae02a282558c53dda67
Reviewed-on: https://go-review.googlesource.com/69371
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
The existing logic for whether the left and right parts of an assignment
were equal only checked that the gofmt representation of the two was
equal. This only checks that the ASTs were equal.
However, that method is flawed. For example, if either of the
expressions contains a function call, the expressions may actually be
different even if their ASTs are the same. An obvious case is a func
call to math/rand to get a random integer, such as the one added in the
test.
If either of the expressions may have side effects, simply skip the
check. Reuse the logic from bool.go's hasSideEffects.
Fixes#22174.
Change-Id: Ied7f7543dc2bb8852e817230756c6d23bc801d90
Reviewed-on: https://go-review.googlesource.com/69116
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
This attempts to choose better values for values that are
rematerialized (uses the XPos of the consumer, not the
original) and for unconditional branches (uses the last
assigned XPos in the block).
The JMP branches seem to sometimes end up with a PC in the
destination block, I think because of register movement
or rematerialization that gets placed in predecessor blocks.
This may be acceptable because (eyeball-empirically) that is
often the line number of the target block, so the line number
flow is correct.
Added proper test, that checks both -N -l and regular compilation.
The test is also capable (for gdb, delve soon) of tracking
variable printing based on comments in the source code.
There's substantial room for improvement in debugger behavior.
Updates #21098.
Change-Id: I13abd48a39141583b85576a015f561065819afd0
Reviewed-on: https://go-review.googlesource.com/50610
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Move some local declarations closer to their use, reducing their
respective lifetimes, also improve few error messages.
Follow up of CL 67370.
Updates #22095
Change-Id: I6131159ae8de571015ef5459b33d5c186e543a37
Reviewed-on: https://go-review.googlesource.com/69110
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
For #22095
Change-Id: I660080279692b74669c45f42c28cccff71bd33b5
Reviewed-on: https://go-review.googlesource.com/68930
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also add -V=full to print a unique identifier of the specific tool being invoked.
This will be used for content-based staleness.
Also sort and clean up a few of the flag doc comments.
Change-Id: I786fe50be0b8e5f77af809d8d2dab721185c2abd
Reviewed-on: https://go-review.googlesource.com/68590
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
From a compact specification of control flow graphs,
generate complete set of possible assignment patterns to
output y, and also generate an interpretable specification.
Compiles (hoping for crash, or not) and then runs, where
the run checks function output against interpreted output
for various inputs observed to terminate in the interpreter.
In ssa_test.go, added ability to generate a test and run
(compile and run) the generated test, possibly with
modified environment variables. The generated test is
compiled including the -D=ssa/check/on flag, and if the
interpreter terminates in a small number of steps, then it
is also run to check the result.
Change-Id: I392c828e36c543411b7733ca0799628452733276
Reviewed-on: https://go-review.googlesource.com/22751
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Keep left-to-right order when referring to the number of
variables and values involved.
Fixes#22159.
Change-Id: Iccca12d3222f9d5e049939a9ccec07513c393faa
Reviewed-on: https://go-review.googlesource.com/68690
Reviewed-by: Russ Cox <rsc@golang.org>
"VEX.vvvv" field (VSR, VEX-specified-register) made explicit
in Optab encoding.
vexNDS, vexNDD, vexDDS and vexNOVSR do nothing,
this change does not produce any noticeable effect.
Rationale behind this change:
- keep more information inside optab entries
- make encodings match SDM more closely
- one less special rule to keep in mind
Pvex optabs are updated based on the Intel SDM descriptions.
Unused VEX combinations are removed;
it is problematic to choose VSR combinations for them
without actual Optabs that use them.
The origin of this idea can be found in:
https://go-review.googlesource.com/#/c/arch/+/66972/
Change-Id: I54634a72b44d61f4b924a1e45f2240aab7384dc2
Reviewed-on: https://go-review.googlesource.com/67890
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Adds last missing SSE4 instruction.
Also introduces additional ytab set 'yextractps'.
See https://golang.org/cl/57470 that adds other SSE4 instructions
but skips this one due to 'yextractps'.
To make EXTRACTPS less "sloppy", Yu2 oclass added to forbid
usage of invalid offset values in immediate operand.
Part of the mission to add missing amd64 SSE4 instructions to Go asm.
Change-Id: I0e67e3497054f53257dd8eb4c6268da5118b4853
Reviewed-on: https://go-review.googlesource.com/57490
TryBot-Result: Gobot Gobot <gobot@golang.org>
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
Reviewed-by: Russ Cox <rsc@golang.org>
This CL optimizes assembly for len() or cap() division
by a power of 2 constants:
func lenDiv(s []int) int {
return len(s) / 16
}
amd64 assembly before the CL:
MOVQ "".s+16(SP), AX
MOVQ AX, CX
SARQ $63, AX
SHRQ $60, AX
ADDQ CX, AX
SARQ $4, AX
MOVQ AX, "".~r1+32(SP)
RET
amd64 assembly after the CL:
MOVQ "".s+16(SP), AX
SHRQ $4, AX
MOVQ AX, "".~r1+32(SP)
RET
The CL relies on the fact that len() and cap() result cannot
be negative.
Trigger stats for the added SSA rules on linux/amd64 when running
make.bash:
46 Div64
12 Mod64
The added SSA rules may trigger on more cases in the future
when SSA values will be populated with the info on their
lower bounds.
For instance:
func f(i int16) int16 {
if i < 3 {
return -1
}
// Lower bound of i is 3 here -> i is non-negative,
// so unsigned arithmetics may be used here.
return i % 16
}
Change-Id: I8bc6be5a03e71157ced533c01416451ff6f1a7f0
Reviewed-on: https://go-review.googlesource.com/65530
Reviewed-by: Keith Randall <khr@golang.org>
Replace Buildmode with BuildMode and Linkmode with LinkMode.
For #22095
Change-Id: I51a6f5719d107727bca29ec8e68e3e9d87e31e33
Reviewed-on: https://go-review.googlesource.com/68334
Run-TryBot: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
For #22095
Change-Id: Ie9ae84c758af99ac7daed26d0b3e3b0a47599edd
Reviewed-on: https://go-review.googlesource.com/67315
Run-TryBot: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
amd64 and 386 have rules to reduce multiplication by a positive power
of two, but a more general reduction (both for positive and negative
powers of two) is already performed by generic rules that were added
in CL 36323 to replace walkmul (see lines 166:173 in generic.rules).
The x86 and amd64 rules are never triggered during all.bash and can be
removed, reducing rules duplication.
The change also adds a few code generation tests for amd64 and 386.
Change-Id: I566d48186643bd722a4c0137fe94e513b8b20e36
Reviewed-on: https://go-review.googlesource.com/68450
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
ORETJMP doesn't need an ONAME if we just set the target method on Sym
instead of Left. Conveniently, this is where fmt.go was looking for it
anyway.
Change the iface parameter and global compiling_wrappers to bool.
Passes toolstash-check.
Change-Id: I5333f8bcb4e06bf8161808041125eb95c439aafe
Reviewed-on: https://go-review.googlesource.com/68252
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Robert Griesemer <gri@golang.org>
C_PPAUTO was matching offsets that is a multiple 8. But this
condition is dropped in CL 55610, causing unaligned offset
between 256 and 504 mistakenly matched to some classes, e.g.
C_UAUTO8K. This CL restores this condition, also fixes an
error that C_PPAUTO shouldn't match C_PSAUTO, because the
latter is not guaranteed to be multiple of 8. C_PPAUTO_8 is
unnecessary, removed.
Fixes#21992.
Change-Id: I75d5a0e5f5dc3dae335721fbec1bbcd4a3b862f2
Reviewed-on: https://go-review.googlesource.com/65730
Reviewed-by: David Chase <drchase@google.com>
Combine setcc and store of result into setcc that writes directly to memory.
Triggers 200+ times in go tool.
Fixes#21630
Change-Id: Iafa22607426f4120140c88fae4b9aecb46e0bba8
Reviewed-on: https://go-review.googlesource.com/67950
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This was for cmd/cgo, and cmd/cgo isn't special anymore.
Change-Id: I71efaf553b24798b523c7102859428d36b470698
Reviewed-on: https://go-review.googlesource.com/68530
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The primary build tools cmd/asm, cmd/compile, and cmd/link are
built during cmd/dist bootstrap and then assumed by cmd/go to
be available for any future builds.
The only tool invoked by cmd/go during a build and not in this list
is cmd/cgo; instead of being built during cmd/dist and assumed by
cmd/go, cmd/go arranges to build cmd/cgo if needed as part of
the regular build. We got here because at the time cmd/go was written,
cmd/cgo was the only build tool written in Go (the others were in C),
and so it made some sense to put cmd/dist in charge of building
the C tools and to have custom code in cmd/go to build cmd/cgo
just in time for it to be used by a particular build.
This custom code has historically been quite subtle, though, because
the build of cmd/cgo inherits whatever build flags apply to the
build that wants to use cmd/cgo. If you're not careful,
"go install -race strings" might under the wrong circumstances
also install a race-enabled cmd/cgo binary, which is unexpected
at the least.
The custom code is only going to get more problematic as we
move toward more precise analysis of whether dependencies are
up-to-date. In that case, "go build -race strings" will check to
see if there is not just a cmd/cgo already but a race-enabled
cmd/cgo, which makes no sense.
Instead of perpetuating the special case, treat cgo like all the
other build tools: build it first in cmd/dist, and then assume it is present.
This simplifies cmd/go.
Building cmd/cgo during bootstrap also allows the default
build of cmd/cgo to be built using cgo, which may be necessary
on future essentially-cgo-only systems.
Change-Id: I414e22c10c9920f4e98f97fa35ff22058c0f143d
Reviewed-on: https://go-review.googlesource.com/68338
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The loop finder doesn't return good information if it
encounters an irreducible loop. Make a start on improving
this, and set a function-level flag to indicate when there
is such a loop (and the returned information might be flaky).
Use that flag to prevent the loop rotater from getting
confused; the existing code seems to depend on artifacts
of the previous loop-finding algorithm. (There is one
irreducible loop in the go library, in "inflate.go").
Change-Id: If6e26feab38d9b009d2252d556e1470c803bde40
Reviewed-on: https://go-review.googlesource.com/42150
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Telling the truth about these will be important for
content-based staleness checks.
Change-Id: Iaed6ca6c945eb805d815156753a3e5dc48c6f0b9
Reviewed-on: https://go-review.googlesource.com/68336
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Init functions are problematic because we want cmd/link to be
able to insert an import of runtime/cgo for external linking.
For all the other systems that just means putting runtime/cgo into
the binary. The linker is not set up to generate calls to init functions,
and luckily this one can be avoided entirely.
This means people don't have to import _ "runtime/cgo" in their
iOS programs anymore. The linker's default import is now enough.
This CL also adjusts cmd/go to record the linker's default import,
now that the explicit import is gone.
Change-Id: I81d23476663e03664f90d531c24db2e4f2e6c66b
Reviewed-on: https://go-review.googlesource.com/68490
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Historically, gc optimistically parsed the left-hand side of
assignments as expressions. Later, if it discovered a ":=" assignment,
it rewrote the parsed expressions as declarations.
This failed in the presence of dot imports though, because we lost
information about whether an imported object was named via a bare
identifier "Foo" or a normal qualified "pkg.Foo".
This CL fixes the issue by specially noding the left-hand side of ":="
assignments.
Fixes#22076.
Change-Id: I18190ecdb863112e7d009e1687e6112eec559921
Reviewed-on: https://go-review.googlesource.com/66810
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This shortens the args lists but also sets up better for
the content-based staleness changes.
While we're here, delete the now-unused Pkgpath method.
Change-Id: Ic60fa03efbc37a7c7fe9758a1cfa5dddef1a4151
Reviewed-on: https://go-review.googlesource.com/68335
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The following instructions were introduced in ARMv6, and the compiler
can do more optimization with them.
1. "MOVBS Rm@>i, Rd": rotates Rm 0/8/16/24 bits, does signed
byte extension to word, and writes the result to Rd.
2. "MOVHS Rm@>i, Rd": rotates Rm 0/8/16/24 bits, does signed
halfword extension to word, and writes the result to Rd.
3. "MOVBU Rm@>i, Rd": rotates Rm 0/8/16/24 bits, does unsigned
byte extension to word, and writes the result to Rd.
4. "MOVHU Rm@>i, Rd": rotates Rm 0/8/16/24 bits, does unsigned
half-word extension to word, and writes the result to Rd.
5. "XTAB Rm@>i, Rn, Rd": rotates Rm 0/8/16/24 bits, does signed
byte extension to word, adds Rn, and writes the result to Rd.
6. "XTAH Rm@>i, Rn, Rd": rotates Rm 0/8/16/24 bits, does signed
half-word extension to word, adds Rn, and writes the result to Rd.
7. "XTABU Rm@>i, Rn, Rd": rotates Rm 0/8/16/24 bits, does unsigned
byte extension to word, adds Rn, and writes the result to Rd.
8. "XTAHU Rm@>i, Rn, Rd": rotates Rm 0/8/16/24 bits, does unsigned
half-word extension to word, adds Rn, and writes the result to Rd.
Change-Id: I4306d7ebac93015d7e2e40d307f2c4271c03f466
Reviewed-on: https://go-review.googlesource.com/65790
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
For #22095
Change-Id: I2cb0d3e0aaf9f97952cf8dda0e99a4379e275020
Reviewed-on: https://go-review.googlesource.com/68332
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
For #22095
Change-Id: I07c288208d94dabae164c2ca0a067402a8e5c2e6
Reviewed-on: https://go-review.googlesource.com/68331
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Moves type symbol name mangling out of the object reader
and into a separate pass. Requires some care, as changing
the name of a type may require dealing with duplicate
symbols for the first time.
Disables DWARF for both plugins and programs that use plugin.Open,
because type manging is currently incompatible with the go.info.*
symbol generator in cmd/link. (It relies on the symbol names to
find type information.) A future fix for this would be moving the
go.info.* generation into the compiler, with the logic we use
for generating the type.* symbols.
Fixes#19529
Change-Id: I75615f8bdda86ff9e767e536d9aa36e15c194098
Reviewed-on: https://go-review.googlesource.com/67312
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
There is no cache here yet. This CL defines ActionID, Hash, and HashFile,
which the new content-based staleness code can use. Eventually we'll
put a real cache implementation here, but it's not necessary yet.
Change-Id: Ide433cb449f4dbe658694453f348c947642df79b
Reviewed-on: https://go-review.googlesource.com/67311
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Currently the linker figures out where runtime-gdb.py should be by
looking for the path to runtime/debug.go. However, debug.go contains
only a few symbols and can easily get dead-code eliminated entirely,
especially from simple binaries. When this happens, the resulting
binary lacks a reference to runtime-gdb.py, so the GDB helpers don't
get loaded.
Fix this by instead sniffing for runtime/proc.go. This contains
runtime.main and the scheduler, so it's not going anywhere.
Change-Id: Ie3380c77c840d28614fada68b8c5861625f2aff5
Reviewed-on: https://go-review.googlesource.com/68019
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
All of the callers want a *obj.LSym instead of a *types.Sym, and the
runtime type descriptors don't need Go source symbols anyway.
Passes toolstash-check.
Change-Id: I8ae4b64380fbb547857f49b42465118f41884eed
Reviewed-on: https://go-review.googlesource.com/68251
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Marvin Stenger <marvin.stenger94@gmail.com>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This can only lead to confusion.
Change-Id: Iae84c6404ab5eeb6950faa2364f97a017c67c506
Reviewed-on: https://go-review.googlesource.com/68110
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Simon Rawet <Simon@rawet.se>
Preparation for moving the object file reader to its own package.
For #22095
Change-Id: I31fe4a10a2c465f8ea4bf548f40918807e4ec6b5
Reviewed-on: https://go-review.googlesource.com/67314
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Also reduce the passed context from *Link to *sys.Arch, so fewer
data dependencies need to be wired through all the code dealing
with symbols.
For #22095
Change-Id: I50969405d6562c5152bd1a3c443b72413e9b70bc
Reviewed-on: https://go-review.googlesource.com/67313
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Begin passing coutbuf by as a parameter. To make the initial plumbing
pass easier, it is also a field in the standard ctxt parameter.
Consolidate the byte writing functions into the OutBuf object.
The result is less architecture-dependent initialization.
To avoid plumbing out everywhere we want to report an error, move
handling of out file deletion to an AtExit function.
For #22095
Change-Id: I0863695241562e0662ae3669666c7922b8c846f9
Reviewed-on: https://go-review.googlesource.com/67318
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
For #22095
Change-Id: I9d1f0d93f8fd701a24af826dc903eea2bc235de2
Reviewed-on: https://go-review.googlesource.com/67317
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
cmd/cgo is special among the build tools because it is (re)built on demand
when needed for a package using cgo, to avoid additional bootstrap logic
elsewhere. (This is in contrast to cmd/compile, cmd/link, and so on, which
must be specially built before even invoking the go command.)
When the go command starts using content-based decisions about staleness,
it is important that the build of cmd/cgo never use -linkmode=external,
because that depends on runtime/cgo, which in turn depends on cmd/cgo.
Change-Id: I72a2be748606d1ed4b93a54f2a5c7084e87d5fbc
Reviewed-on: https://go-review.googlesource.com/67310
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Change-Id: Iab8f0d201780bd571541a6806f071e883a553d35
Reviewed-on: https://go-review.googlesource.com/56286
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
This is an intermediate step toward not being able to predict
the final generated file name for a package build, so that
parent builds can refer directly to cache files.
Change-Id: I4dea5e8d8b80e6b995b3d9dc1d8c6f0ac9b88d72
Reviewed-on: https://go-review.googlesource.com/56285
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
These are rarely used and can be computed on demand,
to make clear that they are never out of sync with the
lists in the non-internal Package fields.
Change-Id: I8c621dceaff1aeb39a3ed83f18e848adf14d7106
Reviewed-on: https://go-review.googlesource.com/56284
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Actually execute topological sort to get those special dependencies right.
Mistake introduced in CL 67650.
Change-Id: I22fd6efb4f033deaf7f191431c0401b59b8a97d0
Reviewed-on: https://go-review.googlesource.com/67870
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When variables need to be spilled to the stack, they usually get their
own stack slot. Local variables have a slot allocated if they need one,
and arguments start out on the stack. Before this CL, the debug
information made the assumption that this was always the case, and so
didn't bother storing an actual stack offset during SSA analysis.
There's at least one case where this isn't true: variables that alias
arguments. Since the argument is the source of the variable, the
variable will begin its life on the stack in the argument's stack slot,
not its own. Therefore the debug info needs to track the actual stack
slot for each location entry.
No detectable performance change, despite the O(N) loop in getHomeSlot.
Change-Id: I2701adb7eddee17d4524336cb7aa6786e8f32b46
Reviewed-on: https://go-review.googlesource.com/67231
Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
Some tests in misc/cgo/test are run with various options including
'-linkmode=external "-extldflags=-pie"'. On ppc64x passing -pie to
the external linker with code that was not compiled as position
independent is incorrect. This works by luck in many cases but is
not guaranteed to work. I suspect it is an issue on other targets
as well.
This will now run the tests using -buildmode=pie for the platforms
that support that buildmode option.
Fixes#21954
Change-Id: I25fc7573f2d3cb5b0d1c691a0ac91aef7715404f
Reviewed-on: https://go-review.googlesource.com/66870
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The point of this code is to print a warning about repeated go test
invocations rebuilding the same packages over and over.
The new cache will eliminate this failure mode and with it
the need for the warning and this field.
Change-Id: Ied79b3ca67d51a61f44629de6ae4974e6c8dd5a9
Reviewed-on: https://go-review.googlesource.com/56282
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Mistake introduced just before submitting CL 67650 that somehow
did not break a final pre-submit all.bash on my laptop.
Not sure why all.bash passes locally when mkdeps.go doesn't build.
I guess the test only runs on builders?
Change-Id: I18fb91ada47605035345ba4b2f5e360a5c4b7f6e
Reviewed-on: https://go-review.googlesource.com/67850
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
In addition to the obvious formatting change, this also drops
from deps.go any indirect dependencies, so that when you add
a new import to one package, the resulting diff only affects that
one package, not every package that imports that package
directly or indirectly. That makes the file a bit easier to understand,
if you need to debug it or deal with a possible merge conflict.
The code to trim the import lists (but not too much) was more
than I wanted to do in shell, so I rewrote mkdeps in Go.
The shell script is left behind for backwards-compatibility with
people who have learned to run ./mkdeps.bash (or documentation
suggesting the same).
Change-Id: I0bf27b5b27d0440e11ea830b00735c73f58eae03
Reviewed-on: https://go-review.googlesource.com/67650
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This method indicates whether a type contains any *heap* pointers, not
just whether it contains any pointers. Rename the method to make this
clear.
Change-Id: Ifff143e2f02a820444ac26b84250495c0098cb33
Reviewed-on: https://go-review.googlesource.com/67690
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Use entire inlining call stack to decide whether two panic calls
can be merged. We used to merge panic calls when only the leaf
line numbers matched, but that leads to places higher up the call
stack being merged incorrectly.
Fixes#22083
Change-Id: Ia41400a80de4b6ecf3e5089abce0c42b65e9b38a
Reviewed-on: https://go-review.googlesource.com/67632
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Move some local declarations closer to their use, reducing their
respective lifetimes. Spotted while reviewing CL 67318.
Change-Id: I68db67fe8530344d95e50efb6587bc724e1171f5
Reviewed-on: https://go-review.googlesource.com/67370
Run-TryBot: Dave Cheney <dave@cheney.net>
Reviewed-by: Marvin Stenger <marvin.stenger94@gmail.com>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Now that we have -importcfg, there's no need for the
temporary directory trees that mirror the import path structure,
and we can drop a bunch of complex code that was building
and maintaining that structure.
This should fix "file name too long" errors on systems with low limits.
(For example #10651 and #17070, although we fixed those by
adding code to deal with very long file names on Windows instead.)
Change-Id: I11e221c6c1edeb81c3b2f1d89988f5235aa2bbb9
Reviewed-on: https://go-review.googlesource.com/56280
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Until now the subdirectories under $WORK have had predictable
names, so it was OK to strip just $WORK from the file names that
end up in object files. In the future, those predictable names would
cause predictable collisions when compiling one package in two
different ways, so we're moving toward arbitrary, unpredictable
subdirectory names instead. When we do that, if the names appear
in the object files we won't get reproducible builds.
Take the subdirectory names out now, to make the later change safe.
Change-Id: I8057d1cc73f6e35c98b7718c9789c161dcbd87c0
Reviewed-on: https://go-review.googlesource.com/67251
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 64793 removed the collect step, which took all
the generated .o files and merged them into a single _all.o.
Now the generated .o files all go directly into the final .a.
The only property of the _all.o approach that was lost
in CL 64793 was that before we could be sure that the
one name we used was "ar-compatible", that is, short
enough not to be truncated.
Now that the generated .o files are being kept directly,
this CL gives them guaranteed ar-compatible names.
This doesn't matter for nearly all uses today, but for some
future processing it might help not to lose the .o suffix
or not to end up with two identical entries with truncated
names.
I might not have bothered with this except that it's what's
leftover after syncing my own CL disabling _all.o
(necessary for reproducible builds on macOS) against
Ian's CL 64793, which landed first.
Change-Id: Ic86ed2a51432a5a4c58dc523e092a86d341f1997
Reviewed-on: https://go-review.googlesource.com/67250
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
I don't know why these tests must import runtime/cgo
in _testmain.go, but if they must, they must also tell the
rest of the go command that they are doing so.
Should fix the newly-broken darwin/arm and darwin/arm64 builders.
Change-Id: I9b183f8c84c6f403bf3a90cbfc838d6ef428e16f
Reviewed-on: https://go-review.googlesource.com/67230
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Modifying the input slice broke the new test for whether gccgo
supports -fgo-importcfg, as the test passed a slice of the argument
slice it was in the process of building.
Fixes#22089
Change-Id: I45444a82673223c46be0c8579da3e31a74c32d73
Reviewed-on: https://go-review.googlesource.com/67191
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
nilcheckelim2 cleans up by copying b.Values in a loop, omitting
OpUnknowns. However, the common case is that there are no OpUnknowns,
in which case we can skip a lot of work.
So we track the first nilcheck which was eliminated, if any, and only
start copying from there. If no nilcheck was eliminated we won't copy at all.
Fixes#20964
Change-Id: Icd44194cf8ac81ce6485ce257b4d33e093003a40
Reviewed-on: https://go-review.googlesource.com/65651
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Implement importcfg on behalf of gccgo by writing out a
tree of symbolic links. In addition to keeping gccgo working
with the latest changes, this also fixes a precedence bug in
gccgo's cmd/go vendor support (the vendor equivalent of #14271).
Change-Id: I0e5645116e1c84c957936baf22e3126ba6b0d46e
Reviewed-on: https://go-review.googlesource.com/61731
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This is a step toward using cached build artifacts: the importcfg
will direct the compiler and linker to read them right from the cache
if necessary. However, this CL does not have a cache yet, so it still
reads them from the usual install location or build location.
Even so, this fixes a long-standing issue that -I and -L (no longer used)
are not expressive enough to describe complex GOPATH setups.
Shared libraries are handled enough that all.bash passes, but
there may still be more work to do here. If so, tests and fixes
can be added in follow-up CLs.
Gccgo will need updating to support -importcfg as well.
Fixes#14271.
Change-Id: I5c52a0a5df0ffbf7436e1130c74e9e24fceff80f
Reviewed-on: https://go-review.googlesource.com/56279
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Add the package to the table and start it off with a few small, basic
functions. Inspired by CL 66331, which added flag.ro.
Updates #21851.
Change-Id: I3995cde1ff7bb09a718110473bed8b193c2232a5
Reviewed-on: https://go-review.googlesource.com/66990
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Remove an unused type, a few redundant returns and replace a few slice
append loops with a single append.
Change-Id: If07248180bae5631b5b152c6051d9635889997d5
Reviewed-on: https://go-review.googlesource.com/66851
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
The information that's used to generate DWARF location lists is very
ssa.Value centric; it uses Values as start and end coordinates to define
ranges. That mostly works fine, but control flow instructions don't come
from Values, so the ranges couldn't cover them.
Control flow instructions are generated when the SSA representation is
converted to assembly, so that's the best place to extend the ranges
to cover them. (Before that, there's nothing to refer to, and afterward
the boundaries between blocks have been lost.) That requires block
information in the debugInfo type, which then flows down to make
everything else awkward. On the plus side, there's a little less copying
slices around than there used to be, so it should be a little faster.
Previously, the ranges for empty blocks were not very meaningful. That
was fine, because they had no Values to cover, so no debug information
was generated for them. But they do have control flow instructions
(that's why they exist) and so now it's important that the information
be correct. Introduce two sentinel values, BlockStart and BlockEnd, that
denote the boundary of a block, even if the block is empty. BlockEnd
replaces the previous SurvivedBlock flag.
There's one more problem: the last instruction in the function will be a
control flow instruction, so any live ranges need to be extended past
it. But there's no instruction after it to use as the end of the range.
Instead, leave the EndProg field of those ranges as nil and fix it up to
point to past the end of the assembled text at the very last moment.
Change-Id: I81f884020ff36fd6fe8d7888fc57c99412c4245b
Reviewed-on: https://go-review.googlesource.com/63010
Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Very similar fix to the one made in golang.org/cl/65655. This time it's
for switches on interface values, as we look for duplicates in a
different manner to keep types in mind.
As before, add a small regression test.
Updates #22001.
Fixes#22063.
Change-Id: I9a55d08999aeca262ad276b4649b51848a627b02
Reviewed-on: https://go-review.googlesource.com/66450
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Combined the Split and Join call with a Replace. This simplifies
the code as well as makes it fast.
Micro-benchmarks show good improvements -
func BenchmarkJoinSplit(b *testing.B) {
for n := 0; n < b.N; n++ {
strings.Join(strings.Split("this string has some spaces", " "), "")
}
}
func BenchmarkReplace(b *testing.B) {
for n := 0; n < b.N; n++ {
strings.Replace("this string has some spaces", " ", "", -1)
}
}
name old time/op new time/op delta
JoinSplit-4 308ns ± 2% 192ns ± 4% -37.60% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
JoinSplit-4 144B ± 0% 64B ± 0% -55.56% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
JoinSplit-4 3.00 ± 0% 2.00 ± 0% -33.33% (p=0.008 n=5+5)
Change-Id: I1dc32105ae7a0be5a43ab0bedde992cefbed5d7d
Reviewed-on: https://go-review.googlesource.com/66590
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Attempting to set the SetGID bit on iOS 11 fails with a permission
error. Skip the test.
Change-Id: Idac59750447d668091c44fe7cc5ee178014e0e1d
Reviewed-on: https://go-review.googlesource.com/66490
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
