Use "reflect.TypeFor" to simplify the code.
Updates #60088
Change-Id: I93db6cbd4f02813d9a81f5d02996db8128cb81a9
GitHub-Last-Rev: 2aee64dac6
GitHub-Pull-Request: golang/go#75349
Reviewed-on: https://go-review.googlesource.com/c/go/+/701676
Reviewed-by: Mark Freeman <markfreeman@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Several CLs earlier in this stack added optimizations to reduce
user allocations by recognizing and taking advantage of literals,
including CL 649555, CL 649079, and CL 649035.
This CL adds debug hashing of those changes, which enables use of the
bisect tool, such as 'bisect -compile=literalalloc go test -run=Foo'.
This also allows these optimizations to be manually disabled via
'-gcflags=all=-d=literalallochash=n'.
Updates #71359
Change-Id: I854f7742a6efa5b17d914932d61a32b2297f0c88
Reviewed-on: https://go-review.googlesource.com/c/go/+/675415
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
CL 653856 enabled stack allocation of variable-sized makeslice results.
This CL adds debug hashing of that change, plus a debug flag
to control the byte threshold used.
The debug hashing machinery means we also now have a way to disable just
the CL 653856 optimization by doing -gcflags='all=-d=variablemakehash=n'
or similar, though the stderr output will then typically have many
lines of debug hash output.
Using this CL plus the bisect command, I was able to retroactively
find one of the lines of code responsible for #73199:
$ bisect -compile=variablemake go test -skip TestListWireGuardDrivers
[...]
bisect: FOUND failing change set
--- change set #1 (enabling changes causes failure)
./security_windows.go:1321:38 (variablemake)
./security_windows.go:1321:38 (variablemake)
---
Previously, I had tracked down those lines by diffing '-gcflags=-m=1'
output and brief code inspection, but seeing the bisect was very nice.
This CL also adds a compiler debug flag to control the threshold for
stack allocation of variably sized make results. This can help
us identify more code that is relying on certain stack allocations.
This might be a temporary flag that we delete prior to Go 1.25
(given we would not want people to rely on it), or maybe it
might make sense to keep it for some period of time beyond the release
of Go 1.25 to help the ecosystem shake out other bugs.
Using these two flags together (and picking a threshold of 64 rather
than the default of 32), it looks for example like this
x/sys/windows code might be relying on stack allocation of
a byte slice:
$ bisect -compile=variablemake go test -gcflags=-d=variablemakethreshold=64 -skip TestListWireGuardDrivers
[...]
bisect: FOUND failing change set
--- change set #1 (enabling changes causes failure)
./syscall_windows_test.go:1178:16 (variablemake)
Updates #73199Fixes#73253
Change-Id: I160179a0e3c148c3ea86be5c9b6cea8a52c3e5b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/663795
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
For FIPS init-time code+data verification, we need to arrange to
put the FIPS symbols into contiguous regions of the executable
and then record those sections along with the expected checksum.
The cmd/internal/obj changes identify the FIPS symbols and give
them distinguished types, which the linker then places in contiguous
regions. The linker also writes out information to use at run time
to find the FIPS sections, along with the expected hash.
See cmd/internal/obj/fips.go and cmd/link/internal/ld/fips.go
for more details.
The code is disabled in this commit.
CL 625998 and 625999 adds tests.
CL 626000 enables the code.
For #69536.
Change-Id: I48da6db94bc0bea7428c43d4abcf999527bccfcd
Reviewed-on: https://go-review.googlesource.com/c/go/+/625997
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Move the code that opens and increments counters out of the
cmd/internal/telemetry package into cmd/internal/telemetry/counter. The
telemetry package has dependencies on the upload code, which we do not
want to pull into the rest of the go toolchain.
For #68109
Change-Id: I463c106819b169177a783de4a7d93377e81f4e3e
Reviewed-on: https://go-review.googlesource.com/c/go/+/593976
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
CL 577935 enabled hot block alignment on 386
and amd64 architecture.
However, this change broke the plan9/386 build.
This change disables AlignHot on plan9/386.
Updates #67502.
Change-Id: If73b066824c7218a9408c6e8f06aec5908b7a64f
Reviewed-on: https://go-review.googlesource.com/c/go/+/586835
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
As mentioned in CL 584598, linkname is a mechanism that, when
abused, can break API integrity and even safety of Go programs.
CL 584598 is a first step to restrict the use of linknames, by
implementing a blocklist. This CL takes a step further, tightening
up the restriction by allowing linkname references ("pull") only
when the definition side explicitly opts into it, by having a
linkname on the definition (possibly to itself). This way, it is at
least clear on the definition side that the symbol, despite being
unexported, is accessed outside of the package. Unexported symbols
without linkname can now be actually private. This is similar to
the symbol visibility rule used by gccgo for years (which defines
unexported non-linknamed symbols as C static symbols).
As there can be pull-only linknames in the wild that may be broken
by this change, we currently only enforce this rule for symbols
defined in the standard library. Push linknames are added in the
standard library to allow things build.
Linkname references to external (non-Go) symbols are still allowed,
as their visibility is controlled by the C symbol visibility rules
and enforced by the C (static or dynamic) linker.
Assembly symbols are treated similar to linknamed symbols.
This is controlled by -checklinkname linker flag, currently not
enabled by default. A follow-up CL will enable it by default.
Change-Id: I07344f5c7a02124dbbef0fbc8fec3b666a4b2b0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/585358
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
This appears to be useful only on amd64, and was specifically
benchmarked on Apple Silicon and did not produce any benefit there.
This CL adds the assembly instruction `PCALIGNMAX align,amount`
which aligns to `align` if that can be achieved with `amount`
or fewer bytes of padding. (0 means never, but will align the
enclosing function.)
Specifically, if low-order-address-bits + amount are
greater than or equal to align; thus, `PCALIGNMAX 64,63` is
the same as `PCALIGN 64` and `PCALIGNMAX 64,0` will never
emit any alignment, but will still cause the function itself
to be aligned to (at least) 64 bytes.
Change-Id: Id51a056f1672f8095e8f755e01f72836c9686aa3
Reviewed-on: https://go-review.googlesource.com/c/go/+/577935
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Add cmd/internal/telemetry to cmd/dist's bootstrapDirs so it's built
when bootstrapping the compiler. cmd/internal/telemetry is a wrapper
arount telemetry functions that stubs out the functions when built in
bootstrap mode to avoid dependencies on x/telemetry in bootstrap mode.
Call telemetry.Start with an empty config to open the counter file, and
increment a counter for when the command is invoked.
After flags are parsed, increment a counter for each of the names of the
flags that were passed in. The counter names will be compile/flag:<name>
so for example we'll have compile/flag:e and compile/flag:E.
In FatalfAt, increment a stack counter for internal errors.
For #58894
Change-Id: Ia5a8a63aa43b2276641181626cbfbea7e4647faa
Reviewed-on: https://go-review.googlesource.com/c/go/+/570679
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Flag flip to enable stack slot merging by default when optimizing.
Please see the earlier CL for details on what this is doing.
Updates #62737.
Updates #65532.
Updates #65495.
Change-Id: I8e30d553e74ace43d418f883199721f05320d3d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/576681
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
[This is a partial roll-forward of CL 553055, the main change here
is that the stack slot overlap operation is flagged off by default
(can be enabled by hand with -gcflags=-d=mergelocals=1) ]
Preliminary compiler support for merging/overlapping stack slots of
local variables whose access patterns are disjoint.
This patch includes changes in AllocFrame to do the actual
merging/overlapping based on information returned from a new
liveness.MergeLocals helper. The MergeLocals helper identifies
candidates by looking for sets of AUTO variables that either A) have
the same size and GC shape (if types contain pointers), or B) have the
same size (but potentially different types as long as those types have
no pointers). Variables must be greater than (3*types.PtrSize) in size
to be considered for merging.
After forming candidates, MergeLocals collects variables into "can be
overlapped" equivalence classes or partitions; this process is driven
by an additional liveness analysis pass. Ideally it would be nice to
move the existing stackmap liveness pass up before AllocFrame
and "widen" it to include merge candidates so that we can do just a
single liveness as opposed to two passes, however this may be difficult
given that the merge-locals liveness has to take into account
writes corresponding to dead stores.
This patch also required a change to the way ssa.OpVarDef pseudo-ops
are generated; prior to this point they would only be created for
variables whose type included pointers; if stack slot merging is
enabled then the ssagen code creates OpVarDef ops for all auto vars
that are merge candidates.
Note that some temporaries created late in the compilation process
(e.g. during ssa backend) are difficult to reason about, especially in
cases where we take the address of a temp and pass it to the runtime.
For the time being we mark most of the vars created post-ssagen as
"not a merge candidate".
Stack slot merging for locals/autos is enabled by default if "-N" is
not in effect, and can be disabled via "-gcflags=-d=mergelocals=0".
Fixmes/todos/restrictions:
- try lowering size restrictions
- re-evaluate the various skips that happen in SSA-created autotmps
Updates #62737.
Updates #65532.
Updates #65495.
Change-Id: Ifda26bc48cde5667de245c8a9671b3f0a30bb45d
Reviewed-on: https://go-review.googlesource.com/c/go/+/575415
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This reverts CL 553055.
Reason for revert: causes crypto/ecdsa failures on linux ppc64/s390x builders
Change-Id: I9266b030693a5b6b1e667a009de89d613755b048
Reviewed-on: https://go-review.googlesource.com/c/go/+/575236
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Preliminary compiler support for merging/overlapping stack
slots of local variables whose access patterns are disjoint.
This patch includes changes in AllocFrame to do the actual
merging/overlapping based on information returned from a new
liveness.MergeLocals helper. The MergeLocals helper identifies
candidates by looking for sets of AUTO variables that either A) have
the same size and GC shape (if types contain pointers), or B) have the
same size (but potentially different types as long as those types have
no pointers). Variables must be greater than (3*types.PtrSize) in size
to be considered for merging.
After forming candidates, MergeLocals collects variables into "can be
overlapped" equivalence classes or partitions; this process is driven
by an additional liveness analysis pass. Ideally it would be nice to
move the existing stackmap liveness pass up before AllocFrame
and "widen" it to include merge candidates so that we can do just a
single liveness as opposed to two passes, however this may be difficult
given that the merge-locals liveness has to take into account
writes corresponding to dead stores.
This patch also required a change to the way ssa.OpVarDef pseudo-ops
are generated; prior to this point they would only be created for
variables whose type included pointers; if stack slot merging is
enabled then the ssagen code creates OpVarDef ops for all auto vars
that are merge candidates.
Note that some temporaries created late in the compilation process
(e.g. during ssa backend) are difficult to reason about, especially in
cases where we take the address of a temp and pass it to the runtime.
For the time being we mark most of the vars created post-ssagen as
"not a merge candidate".
Stack slot merging for locals/autos is enabled by default if "-N" is
not in effect, and can be disabled via "-gcflags=-d=mergelocals=0".
Fixmes/todos/restrictions:
- try lowering size restrictions
- re-evaluate the various skips that happen in SSA-created autotmps
Fixes#62737.
Updates #65532.
Updates #65495.
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest
Change-Id: Ibc22e8a76c87e47bc9fafe4959804d9ea923623d
Reviewed-on: https://go-review.googlesource.com/c/go/+/553055
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The pgo compilation time is very long if the profile file is large.
We added a preprocess tool to pre-parse profile file in order to
expedite the compile time.
Change-Id: I6f50bbd01f242448e2463607a9b63483c6ca9a12
Reviewed-on: https://go-review.googlesource.com/c/go/+/529738
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Shape-based stenciling in the Go compiler's generic instantiation
phase looks up shape types using the underlying type of a given target
type. This has a beneficial effect in most cases (e.g. we can use the
same shape type for two different named types whose underlying type is
"int"), but causes some problems when the underlying type is a very
large structure. The link string for the underlying type of a large
imported struct can be extremely long, since the link string
essentially enumerates the full package path for every field type;
this can produce a "go.shape.struct { ... " symbol name that is
absurdly long.
This patch switches the compiler to use a hash of the underlying type
link string instead of the string itself, which should continue to
provide commoning but keep symbol name lengths reasonable for shape
types based on large imported structs.
Fixes#65030.
Change-Id: I87d602626c43172beb99c186b8ef72327b8227a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/554975
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Than McIntosh <thanm@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Extend the pgodevirtualize debug flag to distinguish interface and
function devirtualization. Setting 1 keeps interface devirtualization
enabled but disables function value devirtualization.
For #64209.
Change-Id: I33aa7eb95ca0bdb215256d8c7cc8f9dac53ae30e
Reviewed-on: https://go-review.googlesource.com/c/go/+/543115
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
E.g.
`GOEXPERIMENT=rangefunc go test -v -gcflags=-d=rangefunccheck=0 rangefunc_test.go`
will turn off the checking and fail.
The benchmarks, which do not use pathological iterators, run slightly faster.
Change-Id: Ia3e175e86d67ef74bbae9bcc5d2def6a2cdf519d
Reviewed-on: https://go-review.googlesource.com/c/go/+/541995
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Relocate the 'covcmd' package from .../internal/coverage to
.../cmd/internal/cov, to reflect the fact that the definitions in this
package are used only in cmd, not in std.
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-windows-amd64-longtest
Change-Id: I65bcc34736d1d0a23134a6c91c17ff138cd45431
Reviewed-on: https://go-review.googlesource.com/c/go/+/526595
Reviewed-by: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
When a PGO build fails or produces incorrect program, it is often
unclear what the problem is. Add pgo hash so we can bisect to
individual optimization decisions, which often helps debugging.
Related to #58153.
Change-Id: I651ffd9c53bad60f2f28c8ec2a90a3f532982712
Reviewed-on: https://go-review.googlesource.com/c/go/+/528400
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This patch fixes an inconsistency in compiler flag handling introduced
accidentally in CL 521699. In the compiler we have both base.Flag.N
(which records whether the user has supplied the "-N" flag to disable
optimization) and base.Ctxt.Flag_optimize (which tracks whether
optimization is turned on). In this case Flag.N was updated without a
corresponding change to Ctxt.Flag_optimize, which led to problems with
DWARF generation for the runtime.
This CL doesn't include a regression test; a test will be added later
in the x/debug repo in a subsequent CL.
Updates #62523.
Change-Id: I0c383bb43ec0a0e7c12e7e2852c0590731416d6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/527319
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
As we did for the asm -compiling-runtime flag, this CL modifies the
compiler to compute the -+ (compiling runtime) flag from the package
path. Unlike for asm, some tests use -+ explicitly to opt in to
runtime restrictions, so we leave the flag, but it's no longer passed
by any build tools.
This lets us eliminate cmd/go's list of "runtime packages" in favor of
the unified objabi.LookupPkgSpecial. It also fixes an inconsistency
with dist, which only passed -+ when compiling "runtime" itself.
One consequence of this is that the compiler now ignores the -N flag
when compiling runtime packages. Previously, cmd/go would strip -N
when passing -+ and the compiler would fatal if it got both -N and -+,
so the overall effect was that the compiler never saw -N when
compiling a runtime package. Now we simply move that logic to disable
-N down into the compiler.
Change-Id: I4876047a1563210ed122a31b72d62798762cbcf5
Reviewed-on: https://go-review.googlesource.com/c/go/+/521699
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Relocate the definitions in cmddefs.go (used by the compiler and
the cover tool) to a separate package "covcmd". No change
in functionality, this is a pure refactoring, in preparation
for a subsequent change that will require updating the
imports for the package.
Change-Id: Ic1d277c94d9a574de0a11ec5ed77e892302b9a47
Reviewed-on: https://go-review.googlesource.com/c/go/+/517696
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: David Chase <drchase@google.com>
This flag is not terribly useful with the go command, which will pass
all environment variables through to subprocesses it invokes,
but it can be useful in other build systems, notably blaze and bazel,
to pass compiler-debugging variables like GOSSAFUNC through to
the compiler.
We have been maintaining this as a patch against Google's internal
toolchain for many years, and it has proven useful in those non-go-command
contexts.
Change-Id: Ic123193319f3c838a694eda2575347c516b85ac7
Reviewed-on: https://go-review.googlesource.com/c/go/+/507977
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Change-Id: If8fc77d5b04b2979707032d91112d4f33ef5e3da
GitHub-Last-Rev: d82d769c04
GitHub-Pull-Request: golang/go#55913
Reviewed-on: https://go-review.googlesource.com/c/go/+/435536
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: shuang cui <imcusg@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: qiulaidongfeng <2645477756@qq.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This reverts commit e7a9ca0a53.
Reason for revert: Decided to delay to Go 1.22.
Change-Id: I4635cb4c1372b54cac573041be8a43e294de5183
Reviewed-on: https://go-review.googlesource.com/c/go/+/497975
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
In the type checkers, add Config.ErrorURL (or Config._ErrorURL for
go/types) to configure whether and how an error message should report
a URL for errors that have an error code.
In the compiler, configure types2 to report an error URL of the form
" [go.dev/e/XXX]", where XXX stands for the error code, with the URL
appended to the first line of an error.
Rename the compiler flag -url to -errorurl. At the moment this flag
is disabled by default.
Example for a one-line error message:
<pos>: undefined: f [go.dev/e/UndeclaredName]
Example for a multi-line error message:
<pos>: not enough arguments in call to min [go.dev/e/WrongArgCount]
have ()
want (P, P)
Change-Id: I26651ce2c92ad32fddd641f003db37fe12fdb1cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/497715
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
This CL is originally based on CL 484838 from rajbarik@uber.com.
Add a new PGO-based devirtualize pass. This pass conditionally
devirtualizes interface calls for the hottest callee. That is, it
performs a transformation like:
type Iface interface {
Foo()
}
type Concrete struct{}
func (Concrete) Foo() {}
func foo(i Iface) {
i.Foo()
}
to:
func foo(i Iface) {
if c, ok := i.(Concrete); ok {
c.Foo()
} else {
i.Foo()
}
}
The primary benefit of this transformation is enabling inlining of the
direct calls.
Today this change has no impact on the escape behavior, as the fallback
interface always forces an escape. But improving escape analysis to take
advantage of this is an area of potential work.
This CL is the bare minimum of a devirtualization implementation. There
are still numerous limitations:
* Callees not directly referenced in the current package can be missed
(even if they are in the transitive dependences).
* Callees not in the transitive dependencies of the current package are
missed.
* Only interface method calls are supported, not other indirect function
calls.
* Multiple calls to compatible interfaces on the same line cannot be
distinguished and will use the same callee target.
* Callees that only partially implement an interface (they are embedded
in another type that completes the interface) cannot be devirtualized.
* Others, mentioned in TODOs.
Fixes#59959
Change-Id: I8bedb516139695ee4069650b099d05957b7ce5ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/492436
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
We will soon have PGO specialization. It doesn't make sense for the
debug flag to have inline in the name, so rename it to pgodebug.
pgoinline is now a flag that can be used to disable PGO inlining.
Devirtualization will have a similar debug flag.
For #59959.
Change-Id: I9770ff1f0d132dfa3cd417018a887a1bd5555bba
Reviewed-on: https://go-review.googlesource.com/c/go/+/494716
Auto-Submit: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
The FS form was only necessary for reliable hashes in tests,
and for that we can use -trimpath.
Another potential concern would be temporary work directory
names leaking into the names of files generated by cgo and the
like, but we already make sure to avoid those to ensure
reproducible builds: the compiler never sees those paths.
So the FS form is not necessary for that either.
Change-Id: Idae2c6acb22ab64dfb33bb053244d23fbe153830
Reviewed-on: https://go-review.googlesource.com/c/go/+/493737
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
The modification of these rules is optimization to load/store global
variables. If there are a sequence of loads/stores nearby a global
variable address, the address can only be loaded from GOT once instead
of every time.
For #43264
Change-Id: Idedaf6c81f085955371320f51bca148ffb42a2d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/348732
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
This is actually not tied to the ELF psABI v2 upgrade, and can be
enabled "for free".
Change-Id: I6906d9eb4bd8655c685b059283e200cb7e210369
Reviewed-on: https://go-review.googlesource.com/c/go/+/455075
Reviewed-by: abner chenc <chenguoqi@loongson.cn>
Run-TryBot: Ben Shi <powerman1st@163.com>
Reviewed-by: Meidan Li <limeidan@loongson.cn>
Run-TryBot: WANG Xuerui <git@xen0n.name>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: David Chase <drchase@google.com>
HashDebugPos function/method included a parameter that was always
the same, and a variable in the same package as the hashdebug code.
So remove it.
(I wrote that code, there was no reason for it to be that way).
Also corrects a stale comment in the loopvar code.
Change-Id: Id3da69cfe6dadeb31d5de62fb76d15103a5d6152
Reviewed-on: https://go-review.googlesource.com/c/go/+/482816
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
If the -url flag is provided, when encountering a type checking error,
the compiler will also print a URL to a more detailed description of
the error and an example, if available.
Example uses:
go tool compile -url filename.go
go build -gcflags=-url pkg/path
For instance, a duplicate declaration of an identifier will report
https://pkg.go.dev/internal/types/errors#DuplicateDecl
We may refine the provided URL over time.
Change-Id: Iabe3008a49d9dd88bf690f99e4a4a5432dc08786
Reviewed-on: https://go-review.googlesource.com/c/go/+/476716
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Remove the compiler's "-wrapglobalmapinit" flag; it is potentially
confusing for users and isn't appropriate as a top level flag. Move
the enable/disable control to the "wrapglobalmapctl" debug flag
(values: 0 on by default, 1 disabled, 2 stress mode). No other changes
to compiler functionality.
Change-Id: I0d120eaf90ee34e29d5032889e673d42fe99e5dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/475035
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Adds:
GOEXPERIMENT=loopvar (expected way of invoking)
-d=loopvar={-1,0,1,2,11,12} (for per-package control and/or logging)
-d=loopvarhash=... (for hash debugging)
loopvar=11,12 are for testing, benchmarking, and debugging.
If enabled,for loops of the form `for x,y := range thing`, if x and/or
y are addressed or captured by a closure, are transformed by renaming
x/y to a temporary and prepending an assignment to the body of the
loop x := tmp_x. This changes the loop semantics by making each
iteration's instance of x be distinct from the others (currently they
are all aliased, and when this matters, it is almost always a bug).
3-range with captured iteration variables are also transformed,
though it is a more complex transformation.
"Optimized" to do a simpler transformation for
3-clause for where the increment is empty.
(Prior optimization of address-taking under Return disabled, because
it was incorrect; returns can have loops for children. Restored in
a later CL.)
Includes support for -d=loopvarhash=<binary string> intended for use
with hash search and GOCOMPILEDEBUG=loopvarhash=<binary string>
(use `gossahash -e loopvarhash command-that-fails`).
Minor feature upgrades to hash-triggered features; clients can specify
that file-position hashes use only the most-inline position, and/or that
they use only the basenames of source files (not the full directory path).
Most-inlined is the right choice for debugging loop-iteration change
once the semantics are linked to the package across inlining; basename-only
makes it tractable to write tests (which, otherwise, depend on the full
pathname of the source file and thus vary).
Updates #57969.
Change-Id: I180a51a3f8d4173f6210c861f10de23de8a1b1db
Reviewed-on: https://go-review.googlesource.com/c/go/+/411904
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
There are a plenty of regression in 1.20 with this optimization. This CL
disable inline static init, so it's safer to backport to 1.20 branch.
The optimization will be enabled again during 1.21 cycle.
Updates #58293
Updates #58339
For #58293
Change-Id: If5916008597b46146b4dc7108c6b389d53f35e95
Reviewed-on: https://go-review.googlesource.com/c/go/+/467015
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
This patch changes the compiler's pkg init machinery to pick out large
initialization assignments to global maps (e.g.
var mymap = map[string]int{"foo":1, "bar":2, ... }
and extract the map init code into a separate outlined function, which is
then called from the main init function with a weak relocation:
var mymap map[string]int // KEEP reloc -> map.init.0
func init() {
map.init.0() // weak relocation
}
func map.init.0() {
mymap = map[string]int{"foo":1, "bar":2}
}
The map init outlining is done selectively (only in the case where the
RHS code exceeds a size limit of 20 IR nodes).
In order to ensure that a given map.init.NNN function is included when
its corresponding map is live, we add dummy R_KEEP relocation from the
map variable to the map init function.
This first patch includes the main compiler compiler changes, and with
the weak relocation addition disabled. Subsequent patch includes the
requred linker changes along with switching to the call to the
outlined routine to a weak relocation. See the later linker change for
associated compile time performance numbers.
Updates #2559.
Updates #36021.
Updates #14840.
Change-Id: I1fd6fd6397772be1ebd3eb397caf68ae9a3147e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/461315
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This CL removes the GOEXPERIMENT=nounified knob, and any conditional
statements that depend on that knob. Further CLs to remove unreachable
code follow this one.
Updates #57410.
Change-Id: I39c147e1a83601c73f8316a001705778fee64a91
Reviewed-on: https://go-review.googlesource.com/c/go/+/458615
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Ordinary interface types now satisfy comparable constraints. This
is a fully backward-compatible change: it simply permits additional
code to be valid that wasn't valid before.
This change makes the new comparable semantics the default behavior,
depending on the Go -lang version.
It also renames the flag types2.Config.AltComparableSemantics to
types2.Config.OldComparableSemantics and inverts its meaning
(or types.Config.oldComparableSemantics respectively).
Add new predicate Satisfies (matching the predicate Implements but
for constraint satisfaction), per the proposal description.
Adjust some existing tests by setting -oldComparableSemantics
and add some new tests that verify version-dependent behavior.
The compiler flag -oldcomparable may be used to temporarily
switch back to the Go 1.18/1.19 behavior should this change
cause problems, or to identify that a problem is unrelated
to this change. The flag will be removed for Go 1.21.
For #52509.
For #56548.
For #57011.
Change-Id: I8b3b3d9d492fc24b0693567055f0053ccb5aeb42
Reviewed-on: https://go-review.googlesource.com/c/go/+/454575
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
The CL below was accidentally submitted, while waiting for the freeze
exception. Reverting.
This reverts commit 15e705ea96.
Change-Id: I4dbf92dcb01fa9245a6e6a2d1514d8aa898d0048
Reviewed-on: https://go-review.googlesource.com/c/go/+/454476
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Ordinary interface types now satisfy comparable constraints.
This change makes the new comparable semantics the default behavior,
depending on the Go -lang version.
It also renames the flag types2.Config.AltComparableSemantics to
types2.Config.OldComparableSemantics and inverts its meaning
(or types.Config.oldComparableSemantics respectively).
Adjust some existing tests by setting -oldComparableSemantics
and add some new tests that verify version-dependent behavior.
The compiler flag -oldcomparable may be used to temporarily
switch back to the Go 1.18/1.19 behavior should this change
cause problems, or to identify that a problem is unrelated
to this change. The flag will be removed for Go 1.21.
For #52509.
For #56548.
Change-Id: Ic2b22db9433a8dd81dc1ed9d30835f0395fb7205
Reviewed-on: https://go-review.googlesource.com/c/go/+/453978
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com>
This was disabled in CL 452676 out of an abundance of caution,
but further analysis has shown that the failures were not being
caused by this optimization. Instead the sequence of commits was:
CL 450136 cmd/compile: handle simple inlined calls in staticinit
...
CL 449937 archive/tar, archive/zip: return ErrInsecurePath for unsafe paths
...
CL 451555 cmd/compile: fix static init for inlined calls
The failures in question became compile failures in the first CL
and started building again after the last CL.
But in the interim the code had been broken by the middle CL.
CL 451555 was just the first time that the tests could run and fail.
For #30820.
Change-Id: I65064032355b56fdb43d9731be2f9f32ef6ee600
Reviewed-on: https://go-review.googlesource.com/c/go/+/452817
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
This CL adds -d=inlstaticinit to control whether static initialization
of inlined function calls (added in CL 450136) is allowed.
We've needed to fix it once already (CL 451555) and Google-internal
testing is hitting additional failure cases, so putting this
optimization behind a feature flag seems appropriate regardless.
Also, while we diagnose and fix the remaining cases, this CL also
disables the optimization to avoid miscompilations.
Updates #56894.
Change-Id: If52a358ad1e9d6aad1c74fac5a81ff9cfa5a3793
Reviewed-on: https://go-review.googlesource.com/c/go/+/452676
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
