Michael Matloob [Wed, 12 Apr 2023 23:00:30 +0000 (19:00 -0400)]
cmd/go: parallelize part of loading test packages in list
load.TestPackagesAndErrors is given an optional done func() argument.
If set, load.TestPackagesAndErrors will perform part of its work
asynchronously and call done when done. This allows go list to run
testPackagesAndErrors so that the parallelizable parts of
TestPackagesAndErrors run in parallel, making go list -e faster.
wasm: restrict supported types in go:wasmimport function signatures
Changes the set of types supported in functions declared with the
go:wasmimport directive to only allow 32 bits and 64 bits integers
and floats, as well as unsafe.Pointer in parameters only. Both the
compiler code and the standard library are updated because the new
restrictions require modifying the use of go:wasmimport in the
syscall and runtime packages.
In preparation of enabling packages outside of the standard library
to use the go:wasmimport directive, the error messages are modified
to carry more context and use ErrorfAt instead of Fatalf to avoid
printing the compiler stack trace when a function with an invalid
signature is encountered.
Fixes #59156
Change-Id: Ied8317f8ead9c28f0297060ac35a5b5255ab49db
Reviewed-on: https://go-review.googlesource.com/c/go/+/483415 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Russ Cox [Mon, 13 Mar 2023 21:35:03 +0000 (17:35 -0400)]
go/parser: report //go:build-derived Go version in ast.File.GoVersion
For #57001, compilers and others tools will need to understand that
a different Go version can be used in different files in a program,
according to the //go:build lines in those files.
Update go/parser to populate the new ast.File.GoVersion field.
This requires running the go/scanner in ParseComments mode
always and then implementing discarding of comments in the
parser instead of the scanner. The same work is done either way,
since the scanner was already preparing the comment result
and then looping. The loop has just moved into go/parser.
Also make the same changes to cmd/compile/internal/syntax,
both because they're necessary and to keep in sync with go/parser.
Ian Lance Taylor [Wed, 12 Apr 2023 20:57:21 +0000 (13:57 -0700)]
slices: amortize allocations in Insert
Fixes #54948
Change-Id: I467afb940b539b100dcce687b05914a9da7b9ed2
Reviewed-on: https://go-review.googlesource.com/c/go/+/484159
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Valentin Deleplace <deleplace@google.com>
path/filepath: add test cases for walking a symlink-to-symlink-to-dir
The "double link with slash" test is skipped on darwin due to an
apparent kernel / libc bug. If the bug is present on other platforms
too, I'd like to know about it.
Reason for revert: it is trickier than expected to enforce an invariant that x.typ == Typ[Invalid] => x.mode == invalid. For example, builtins have invalid type until their call is evaluated.
I think it is better to keep this defensive code for now. My bad for suggesting this strictness. I will send a follow-up CL with a test that exercises the panic discovered inside Google, and a bit more commentary about what 'invalid' means in both contexts.
Fixes #59603
Change-Id: If291f7268e7ef7ae6cd9bb861bb9af349a729cb2
Reviewed-on: https://go-review.googlesource.com/c/go/+/484375
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Robert Griesemer [Wed, 12 Apr 2023 16:43:41 +0000 (09:43 -0700)]
go/types, types2: generalize tparamIndex to arbitrary type parameter lists
tparamIndex returns the index of a type parameter given the type
parameter and a list of type parameters. If an index >= 0 is returned,
it is the index assigned to the type parameter (TypeParam.index), and
the index of the type parameter in the provided list of parameters.
For it to work correctly, the type parameter list must be from a single
type parameter declaration.
To allow for lists of arbitrary type parameters (from different generic
signatures), change the implementation to do a linear search. The result
is the index of the type parameter in the provided type parameter list,
which may be different from the index assigned to the type parameter.
The linear search is likely fast enough since type parameter lists tend
to be very short.
Change-Id: I913f97fa4c042abeb535ee86ca6657241a4cf796
Reviewed-on: https://go-review.googlesource.com/c/go/+/483995 Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
cmd/internal/moddeps: preserve PWD more carefully in commands
On macOS, TMPDIR is typically a symlink, and the GOROOT for the
buildlet is in TMPDIR as well. PWD must be preserved in order for
os.Getwd (and functions based on it) to report paths that remain
relative to GOROOT, and paths relative to GOROOT are necessary in
order for filepath.Rel to report subdirectories as subdirectories
(rather than paths with long "../../…" prefixes).
Fortunately, the (*Cmd).Environ method added for #50599 makes
preserving PWD somewhat easier.
This fixes 'go test cmd/internal/moddeps' on the new
darwin-amd64-longtest builder.
cmd/go: skip TestScript/list_goroot_symlink on darwin
The list_goroot_symlink test relies on fsys.Walk (and ultimately
syscall.Lstat) conforming to POSIX pathname resolution semantics.
POSIX requires that symlinks ending in a slash be fully resolved,
but it appears that lstat in current darwin kernels does not fully
resolve the last pathname component when it is a symlink to a symlink.
Matthew Dempsky [Wed, 12 Apr 2023 19:41:45 +0000 (12:41 -0700)]
cmd/compile: fix reproducible build of aliased generic types
Due to a missing "&& !alias" check, the unified linker was treating
type aliases the same as defined types for the purpose of exporting
method bodies. The methods will get exported anyway alongside the
aliased type, so this mistake is normally harmless.
However, if multiple type aliases instantiated the same generic type
but with different type arguments, this could result in the
same (generic) method body being exported multiple times under
different symbol names. Further, because bodies aren't expected to be
exported multiple times, we were sorting them simply based on index.
And consequently, the sort wasn't total and is sensitive to the map
iteration order used while ranging over linker.bodies.
The fix is simply to add the missing "&& !alias" check, so that we
don't end up with duplicate bodies in the first place.
Thanks rsc@ for providing a minimal repro case.
Fixes #59571.
Change-Id: Iaa55968cc7110b601e2f0f9b620901c2d55f7014
Reviewed-on: https://go-review.googlesource.com/c/go/+/484155 Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Jonathan Amsterdam [Wed, 12 Apr 2023 14:07:35 +0000 (10:07 -0400)]
log/slog: catch panics in LogValue
If a LogValue call panics, recover and return an error instead.
The error contains some stack information to make it easier to
find the problem. A number of people complained that panics
in fmt.Formatter.Format functions are hard to debug because
there is no context.
This is an example of the error text:
LogValue panicked
called from log/slog.panickingLogValue.LogValue (/usr/local/google/home/jba/repos/go/src/log/slog/value_test.go:221)
called from log/slog.Value.resolve (/usr/local/google/home/jba/repos/go/src/log/slog/value.go:465)
called from log/slog.Value.Resolve (/usr/local/google/home/jba/repos/go/src/log/slog/value.go:446)
called from log/slog.TestLogValue (/usr/local/google/home/jba/repos/go/src/log/slog/value_test.go:192)
called from testing.tRunner (/usr/local/google/home/jba/repos/go/src/testing/testing.go:1595)
(rest of stack elided)
Fixes #59141.
Change-Id: I62e6ff6968d1aa34873e955c2d606d25418a673b
Reviewed-on: https://go-review.googlesource.com/c/go/+/484097
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
limeidan [Thu, 25 Aug 2022 03:14:15 +0000 (11:14 +0800)]
cmd/internal/sys, cmd/dist, misc/cgo/testcshared: enable c-shared feature and test on loong64
Linux kernel on loong64 has no Dup2 syscall support, so we use Dup3 to replace it like arm64 and riscv64.
Updates #53301
Fixes #58784
Change-Id: I4e0be140a71b86f4626ed39d76cf3ac78f842018
Reviewed-on: https://go-review.googlesource.com/c/go/+/425478
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: WANG Xuerui <git@xen0n.name> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: abner chenc <chenguoqi@loongson.cn>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: xiaodong liu <teaofmoli@gmail.com> Reviewed-by: WANG Xuerui <git@xen0n.name>
Jonathan Hall [Mon, 27 Mar 2023 11:23:36 +0000 (13:23 +0200)]
regexp: add Regexp.TextMarshaler/TextUnmarshaler
Fixes #46159
Change-Id: I51dc4e9e8915ab5a73f053690fb2395edbeb1151
Reviewed-on: https://go-review.googlesource.com/c/go/+/479401
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Implementation copied from testing/fstest/mapfs.go
Fixes #57803
Change-Id: I531682b50ab6663511bac41fce7614ab9197bf38
GitHub-Last-Rev: d9bcc1c0bf4d339edcdabaf4d4d67b848ad8818d
GitHub-Pull-Request: golang/go#59489
Reviewed-on: https://go-review.googlesource.com/c/go/+/483235
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: David Chase <drchase@google.com>
Jonathan Amsterdam [Wed, 5 Apr 2023 10:13:17 +0000 (06:13 -0400)]
log/slog: don't call SetDefault in examples
Replace the default Logger in some examples with a locally constructed
Logger.
Calling SetDefault changes global state that could affect other tests.
Although we could use a defer to restore the state, that clutters
the example and would not work if tests were run concurrently.
Change-Id: Ib2595c57f8e6c3e0b39b982f682ba287c2ae249d
Reviewed-on: https://go-review.googlesource.com/c/go/+/482475
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com>
Jonathan Amsterdam [Tue, 4 Apr 2023 11:08:57 +0000 (07:08 -0400)]
log,log/slog: get correct source line when slog calls log
Before slog.SetDefault is called the first time, calls to slog's
default Logger invoke log's default Logger.
Originally, this was done by calling log.Output. This caused source
line information to be wrong sometimes, because log.Output requires a
call depth and the code invoking it could not know how many calls were
between it and the original logging call (slog.Info, etc.). The line
information would be right if the default handler was called directly,
but wrong if it was wrapped by another handler. The handler has the pc
of the logging call, but it couldn't give that pc to the log package.
This CL fixes the problem by adding a function in the log package
that uses the pc instead of a call depth, and making that function
available to slog.
The simplest way to add pc functionality to the log package is to add
a pc argument to Logger.output, which uses it only if it's not zero.
To make that function visible to slog without exporting it, we store
the function in a variable that lives in the new log/internal package.
Change-Id: I0bb6daebb4abc518a7ccc4e6d2f3c1093b1d0fe4
Reviewed-on: https://go-review.googlesource.com/c/go/+/482239
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com> Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
net/http: avoid leaking writer goroutines in tests
In TestTransportPrefersResponseOverWriteError and TestMaxBytesHandler,
the server may respond to an incoming request without ever reading the
request body. The client's Do method will return as soon as the
server's response headers are read, but the Transport will remain
active until it notices that the server has closed the connection,
which may be arbitrarily later.
When the server has closed the connection, it will call the Close
method on the request body (if it has such a method). So we can use
that method to find out when the Transport is close enough to done for
the test to complete without interfering too much with other tests.
net/http: only report the first leak of each test run
We don't have a way to terminate the leaked goroutines, and we can't
wait forever for them to exit (or else we would risk timing out the
test and losing the log line describing what exactly leaked).
So we have reason to believe that they will remain leaked while we run
the next test, and we don't want the goroutines from the first leak to
generate a spurious error when the second test completes.
This also removes a racy Parallel call I added in CL 476036, which was
flagged by the race detector in the duplicate-suppression check.
(I hadn't considered the potential interaction with the leak checker.)
Change-Id: Ib5bcd3d1e9618d65b4d4b0895d0e40dbd76646c6
GitHub-Last-Rev: 174084ca6c67ff99a4b3628d213b2de198f74dd0
GitHub-Pull-Request: golang/go#59516
Reviewed-on: https://go-review.googlesource.com/c/go/+/483435
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
qmuntal [Wed, 29 Mar 2023 08:28:33 +0000 (10:28 +0200)]
syscall: implement Fchdir on Windows
This CL adds support for os.File.Chdir() on Windows by implementing
syscall.Fchdir, which is internally used by Chdir.
Windows does not provide a function that sets the working directory
using a file handle, so we have to fallback to retrieving the file
handle path and then use it in SetCurrentDirectory.
Change-Id: I2ae93575e50411e5a9426ea531541958d7c9e812
Reviewed-on: https://go-review.googlesource.com/c/go/+/480135
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Pull in CL 483375. This also updates golang.org/x/sys to v0.7.0 and thus
we also need to update it to that version in cmd to keep
TestDependencyVersionsConsistent happy.
Russ Cox [Thu, 19 Jan 2023 22:04:07 +0000 (17:04 -0500)]
cmd/dist: add -distpack flag to build distribution archives
We want to enable others to reproduce the exact distribution archives
we are serving on go.dev/dl. Today the code for building those
archives lives in golang.org/x/build, which is fundamentally tied to
running on the Go team build infrastructure and not easy for others to
run. This CL adds a new flag -distpack to cmd/dist, usually invoked as
make.bash -distpack, to build the distribution archives using code in
the main repository that anyone can run. Starting in Go 1.21,
the Go team build infrastructure will run this instead of its current
custom code to build those archives.
The current builds are not reproducible even given identical
infrastructure, because the archives are stamped with the current
time. It is helpful to have a timestamp in the archives indicating
when the code is from, but that time needs to be reproducible.
To ensure this, the new -distpack flag extends the VERSION file to
include a time stamp, which it uses as the modification time for all
files in the archive.
The new -distpack flag is implemented by a separate program,
cmd/distpack, instead of being in cmd/dist, so that it can be compiled
by the toolchain being distributed and not the bootstrap toolchain.
Otherwise details like the exact compression algorithms might vary
from one bootstrap toolchain to another and produce non-reproducible
builds. So there is a new 'go tool distpack', but it's omitted from
the distributions themselves, just as 'go tool dist' is.
make.bash already accepts any flags for cmd/dist, including -distpack.
make.bat is less sophisticated and looks for each known flag, so this
CL adds an update to look for -distpack. The CL also changes make.bat
to accept the idiomatic Go -flagname in addition to the non-idiomatic
(for Go) --flagname. Previously it insisted on the --flag form.
I have confirmed that using make.bash -distpack produces the
identical distribution archives for windows/amd64, linux/amd64,
darwin/amd64, and darwin/arm64 whether it is run on
windows/amd64, linux/amd64, or darwin/amd64 hosts.
syscall: add a Go directive "go:nocheckptr" to forkAndExecInChild1
forkAndExecInChild1 func must not acquire any locks in child, because
they might have been locked at the time of the fork. This implies to
no rescheduling, no malloc calls, and no new stack segments.
So, doing a "checkptrAlignment" is bad here, because checkptr
functions added by the instrumentation could grow the stack, which
should not be done between fork and exec calls.
Hence using a Go directive "go:nocheckptr" to forkAndExecInChild1
func,so that the compiler should not do "checkptrAlignment" when
functions marked with "go:norace".
This race detection bug was caught in go 1.21 on s390x.
Running a "./race.bash" script from "go/src" directory failed and the
bug details are provided in issue link mentioned below.
Fixes #58785
Change-Id: I254091368b0789d886acdf26f8aa8d8f5a986b24
Reviewed-on: https://go-review.googlesource.com/c/go/+/481415
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Johan Brandhorst-Satzkorn [Sun, 26 Mar 2023 00:01:38 +0000 (17:01 -0700)]
all: add wasip1 support
Fixes #58141
Co-authored-by: Richard Musiol <neelance@gmail.com> Co-authored-by: Achille Roussel <achille.roussel@gmail.com> Co-authored-by: Julien Fabre <ju.pryz@gmail.com> Co-authored-by: Evan Phoenix <evan@phx.io>
Change-Id: I49b66946acc90fdf09ed9223096bfec9a1e5b923
Reviewed-on: https://go-review.googlesource.com/c/go/+/479627
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Bypass: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Tue, 11 Apr 2023 18:54:46 +0000 (11:54 -0700)]
crypto/internal/nistec: update generator for re-enabled s390x asm
CL 404058 modified the generated file p256.go without modifying
the generator program. This CL updates the generator program so
that it generates the p256.go file currently in the tree.
For #52709
Change-Id: I86927a8bb84b2e17426b7ba74a86f1aba781628e
Reviewed-on: https://go-review.googlesource.com/c/go/+/483715
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Ian Lance Taylor [Tue, 11 Apr 2023 18:57:47 +0000 (11:57 -0700)]
time: update windows zoneinfo_abbrs
While running "go generate" I noticed that there has been a change.
For #58113
Change-Id: Icca349d0a100dd4b11d34e4bbcfd1aef57346db7
Reviewed-on: https://go-review.googlesource.com/c/go/+/483716
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Quim Muntal <quimmuntal@gmail.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Ian Lance Taylor [Tue, 11 Apr 2023 19:01:43 +0000 (12:01 -0700)]
all: re-run stringer
Re-run all go:generate stringer commands. This mostly adds checks
that the constant values did not change, but does add new strings
for the debug/dwarf and internal/pkgbits packages.
Change-Id: I5fc41f20da47338152c183d45d5ae65074e2fccf
Reviewed-on: https://go-review.googlesource.com/c/go/+/483717 Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Russ Cox [Mon, 13 Mar 2023 21:12:40 +0000 (17:12 -0400)]
go/ast: add File.GoVersion
For #57001, compilers and others tools will need to understand that
a different Go version can be used in different files in a program,
according to the //go:build lines in those files.
This CL adds a GoVersion string field to ast.File, to allow exposing this
per-file Go version information.
Russ Cox [Mon, 13 Mar 2023 21:23:13 +0000 (17:23 -0400)]
go/build/constraint: add GoVersion
For #57001, programs need to be able to deduce the Go version
implied by a given build constraint. GoVersion determines that,
by discarding all build tags other than Go versions and computing
the minimum Go version implied by the resulting expression.
Will Hawkins [Wed, 5 Apr 2023 07:15:56 +0000 (03:15 -0400)]
runtime: filter i/o async entries using completion key on windows
In the case where a user program requests overlapped I/O directly on a
handlethat is managed by the runtime, it is possible that
runtime.netpoll will attempt to dereference a pointer with an invalid
value. This CL prevents the runtime from accessing the invalid pointer
value by adding a special key to each overlapped I/O operation that it
creates.
Fixes #58870
Co-authored-by: quimmuntal@gmail.com
Change-Id: Ib58ee757bb5555efba24c29101fc6d1a0dedd61a
Reviewed-on: https://go-review.googlesource.com/c/go/+/482495 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Joel Sing [Thu, 6 Apr 2023 20:15:59 +0000 (06:15 +1000)]
cmd/asm,cmd/internal/obj/x86: add RDPID instruction to x86 assembler
Add support for the Read Processor ID (RDPID) instruction to the x86
assembler. This returns the current logical processor's ID in the
specified register, as a faster alternative to RDTSCP.
Fixes #56525
Change-Id: I43482e42431dfc385ce2e7f6d44b9746b0cc4548
Reviewed-on: https://go-review.googlesource.com/c/go/+/482955
Run-TryBot: Joel Sing <joel@sing.id.au> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Daniel Frederick Crisman [Mon, 10 Apr 2023 23:14:56 +0000 (23:14 +0000)]
doc: add missing oxford comma in Constants
In the language specification under "Constants" the lists matching default
types to untyped contstant types is missing an Oxford comma in the first
list. I found a number of other places in the spec and #23442 that use the
Oxford comma to support its use.
Add missing Oxford comma in Constants default type list.
Change-Id: I4562d692610334bc82452db076145d2414617a04
GitHub-Last-Rev: 8acdb60d6e255f73fdeb908d2540d4ee35db3fd7
GitHub-Pull-Request: golang/go#59528
Reviewed-on: https://go-review.googlesource.com/c/go/+/483555
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
cmd/go/internal/cache: return and check errors from Trim
It's fine to ignore errors when reading trim.txt, since it might not
exist or might be corrupted. However, if we encounter an error in
writing the file, we will end up trimming again at every cmd/go
invocation, which will cause invocations to become progressively
slower (because each command will check more and more cache files for
trimming).
Although that situation would not cause the output of any 'go' command
to be invalid, it still seems better to escalate the problem to the
user to be fixed instead of proceeding in a degraded state.
Returning an explicit error also allows TestCacheTrim to skip if the
Trim error indicates that a required operation (in this case, file
locking) is not supported by the platform or filesystem.
Update to use the common macros to ensure all ELFv2 callee-save registers
are saved properly when transitioning from ELFv2 to Go calling/stack
conventions. Simplify the inlined Go function call, and remove the asm
hacks which inhibited implicit stack frame management.
Change-Id: Iee118a4069962a791436c6fe19370e1929404a8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/479795 Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com> Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In rare cases, elf will get a corrupted section starts with 0x1,
which happens to be COMPRESS_ZLIB that causing decompress twice.
This CL drops sectionData decompress data after open section.
Fixes #59208
Change-Id: I7999a55868b3b3481509e1ac35985f7580f0f688
Reviewed-on: https://go-review.googlesource.com/c/go/+/480895
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Benny Siegert <bsiegert@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: M Zhuo <mzh@golangcn.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
WANG Xuerui [Tue, 28 Mar 2023 13:10:16 +0000 (21:10 +0800)]
cmd/internal/obj/loong64: auto-align loop heads to 16-byte boundaries
CL 479816 took care of loops in hand-written assembly, but did not
account for those written in Go, that may become performance-sensitive
as well.
In this patch, all loop heads are automatically identified and aligned
to 16-byte boundaries, by inserting a synthetic `PCALIGN $16` before
them. "Loop heads" are defined as targets of backward branches.
While at it, tweak some of the local comments so the flow is hopefully
clearer.
Because LoongArch instructions are all 32 bits long, at most 3 NOOPs
can be inserted for each target Prog. This may sound excessive, but
benchmark results indicate the current approach is overall profitable
anyway.
The slight regression on the Regexp cases is likely because the previous
numbers are just coincidental: indeed, large regressions or improvements
(of roughly ±10%) happen with definitely irrelevant changes during
development. This CL should (hopefully) bring such random performance
fluctuations down a bit.
Change-Id: I8bdda6e65336da00d4ad79650937b3eeb9db0e7c
Reviewed-on: https://go-review.googlesource.com/c/go/+/479817 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: abner chenc <chenguoqi@loongson.cn> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: WANG Xuerui <git@xen0n.name>
Daniel Martí [Thu, 12 Jan 2023 18:29:28 +0000 (18:29 +0000)]
go/printer: reuse go/token.FileSet.PositionFor calls in setPos
setPos is called for most nodes, and in a number of cases,
the position is already the same.
PositionFor is a relatively expensive call,
as it needs to "unpack" a token.Pos into a token.Position.
If we can tell that the position is the same in a cheap way,
we can then avoid calling setPos and PositionFor.
Luckily, we can get the position's offset within the file,
and it doesn't involve the relatively expensive unpacking.
name old time/op new time/op delta
PrintFile-16 4.79ms ± 1% 4.36ms ± 1% -8.88% (p=0.008 n=5+5)
name old speed new speed delta
PrintFile-16 10.8MB/s ± 1% 11.9MB/s ± 1% +9.73% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
PrintFile-16 106kB ± 1% 106kB ± 1% ~ (p=0.167 n=5+5)
name old allocs/op new allocs/op delta
PrintFile-16 2.42k ± 0% 2.42k ± 0% ~ (all equal)
This does assume that the positions of a node being printed are all
within a file, as go/token.Position.Offset is relative to each file.
This seems like a perfectly fine assumption to make right now,
as the largest node which can be printed is an *ast.File.
Change-Id: I2ae55f507ba8ba9f280898c9c8e01c994d9b2a26
Reviewed-on: https://go-review.googlesource.com/c/go/+/461739 Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Wayne Zuo [Thu, 2 Mar 2023 07:55:03 +0000 (15:55 +0800)]
cmd/compile: split DIVV/DIVVU op on loong64
Previously, we need calculate both quotient and remainder together.
However, in most cases, only one result is needed. By separating these
instructions, we can save one instruction in most cases.
Change-Id: I0a2d4167cda68ab606783ba1aa2720ede19d6b53
Reviewed-on: https://go-review.googlesource.com/c/go/+/475315 Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org> Reviewed-by: abner chenc <chenguoqi@loongson.cn>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
cui fliter [Sun, 9 Apr 2023 14:51:47 +0000 (22:51 +0800)]
doc: fix "are" to "or" in core types
Fixes #59506
Change-Id: I2f8b92e93b706b061ca0eb0bd52e5cf798ce9ede
Reviewed-on: https://go-review.googlesource.com/c/go/+/483358 Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Bypass: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
WANG Xuerui [Fri, 7 Apr 2023 18:06:18 +0000 (02:06 +0800)]
cmd/internal/obj/arm64: improve comment and signature of pcAlignPadLength
The function just calculates the number of needed padding bytes,
instead of actually carrying out the alignment operation. And it has
the context argument at the end of the argument list, while contexts
idiomatically come first. Indeed, this is the only case in
cmd/internal/obj where ctxt is not the only argument and does not come
first.
Fix those two nits; no functional change intended.
Suggested by Ian during review of CL 479815 (that introduces a copy of
this helper into the loong64 port).
Change-Id: Ieb221ead23282abe6e04804d537e1234c7ab21d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/483155
Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
net: avoid opening an external port in TestDialListenerAddr
The behavior in #18806 can be tested with a localhost-only port,
provided that we're willing to assume what format the listener would
report for an external dual-stack port.
Ian Lance Taylor [Fri, 7 Apr 2023 02:36:01 +0000 (19:36 -0700)]
cmd/doc: format field doc comments when printing entire struct
cmd/doc passes structs to go/format, but that means that comments
on fields within those structs don't look like what cmd/doc prints
when asked for a struct field directly. Tweak the field comments
so that they look the same either way.
Fixes #56592
Change-Id: I198cb7a58e3d8558406c386072c630332f91c6b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/483055
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Michael Matloob [Sat, 8 Apr 2023 01:13:30 +0000 (21:13 -0400)]
cmd/go: localize computation of deps/depserrors in list
Stop depending on DepsErrors to report errors to the user and instead
only use it and compute it in list. Instead, use Incomplete to figure
out when a package or its depencies have an error, and only if they
do, do the work of finding all those errors.
For #59157
Change-Id: Ied927f53e7b1f66fad9248b40dd11ed960b3ef91
Reviewed-on: https://go-review.googlesource.com/c/go/+/483495
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
Ian Lance Taylor [Tue, 4 Apr 2023 21:20:26 +0000 (14:20 -0700)]
misc/cgo: remove +build lines, add go:build where needed
Change-Id: Iae6ac32db5c2aacb323793a7e0dc34e09648d035
Reviewed-on: https://go-review.googlesource.com/c/go/+/482295
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Tue, 4 Apr 2023 20:59:06 +0000 (13:59 -0700)]
misc/cgo: gofmt
Change-Id: I5d02279d0593a8368b2f249a6b53650b89aed7b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/482275 Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Pratt [Tue, 28 Mar 2023 16:45:17 +0000 (12:45 -0400)]
cmd/compile: don't inline hot calls into big functions
Standard inlining has a reduced maximum cost of callees (20 instead of
80) when inlining into a "big" function, to limit how much bigger we
will make an already big function.
When adding PGO hot call budget increases, we inadvertently bypassed
this "big" function restriction, allowing hot calls of up to
inlineHotMaxBudget, even into big functions.
Add the restriction back, even for hot calls. If a function is already
very large, we probably shouldn't inline even more.
A very important note here is that function "big"-ness is computed prior
to any inlining. One potential problem with PGO is that many hot calls
inline into an initially-small function and ultimately make it very
large. This CL does nothing to address that case, which would require
recomputing size after inlining.
This CL has no impact on sweet PGO benchmarks. I specifically dug into
tile38, which contained 0 hot big functions. Other benchmarks are
probably similar.
Change-Id: I3b6304eaf7738a219359d4b8bb121d68babfea8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/482157
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Michael Matloob [Mon, 3 Apr 2023 20:07:41 +0000 (16:07 -0400)]
cmd/go: return and handle errors from loadImport for bad imports
And assign the error to the importing package. Before this change, for
some errors for bad imports, such as importing a vendored package with
the wrong path, we would make a dummy package for the imported package
with the error on it. Instead report the error on the importing
package where it belongs. Do so by returning an error from loadImport
and handling it on the importing package.
For #59157
Change-Id: I952e1a82af3857efc5da4fd3f8bc6be473a60cf5
Reviewed-on: https://go-review.googlesource.com/c/go/+/482877 Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Daniel Martí [Fri, 7 Apr 2023 17:41:10 +0000 (18:41 +0100)]
reflect: remove typedmemmovepartial as it is unused
It appears to have been unused since https://go.dev/cl/298670
in April 2021, as that change removed its only use.
It is always in the git history if it is needed again.
Change-Id: Ie55d059c102dfaa75bd253e09d48a4b30f45e941
Reviewed-on: https://go-review.googlesource.com/c/go/+/483136 Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
qmuntal [Thu, 9 Mar 2023 13:55:31 +0000 (14:55 +0100)]
runtime: support GOTRACEBACK=wer on Windows
GOTRACEBACK=wer is a new traceback level that acts as "crash" and
also enables WER. The same effect can be achieved using
debug.SetTraceback("wer").
The Go runtime currently crashes using exit(2), which bypasses WER
even if it is enabled. To best way to trigger WER is calling
RaiseFailFastException [1] instead, which internally launches the
WER machinery.
This CL also changes how GOTRACEBACK=crash crashes, so both "wer" and
"crash" crash using RaiseFailFastException, which simplifies the
implementation and resolves a longstanding TODO.
cmd/compile: teach prove about bitwise OR operation
For now, only apply the rule if either of arguments are constants. That
would catch a lot of real user code, without slowing down the compiler
with code generated for string comparison (experience in CL 410336).
Updates #57959
Fixes #45928
Change-Id: Ie2e830d6d0d71cda3947818b22c2775bd94f7971
Reviewed-on: https://go-review.googlesource.com/c/go/+/483359
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
limeidan [Thu, 25 Aug 2022 03:07:25 +0000 (11:07 +0800)]
runtime: add support for --buildmode=c-shared on loong64
These c-shared related CLs are follow up of CLs 455016, 455017, 455018. Here we
follow the LoongArch ELF psABI v2 standard, which requires the support of the
PCALAU12I instruction.
Updates #53301
Updates #58784
Change-Id: I7f1ddbf3b2470d610f12069d147aa9b3a6a96f32
Reviewed-on: https://go-review.googlesource.com/c/go/+/425474 Reviewed-by: abner chenc <chenguoqi@loongson.cn> Reviewed-by: WANG Xuerui <git@xen0n.name> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
WANG Xuerui [Sat, 3 Dec 2022 13:16:49 +0000 (21:16 +0800)]
cmd/internal/obj/loong64, cmd/link/internal: switch to LoongArch ELF psABI v2 relocs
The LoongArch ELF psABI v2 [1] relocs are vastly simplified from the v1
which involved a stack machine for computing the reloc values, but the
details of PC-relative addressing are changed as well. Specifically, the
`pcaddu12i` instruction is substituted with the `pcalau12i`, which is
like arm64's `adrp` -- meaning the lower bits of a symbol's address now
have to be absolute and not PC-relative.
However, apart from the little bit of added complexity, the obvious
advantage is that only 1 reloc needs to be emitted for every kind of
external reloc we care about. This can mean substantial space savings
(each RELA reloc occupies 24 bytes), and no open-coded stack ops has to
remain any more.
While at it, update the preset value for the output ELF's flags to
indicate the psABI update.
Change-Id: I5c13bc710eaf58293a32e930dd33feff2ef14c28
Reviewed-on: https://go-review.googlesource.com/c/go/+/455017
Run-TryBot: Ben Shi <powerman1st@163.com> Reviewed-by: xiaodong liu <teaofmoli@gmail.com> Reviewed-by: abner chenc <chenguoqi@loongson.cn>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Meidan Li <limeidan@loongson.cn>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
WANG Xuerui [Sun, 4 Dec 2022 07:06:45 +0000 (15:06 +0800)]
cmd/compile, cmd/dist, internal: enable buildmode=pie for linux/loong64
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>
Joel Sing [Wed, 5 Apr 2023 20:11:14 +0000 (06:11 +1000)]
cmd/compile/internal/amd64: improve fix up code for signed division
In order to avoid a CPU exception resulting from signed overflow, the signed
division code tests if the divisor is -1 and if it is, runs fix up code to
manually compute the quotient and remainder (thus avoiding IDIV and potential
signed overflow).
However, the way that this is currently structured means that the normal code
path for the case where the divisor is not -1 results in five instructions
and two branches (CMP, JEQ, followed by sign extension, IDIV and another JMP
to skip over the fix up code).
Rework the fix up code such that the final JMP is incurred by the less likely
divisor is -1 code path, rather than more likely code path (which is already
more expensive due to IDIV). This result in a four instruction sequence
(CMP, JNE, sign extension, IDIV), with only a single branch.
Updates #59089
Change-Id: Ie8d065750a178518d7397e194920b201afeb0530
Reviewed-on: https://go-review.googlesource.com/c/go/+/482658
Run-TryBot: Joel Sing <joel@sing.id.au> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Joel Sing [Wed, 5 Apr 2023 20:11:10 +0000 (06:11 +1000)]
cmd/compile/internal/amd64: simplify code generation for signed division
The same switch statement handles code generation for signed division of
words, double words and quad words. Rather than using multiple switch
statements to select the appropriate instructions, determine all of the
correctly sized operands up front, then use them as needed.
Updates #59089
Change-Id: I2b7567c8e0ecb9904c37607332538c95b0521dca
Reviewed-on: https://go-review.googlesource.com/c/go/+/482657
Run-TryBot: Joel Sing <joel@sing.id.au> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Ian Lance Taylor [Fri, 7 Apr 2023 21:03:55 +0000 (14:03 -0700)]
slices: add DeleteFunc
Fixes #54768
Change-Id: I588ae33c13e0bbd9d324c11771667b22a864047d
Reviewed-on: https://go-review.googlesource.com/c/go/+/483175 Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Eli Bendersky <eliben@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Keith Randall [Sat, 1 Apr 2023 17:33:26 +0000 (10:33 -0700)]
cmd/compile: use correct type for byteswaps on multi-byte stores
Use the type of the store for the byteswap, not the type of the
store's value argument.
Normally when we're storing a 16-bit value, the value being stored is
also typed as 16 bits. But sometimes it is typed as something smaller,
usually because it is the result of an upcast from a smaller value,
and that upcast needs no instructions.
If the type of the store's arg is thinner than the type being stored,
and the byteswap'd value uses that thinner type, and the byteswap'd
value needs to be spilled & restored, that spill/restore happens using
the thinner type, which causes us to lose some of the top bits of the
value.
Fixes #59367
Change-Id: If6ce1e8a76f18bf8e9d79871b6caa438bc3cce4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/481395 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cherry Mui [Fri, 7 Apr 2023 00:26:55 +0000 (20:26 -0400)]
cmd/internal/objfile: align the load address of ELF binary
The ELF ABI just requires that the address and the offset of a
segment are congruent modulo the alignment, but does not require
the start address to be aligned. While usually the segment's
start address is aligned, apparently the zig linker generates
binary with unaligned address.
At the run time, the memory mapping that contains the segment
starts at an aligned address (rounding down). Use the aligned
address for the load address, which matches the mapping.
Apparently this is what the pprof library expects.
Fixes #59466.
Change-Id: Ife78909b20b7bc975ac4c76f2c5f5db325ddec9b
Reviewed-on: https://go-review.googlesource.com/c/go/+/483035
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
WANG Xuerui [Tue, 4 Apr 2023 08:35:06 +0000 (16:35 +0800)]
cmd/link: bump loong64 function alignment to 16 bytes
The loong64 PCALIGN directive works with PCs relative to beginning of
functions. So if the function alignment is less than that requested by
PCALIGN, the following code may in fact not be aligned as such, leading
to unexpected performance.
The current function alignment on loong64 is 8 bytes, which seems to
stem from mips64 or riscv64. In order to make performance more
predictable on loong64, it is raised to 16 bytes to ensure that at
least `PCALIGN $16` works.
As alignment of loops written in Go is yet to be tackled, and the
codegen is not otherwise touched, benchmark numbers for this change are
not going to be meaningful, and not included.
Change-Id: I2120ef3746ce067e274920c82091810073bfa3be
Reviewed-on: https://go-review.googlesource.com/c/go/+/481936
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: WANG Xuerui <git@xen0n.name> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Keith Randall <khr@golang.org>
WANG Xuerui [Tue, 28 Mar 2023 11:58:17 +0000 (19:58 +0800)]
internal/bytealg, runtime: align some loong64 asm loops to 16-byte boundaries
The LA464 micro-architecture is very sensitive to alignment of loops,
so the final performance of linked binaries can vary wildly due to
uncontrolled alignment of certain performance-critical loops. Now that
PCALIGN is available on loong64, let's make use of it and manually align
some assembly loops. The functions are identified based on perf records
of some easily regressed go1 benchmark cases (e.g. FmtFprintfPrefixedInt,
RegexpMatchEasy0_1K and Revcomp are particularly sensitive; even those
optimizations purely reducing dynamic instruction counts can regress
those cases by 6~12%, making the numbers almost useless).
Benchmark results on Loongson 3A5000 (which is an LA464 implementation):
Change-Id: I9ac28d936e03d21c46bb19fa100018f61ace6b42
Reviewed-on: https://go-review.googlesource.com/c/go/+/479816
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: WANG Xuerui <git@xen0n.name> Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Keith Randall <khr@golang.org>
WANG Xuerui [Tue, 28 Mar 2023 11:30:04 +0000 (19:30 +0800)]
cmd/link, cmd/internal/obj/loong64: support the PCALIGN directive
Allow writing `PCALIGN $imm` where imm is a power-of-2 between 8 and
2048 (inclusive), for ensuring that the following instruction is
placed at an imm-byte boundary relative to the beginning of the
function. If the PC is not sufficiently aligned, NOOPs will be
inserted to make it so, otherwise the directive will do nothing.
This could be useful for both asm performance hand-tuning, and future
scenarios where a certain bigger alignment might be required.
Change-Id: Iad6244669a3d5adea88eceb0dc7be1af4f0d4fc9
Reviewed-on: https://go-review.googlesource.com/c/go/+/479815 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: WANG Xuerui <git@xen0n.name>
Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: abner chenc <chenguoqi@loongson.cn>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Daniel Martí [Fri, 7 Apr 2023 17:21:00 +0000 (18:21 +0100)]
encoding/xml: use reflect.Value.Grow
Like https://go.dev/cl/481376 did for encoding/gob,
but now for encoding/xml, which had very similar code.
One minor difference is that encoding/xml now needs a SetLen before the
call to Index, as otherwise we index just past the length.
Still, calling Grow and SetLen is easier to understand,
and avoids needing to make a new zero value.
goos: linux
goarch: amd64
pkg: encoding/xml
cpu: AMD Ryzen 7 PRO 5850U with Radeon Graphics
│ old │ new │
│ sec/op │ sec/op vs base │
Unmarshal-8 6.904µ ± 1% 6.980µ ± 1% +1.10% (p=0.009 n=6)
│ old │ new │
│ B/op │ B/op vs base │
Unmarshal-8 7.656Ki ± 0% 7.586Ki ± 0% -0.92% (p=0.002 n=6)
│ old │ new │
│ allocs/op │ allocs/op vs base │
Unmarshal-8 188.0 ± 0% 185.0 ± 0% -1.60% (p=0.002 n=6)
Change-Id: Id83feb467a9c59c80c7936aa892780aae7e8b809
Reviewed-on: https://go-review.googlesource.com/c/go/+/483135
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Ian Lance Taylor <iant@google.com>
David Chase [Thu, 6 Apr 2023 18:42:53 +0000 (14:42 -0400)]
cmd/compile: remove broken LEA "optimization"
CL 440035 added rewrite rules to simplify "costly" LEA
instructions, but the types in the rewrites were wrong and
the code would go bad if the wrong-typed register was spilled.
CL 482536 attempted to fix this by correcting the type in the
rewrite, but that "fix" broke something on windows-amd64-race.
Instead / for-now, remove the offending rewrite rules.
Updates #21735.
Fixes #59432.
Change-Id: I0497c42db414f2055e1378e0a53e2bceee9cd5d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/482820
Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Damien Neil [Fri, 7 Apr 2023 17:04:42 +0000 (10:04 -0700)]
net/http: wait forever for write results in tests
After performing a round trip on a connection, the connection is
usually returned to the idle connection pool. If the write of the
request did not complete successfully, the connection is not
returned.
It is possible for the response to be read before the write
goroutine has finished signalling that its write has completed.
To allow for this, the check to see if the write completed successfully
waits for 50ms for the write goroutine to report the result of the
write.
See comments in persistConn.wroteRequest for more details.
On a slow builder, it is possible for the write goroutine to take
longer than 50ms to report the status of its write, leading to test
flakiness when successive requests unexpectedly use different connections.
Set the timeout for waiting for the writer to an effectively
infinite duration in tests.
net/http: fix a race in TestResponseControllerSetPastReadDeadline
If the Write goroutine is delayed for long enough after its first
Write, the handler may have closed both the readc and donec channels
by the time it selects over them, and the donec case may be randomly
chosen. Handle that case by explicitly checking readc as well.
Than McIntosh [Tue, 4 Apr 2023 22:31:46 +0000 (18:31 -0400)]
cmd/compile: allow more inlining of functions that construct closures
[This is a roll-forward of CL 479095, which was reverted due to a bad
interaction between inlining and escape analysis since fixed in CL 482355.]
Currently, when the inliner is determining if a function is
inlineable, it descends into the bodies of closures constructed by
that function. This has several unfortunate consequences:
- If the closure contains a disallowed operation (e.g., a defer), then
the outer function can't be inlined. It makes sense that the
*closure* can't be inlined in this case, but it doesn't make sense
to punish the function that constructs the closure.
- The hairiness of the closure counts against the inlining budget of
the outer function. Since we currently copy the closure body when
inlining the outer function, this makes sense from the perspective
of export data size and binary size, but ultimately doesn't make
much sense from the perspective of what should be inlineable.
- Since the inliner walks into every closure created by an outer
function in addition to starting a walk at every closure, this adds
an n^2 factor to inlinability analysis.
This CL simply drops this behavior.
In std, this makes 57 more functions inlinable, and disallows inlining
for 10 (due to the basic instability of our bottom-up inlining
approach), for an net increase of 47 inlinable functions (+0.6%).
This will help significantly with the performance of the functions to
be added for #56102, which have a somewhat complicated nesting of
closures with a performance-critical fast path.
The downside of this seems to be a potential increase in export data
and text size, but the practical impact of this seems to be
negligible:
Than McIntosh [Tue, 4 Apr 2023 22:56:21 +0000 (18:56 -0400)]
cmd/compile: deadcode unreferenced hidden closures during inlining
When a closure is inlined, it may contain other hidden closures, which
the inliner will duplicate, rendering the original nested closures as
unreachable. Because they are unreachable, they don't get processed in
escape analysis, meaning that go/defer statements don't get rewritten,
which can then in turn trigger errors in walk. This patch looks for
nested hidden closures and marks them as dead, so that they can be
skipped later on in the compilation flow. NB: if during escape
analysis we rediscover a hidden closure (due to an explicit reference)
that was previously marked dead, revive it at that point.
Fixes #59404.
Change-Id: I76db1e9cf1ee38bd1147aeae823f916dbbbf081b
Reviewed-on: https://go-review.googlesource.com/c/go/+/482355
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Than McIntosh <thanm@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Ian Lance Taylor [Thu, 6 Apr 2023 23:38:47 +0000 (16:38 -0700)]
runtime: permit core dumps in darwin-amd64
Previously we did not permit them as Go programs generated enormous
core dumps on macOS. However, according to an investigation in #59446,
they are OK now.
For #59446
Change-Id: I1d7a3f500a6bc525aa6de8dfa8a1d8dbb15feadc
Reviewed-on: https://go-review.googlesource.com/c/go/+/483015
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
cmd/link/internal/ld: don't set IMAGE_FILE_DEBUG_STRIPPED on PE binaries
The IMAGE_FILE_DEBUG_STRIPPED characteristic is used to inform that
the debugging information have been removed from the PE files and moved
into a DBG file, but the Go linker doesn't generate DBG files.
Having this characteristic can confuse debugging tools, so better
don't set it.
While here, remove also IMAGE_FILE_LINE_NUMS_STRIPPED, which is
deprecated and should be zero [1].