These comparisons are fairly arbitrary,
but they should be more stable in the face
of other compiler changes than value ID.
This reduces the number of value ID
comparisons in schedule while running
make.bash from 542,442 to 99,703.
There are lots of changes to generated code
from this change, but they appear to
be overall neutral.
It is possible to further reduce the
number of comparisons in schedule;
I have changes locally that reduce the
number to about 25,000 during make.bash.
However, the changes are increasingly
complex and arcane, and reduce in much less
code churn. Given that the goal is stability,
that suggests that this is a reasonable
place to stop, at least for now.
Change-Id: Ie3a75f84fd3f3fdb102fcd0b29299950ea66b827
Reviewed-on: https://go-review.googlesource.com/c/go/+/229799
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Falling back to comparing Value.ID during scheduling
is undesirable: Not only are we simply hoping for a good
outcome, but the decision we make will be easily perturbed
by other compiler changes, leading to random fluctuations.
This change adds another decision point to the scheduler
by scheduling Values with many uses earlier.
Values with fewer uses are less likely to be spilled for
other reasons, so we should issue them as late as possible
in the hope of avoiding a spill.
This reduces the number of Value ID comparisons
in schedule while running make.bash
from 1,000,844 to 542,442.
As you would expect, this changes a lot of functions,
but the overall trend is positive:
file before after Δ %
api 5237184 5233088 -4096 -0.078%
compile 19926480 19918288 -8192 -0.041%
cover 5281816 5277720 -4096 -0.078%
dist 3711608 3707512 -4096 -0.110%
total 113588440 113567960 -20480 -0.018%
Change-Id: Ic99ebc4c614d4ae3807ce44473ec6b04684388ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/229798
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The compiler produces high quality error messages when an interface is
implemented by *T, rather than T. This change improves the analogous
error messages in go/types, from "missing method X" to "missing method
X (X has pointer receiver)".
I am open to improving this message further - I didn't copy the compiler
error message exactly because, at one of the call sites of
(*check).missingMethod, we no longer have access to the name of the
interface.
Fixesgolang/go#36336
Change-Id: Ic4fc38b13fff9e5d9a69cc750c21e0b0c34d85a8
Reviewed-on: https://go-review.googlesource.com/c/go/+/229801
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Previous CL introduced index fingerprint in the object files.
This CL implements the second part: checking fingerprint
consistency in the linker when packages are loaded.
Change-Id: I05dd4c4045a65adfd95e77b625d6c75a7a70e4f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/229618
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
The new object files use indices for symbol references, instead
of names. Fundamental to the design, it requires that the
importing and imported packages have consistent view of symbol
indices. The Go command should already ensure this, when using
"go build". But in case it goes wrong, it could lead to obscure
errors like run-time crashes. It would be better to check the
index consistency at build time.
To do that, we add a fingerprint to each object file, which is
a hash of symbol indices. In the object file it records the
fingerprints of all imported packages, as well as its own
fingerprint. At link time, the linker checks that a package's
fingerprint matches the fingerprint recorded in the importing
packages, and issue an error if they don't match.
This CL does the first part: introducing the fingerprint in the
object file, and propagating fingerprints through
importing/exporting by the compiler. It is not yet used by the
linker. Next CL will do.
Change-Id: I0aa372da652e4afb11f2867cb71689a3e3f9966e
Reviewed-on: https://go-review.googlesource.com/c/go/+/229617
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
In the code there are conditions like !ctxt.IsDarwin(). This will
accidentally be true if HeadType is not yet set. Panic when
HeadType is not set, to catch errors.
Change-Id: Ic891123f27f0276fff5a4b5d29e5b1f7ebbb94ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/229869
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
So we can use it to set per-OS flags.
Also set flagnewDoData after archinit, where IsELF is set.
This should correct the logic of setting flagnewDoData.
Change-Id: I18c7252f141aa35119005c252becc9d7cb74f2f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/229867
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Keep track of all expressions encountered while
generating a rewrite result, and re-use them whenever possible.
Named expressions may still be used for clarity when desired.
Change-Id: I640dca108763eb8baeff8f9a4169300af3445b82
Reviewed-on: https://go-review.googlesource.com/c/go/+/229800
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Add elf/386 arch support for the new dodata() phase.
Change-Id: I78341dfe70a90719d95c0044183980f348a3369f
Reviewed-on: https://go-review.googlesource.com/c/go/+/229797
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
It doesn't do what it says. It has been like that since Go 1.4.
The current ouput is pretty useless. Remove it.
Change-Id: Id9b4ba04139aaf7ea59acbd51428b1c992115389
Reviewed-on: https://go-review.googlesource.com/c/go/+/229859
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
This page has moved to the x/website repo in CL 229482 (commit
golang/website@70f4ee8c7e).
Remove the old copy in this repo since it's no longer used.
For #29206.
Change-Id: Ief093ed8c5dfec43e06d473e4282275f61da74a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/229485
Reviewed-by: Alexander Rakoczy <alex@golang.org>
In the dev.link branch we continued developing the new object
file format support and the linker improvements described in
https://golang.org/s/better-linker . Since the last merge, more
progress has been made to improve the new linker.
This is a clean merge.
Change-Id: I57c510b651a39354d78478a9a4499f770eef2eb1
This patch begins the work of converting the linker's dodata phase to
work with loader APIs. Passes all.bash on linux/amd64, but hasn't been
tested on anything else (more arch-specific code needs to be written).
Use of the new dodata() phase is currently gated by a temporary
command line flag ("-newdodata"), and there is code in the linker's
main routine to insure that we only use the new version for the right
GOOS/GOARCH (currently restricted to ELF + AMD64).
Change-Id: Ied3966677d2a450bc3e0990e0f519b3fceaab806
Reviewed-on: https://go-review.googlesource.com/c/go/+/229706
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Move the guts of ctxt.Errorf into loader.Loader, so that we can make
calls to it from functions that have a "*loader.Loader" available but
not a "ctxt *Link". This is needed to start converting hooks like
"adddynrel" in the arch-specific portions of the linker to use loader
APIs.
Change-Id: Ieedd4583b66504be0e77d7f3fbadafe0d2307a69
Reviewed-on: https://go-review.googlesource.com/c/go/+/229497
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Clients may need to invoke the loader.Reloc2.SetType method to reset
the type of a relocation from external flavor to internal flavor,
meaning that the external type add-in needs to be zeroed (this is
needed when adding dynsym entries).
Add a new SymbolBuider method to support mutating the type of a reloc
for an external symbol, so that the external type can be changed as
well (Reloc2 doesn't have access to that). Also add similar methods
for updating target symbol and addend, so as to have a consistent
interface for ext reloc mutation.
Change-Id: I8e26cdae0a0f353019acba5f9c8a0506e3970266
Reviewed-on: https://go-review.googlesource.com/c/go/+/229604
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Allow for the possibility that a client could call newExtSym(), then
ask for the section of the new sym before SetSectSym is called on it
(check in SymSect for this case).
Change-Id: I7bd78e7b3b7618943705b616f62ea78c4a1b68d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/229603
Reviewed-by: Jeremy Faller <jeremy@golang.org>
On darwin/arm64, the copy of the system roots takes 256 KiB of disk
and 560 KiB of memory after parsing them (which is retained forever in
a package global by x509/root.go). In constrained environments like
iOS NetworkExtensions where total disk+RAM is capped at 15 MiB, these
certs take 5.3% of the total allowed memory.
It turns out you can get down from 816 KiB to 110 KiB by instead
storing compressed x509 certs in the binary and lazily inflating just
the needed certs at runtime as a function of the certs presented to
you by the server, then building a custom root CertPool in the
crypto/tls.Config.VerifyPeerCertificate hook.
This then saves 706 KiB.
Arguably that should be the default Go behavior, but involves
cooperation between x509 and tls, and adds a dependency to
compress/gzip. Also, it may not be the right trade-off for everybody,
as it involves burning more CPU on new TLS connections. Most iOS apps
don't run in a NetworkExtension context limiting them to 15 MiB.
The build tag is chosen to match the existing "nethttpomithttp2".
Change-Id: I7b1c845de08b22674f81dd546e7fadc7dda68bd7
Reviewed-on: https://go-review.googlesource.com/c/go/+/229762
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Fix regression where line numbers in the sources column of generated
ssa.html output became misaligned with the source code. This was due
to some new margins applied to certain h2 elements during the work
to combine identical columns.
Fixes#38612
Change-Id: I067ccbfa30d5de5be29aab9863bc1e21f6ded128
Reviewed-on: https://go-review.googlesource.com/c/go/+/229766
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This allows more exciting changes to compiler-generated assembly
language that might not be correct for tricky hand-crafted
assembly (e.g., nop padding breaking tables of call or branch
instructions).
Updates #35881
Change-Id: I842b811796076c160180a364564f2844604df3fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/229708
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
No longer needed after the last CL. Separate commit because
renumbering Ops causes toolstash to complain.
Change-Id: I6223a790cc341f8184eccb503f95a1dfc32a81e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/229760
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL uses fixVariadicCall before escape analyzing function calls.
This has a number of benefits, though also some minor obstacles:
Most notably, it allows us to remove ODDDARG along with the logic
involved in setting it up, manipulating EscHoles, and later copying
its escape analysis flags to the actual slice argument. Instead, we
uniformly handle all variadic calls the same way. (E.g., issue31573.go
is updated because now f() and f(nil...) are handled identically.)
It also allows us to simplify handling of builtins and generic
function calls. Previously handling of calls was hairy enough to
require multiple dispatches on n.Op, whereas now the logic is uniform
enough that we can easily handle it with a single dispatch.
The downside is handling //go:uintptrescapes is now somewhat clumsy.
(It used to be clumsy, but it still is, too.) The proper fix here is
probably to stop using escape analysis tags for //go:uintptrescapes
and unsafe-uintptr, and have an earlier pass responsible for them.
Finally, note that while we now call fixVariadicCall in Escape, we
still have to call it in Order, because we don't (yet) run Escape on
all compiler-generated functions. In particular, the generated "init"
function for initializing package-level variables can contain calls to
variadic functions and isn't escape analyzed.
Passes toolstash-check -race.
Change-Id: I4cdb92a393ac487910aeee58a5cb8c1500eef881
Reviewed-on: https://go-review.googlesource.com/c/go/+/229759
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
This CL adds a UsesCgo config setting to go/types to specify that the
_cgo_gotypes.go file generated by cmd/cgo has been provided as a
source file. The type checker then internally resolves C.bar qualified
identifiers to _Cfoo_bar as appropriate.
It also adds support to srcimporter to automatically run cgo.
Unfortunately, this functionality is not compatible with overriding
OpenFile, because cmd/cgo and gcc will directly open files.
Updates #16623.
Updates #35721.
Change-Id: I1e1965fe41b765b7a9da3431f2a86cc16025dee2
Reviewed-on: https://go-review.googlesource.com/c/go/+/33677
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This adds a few minor changes from the first review.
Passes toolstash-check
Change-Id: I00f6f1b0235d0a8c686aa8793d0473b8fc6b1495
Reviewed-on: https://go-review.googlesource.com/c/go/+/229699
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Keith Randall <khr@golang.org>
Fix a longstanding TODO.
Provides widespread, minor improvements.
Negligible compiler cost.
Because the freeze nears, put in a safety flag to easily disable.
Change-Id: I338812181ab6d806fecf22afd3c3502e2c94f7a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/229600
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Provides minor widespread benefit to generated code.
Removes one source of random fluctuation when changing
other aspects of the compiler.
Change-Id: I16db6f5e240a97d27f05dc1ba5b8b729af3adb12
Reviewed-on: https://go-review.googlesource.com/c/go/+/229702
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change adds some instructions that were missing from the
ppc64 assembler, mostly power9 but a few others from earlier.
Tests in cmd/asm for ppc64 were updated: ppc64.s includes the
new instructions, and ppc64enc.s now includes not only the
new instructions but most ppc64 opcodes to provide a more
complete test of the ppc64 assembler.
The ppc64 instruction set is used for linux/ppc64le,
linux/ppc64, and aix/ppc64.
Change-Id: I8695f89dbca06174847963f4ef869f2e584d5bbf
Reviewed-on: https://go-review.googlesource.com/c/go/+/229479
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Converted some Println() statements (used to make sure that certain variables were
kept alive and not optimized out) to assignments into global variables, so the
tests don't produce extraneous output when there is a failure.
Fixes#38594
Change-Id: I7eb41bb02b2b1e78afd7849676b5c85bc11c759c
Reviewed-on: https://go-review.googlesource.com/c/go/+/229538
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This CL moves fixVariadicCall from mid-Walk of function calls to
early-Order, in preparation for moving it even earlier in the future.
Notably, rewriting variadic calls this early introduces two
compilation output changes:
1. Previously, Order visited the ODDDARG before the rest of the
arguments list, whereas the natural time to visit it is at the end of
the list (as we visit arguments left-to-right, and the ... argument is
the rightmost one). Changing this ordering permutes the autotmp
allocation order, which in turn permutes autotmp naming and stack
offsets.
2. Previously, Walk separately walked all of the variadic arguments
before walking the entire slice literal, whereas the more natural
thing to do is just walk the entire slice literal. This triggers
slightly different code paths for composite literal construction in
some cases.
Neither of these have semantic impact. They simply mean we're now
compiling f(a,b,c) the same way as we were already compiling
f([]T{a,b,c}...).
Change-Id: I40ccc5725697a116370111ebe746b2639562fe87
Reviewed-on: https://go-review.googlesource.com/c/go/+/229601
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Triggers a handful of times in std+cmd.
Change-Id: I9bb8ce9a5f8bae2547cb61157cd8f256e1b63e76
Reviewed-on: https://go-review.googlesource.com/c/go/+/229602
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Use a separate directory for TestBuildFortvOS test files.
Remove a bad comment in TestTrampoline.
Change-Id: I2dc07ae575ec3f73fb7cea26743094b11a41b464
Reviewed-on: https://go-review.googlesource.com/c/go/+/229619
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Allow go generate to process packages that contain invalid code. Ignore
errors when loading the package, but process only files which have a
valid package clause. Set $GOPACKAGE individually for each file, based
on the package clause.
Add test script for go generate and invalid packages.
Fixes#36422
Change-Id: I91ea088346a1548ccd6678b4595a527b948331ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/229097
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Recommend use of GOINSECURE over -insecure flang and clarify that GOINSECURE
environment variable does not also imply GONOSUMDB.
Updates #37519 by adding documentation as discussed.
Change-Id: Ia8ab6b3ed1aa559343b72e4ca76c372ee6bf1941
GitHub-Last-Rev: 8d86991f0c
GitHub-Pull-Request: golang/go#38572
Reviewed-on: https://go-review.googlesource.com/c/go/+/229223
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In mid-Walk, we rewrite calls to variadic functions to use explicit
slice literals; e.g., rewriting f(a,b,c) into f([]T{a,b,c}...).
However, it would be useful to do that rewrite much earlier in the
compiler, so that other compiler passes can be simplified.
This CL refactors the rewrite logic into a new fixVariadicCall
function, which subsequent CLs can more easily move into earlier
compiler passes.
Passes toolstash-check -race.
Change-Id: I408e655f2d3aa00446a2e6accf8765abc3b16a8a
Reviewed-on: https://go-review.googlesource.com/c/go/+/229486
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
ioutil.TempDir doesn't like path separators in its pattern. Modify
(*common).TempDir to replace path separators with underscores before
using the test name as a pattern for ioutil.TempDir.
Fixes#38465.
Change-Id: I9e8ae48b99648b2bf9f561762e845165aff01972
Reviewed-on: https://go-review.googlesource.com/c/go/+/229399
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
unsafe.Pointer safety rule #4 says "The compiler handles a Pointer
converted to a uintptr in the argument list of a call". Within escape
analysis, we've always required this be a single conversion
unsafe.Pointer->uintptr conversion, but the corresponding logic in
order is somewhat laxer, allowing arbitrary chains of OCONVNOPs from
unsafe.Pointer to uintptr.
This CL changes order to be stricter to match escape analysis.
Passes toolstash-check.
Change-Id: Iadd210d2123accb2020f5728ea2a47814f703352
Reviewed-on: https://go-review.googlesource.com/c/go/+/229578
Reviewed-by: Ian Lance Taylor <iant@golang.org>
