Appending to a global slice is only safe if its length is already
equal to its capacity. That property is not guaranteed for slices in
general, and empirically does not hold for this one.
This is a minimal fix to make it easier to backport.
A more robust cleanup of the base.EnvForDir function will be sent in a
subsequent CL.
Fixes#38077
Updates #37940
Change-Id: I731d5bbd0e516642c2cf43e713eeea15402604e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/225577
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Currently, the capping logic for the GC trigger ratio is such that if
gcpercent is low, we may end up setting the trigger ratio far too high,
breaking the promise of SetGCPercent and GOGC has a trade-off knob (we
won't start a GC early enough, and we will use more memory).
This change modifies the capping logic for the trigger ratio by scaling
the minTriggerRatio with gcpercent the same way we scale
maxTriggerRatio.
Fixes#37927.
Change-Id: I2a048c1808fb67186333d3d5a6bee328be2f35da
Reviewed-on: https://go-review.googlesource.com/c/go/+/223937
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This makes all modern public keys in the standard library implement a
common interface (below) that can be used by applications for better
type safety and allows for checking that public (and private keys via
Public()) are equivalent.
interface {
Equal(crypto.PublicKey) bool
}
Equality for ECDSA keys is complicated, we take a strict interpretation
that works for all secure applications (the ones not using the
unfortunate non-constant time CurveParams implementation) and fails
closed otherwise.
Tests in separate files to make them x_tests and avoid an import loop
with crypto/x509.
Re-landing of CL 223754. Dropped the test that was assuming named curves
are not implemented by CurveParams, because it's not true for all
curves, and anyway is not a property we need to test. There is still a
test to check that different curves make keys not Equal.
Fixes#21704Fixes#38035
Reviewed-on: https://go-review.googlesource.com/c/go/+/223754
Reviewed-by: Katie Hockman <katie@golang.org>
Change-Id: I736759b145bfb4f7f8eecd78c324315d5a05385c
Reviewed-on: https://go-review.googlesource.com/c/go/+/225460
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Variable 'procs' used to calculate the threshold of overuse in
TestPhysicalMemoryUtilization should be updated if GOMAXPROCS
gets changed, otherwise the threshold could be a large number,
making the test meaningless.
Change-Id: I876cbf11457529f56bae77af1e35f4538a721f95
Reviewed-on: https://go-review.googlesource.com/c/go/+/210297
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Make the code more consistent with the rest of the file.
Should have caught this in review of CL 225357.
Change-Id: I12824cb436539c31604684e043ebb7587cc92471
Reviewed-on: https://go-review.googlesource.com/c/go/+/225557
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Write the test output to the temporary directory, not the current
directory.
May fix linux-mips64le-mengzhuo builder.
Change-Id: Ibfeb3d2879c11d498abc31df4efe776fc09a6ad6
Reviewed-on: https://go-review.googlesource.com/c/go/+/225440
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Currently if a goroutine is preempted while owning a timer in the
timerModifying state, it could self-deadlock. When the goroutine is
preempted and calls into the scheduler, it could call checkTimers. If
checkTimers encounters the timerModifying timer and calls runtimer on
it, then runtimer will spin, waiting for that timer to leave the
timerModifying state, which it never will.
So far we got lucky that for the most part that there were no preemption
points while timerModifying is happening, however CL 221077 seems to
have introduced one, leading to sporadic self-deadlocks.
This change disables preemption explicitly while a goroutines holds a
timer in timerModifying. Since only checkTimers (and thus runtimer) is
called from the scheduler, this is sufficient to prevent
preemption-based self-deadlocks.
Fixes#38070.
Updates #37894.
Change-Id: Idbfac310889c92773023733ff7e2ff87e9896f0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/225497
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This partially undoes the optimizations of CL 177597, but makes up
some of the difference by caching the package list and import metadata
and making the initial calls concurrently, including in TestMain.
That reduces the critical path from two sequential 'go list'
invocations to just one (run many times concurrently), and eliminates
the need for assumptions about the consistency of the 'std' dependency
graph across platforms (and hard-coded special cases for packages that
violate those assumptions).
In the process, this simplifies and fixes TestBenchmark (which has
been silently broken since CL 164623).
This increases 'time go tool dist test api' on my workstation from
0m8.4s / 0m13.8s / 0m1.7s to 0m10.5s / 0m23.1s / 0m5.1s,
compared to 0m12.4s / 0m23.2s / 0m4.7s before CL 177597.
(That is, this change retains about half of the wall-time speedup, but
almost none of the user-time speedup.)
Tested manually using 'go test -race -bench=. cmd/api'.
Fixes#37951
Change-Id: Icd537e035e725e1ee7c41d97da5c6651233b927e
Reviewed-on: https://go-review.googlesource.com/c/go/+/224619
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
On arm64, a function's address is 16 bytes aligned, and
the assembler aligns the size of function symbol to 16 bytes,
so to keep the consistent, this patch changes the function
alignment in the linker to 16 bytes.
Change-Id: I4d1e89a56200453b7b586fe3f4656bada7544214
Reviewed-on: https://go-review.googlesource.com/c/go/+/225397
Reviewed-by: eric fang <eric.fang@arm.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Recently, we get requirements of instructions and functions alignment
from the gVisor project. To fit the alignment requirement of interrupt
table, they require an instruction's address to be aligned 128 bytes
and a function's entry address to be aligned 2K bytes. Thus we add
support for PCALIGN directive first. Below is a discussion about this
topic. https://groups.google.com/forum/m/#!topic/golang-dev/RPj90l5x86I
Functions in Go are aligned to 16 bytes on arm64, thus now we only
support 8 and 16 bytes alignment.
This patch adds support for PCALIGN directive. This directive can be
used within Go asm to align instruction by padding NOOP directives.
This patch also adds a test to verify the correnctness of the PCALIGN
directive. The test is contributed by Fannie Zhang <Fannie.Zhang@arm.com>.
Change-Id: I709e6b94847fe9e1824f42f4155355f90c63d523
Reviewed-on: https://go-review.googlesource.com/c/go/+/207117
Reviewed-by: eric fang <eric.fang@arm.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The "runindir" tests used "go run", but relied on relative imports
(which are not supported by "go run" in module mode). Instead, such
tests must use fully-qualified imports, which require either a go.mod
file (in module mode) or that the package be in an appropriate
subdirectory of GOPATH/src (in GOPATH mode).
To set up such a directory, we use yet another copy of the same
overlayDir function currently found in the misc subdirectory of this
repository.
Fixes#33912
Updates #30228
Change-Id: If3d7ea2f7942ba496d98aaaf24a90bcdcf4df9f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/225205
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The load and test instructions compare the given value
against zero and will produce a condition code indicating
one of the following scenarios:
0: Result is zero
1: Result is less than zero
2: Result is greater than zero
3: Result is not a number (NaN)
The instruction can be used to simplify floating point comparisons
against zero, which can enable further optimizations.
This CL also reduces the size of .text section of math.test binary by around
0.7 KB (in hexadecimal, from 1358f0 to 135620).
Change-Id: I33cb714f0c6feebac7a1c46dfcc735e7daceff9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/209159
Reviewed-by: Michael Munday <mike.munday@ibm.com>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The s390x assembly implementation was previously only handling this
case correctly for x = -Pi. Update the special case handling for
any y.
Fixes#35446
Change-Id: I355575e9ec8c7ce8bd9db10d74f42a22f39a2f38
Reviewed-on: https://go-review.googlesource.com/c/go/+/223420
Run-TryBot: Brian Kessler <brian.m.kessler@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Where possible, fold constants into versions of instructions that take
an immediate. This avoids the need to allocate a register and load the
immediate into it.
Change-Id: If911ca41235e218490679aed2ce5f48bf807a2b3
Reviewed-on: https://go-review.googlesource.com/c/go/+/222639
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
riscv64 now has atomic intrinsics, so re-enable the atomic intrinsic tests.
Fixes#36765
Change-Id: I838f27570a94d7fa5774c43f1ca5f4df2ca104cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/223560
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
For #25628
Change-Id: If1dce7ba9310e1418e67b9954c989471b775a28e
Reviewed-on: https://go-review.googlesource.com/c/go/+/225278
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This fixes a potential issue with the previous implementation
of PCALIGN on ppc64. Previously PCALIGN was processed inside of
asmout and indicated the padding size by setting the value in
the optab, changing it back after the alignment instructions
were added. Now PCALIGN is processed outside of asmout, and optab
is not changed.
Change-Id: I8b0093a0e2b7e06176af27e05150d04ae2c55d60
Reviewed-on: https://go-review.googlesource.com/c/go/+/225198
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
As of the Go 1.11 release we require at least Windows 7, so CancelIoEx
is always available. This lets us simplify the code to not require
dedicated threads to handle I/O requests.
Fixes#37956
Change-Id: If1dc4ac4acb61c43e4f2a9f26f225869050262a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/225060
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
newdefer() actually adds the new defer to the current g's defer chain. That
happens even if we are on the system stack, in which case the g will be the g0
stack. For open-coded defers, we call newdefer() (only during panic processing)
while on the system stack, so the new defer is unintentionally added to the
g0._defer defer list. The code later correctly adds the defer to the user g's
defer list.
The g0._defer list is never used. However, that pointer on the g0._defer list can
keep a defer struct alive that is intended to be garbage-collected (smaller defers
use a defer pool, but larger-sized defer records are just GC'ed). freedefer() does
not zero out pointers when it intends that a defer become garbage-collected. So,
we can have the pointers in a defer that is held alive by g0._defer become invalid
(in particular d.link). This is the cause of the bad pointer bug in this issue
The fix is to change newdefer (only used in two places) to not add the new defer
to the gp._defer list. We just do it after the call with the correct gp pointer.
(As mentioned above, this code was already there after the newdefer in
addOneOpenDeferFrame.) That ensures that defers will be correctly
garbage-collected and eliminate the bad pointer.
This fix definitely fixes the original repro. I added a test and tried hard to
reproduce the bug (based on the original repro code), but awasn't actually able to
cause the bug. However, the test is still an interesting mix of heap-allocated,
stack-allocated, and open-coded defers.
Fixes#37688
Change-Id: I1a481b9d9e9b9ba4e8726ef718a1f4512a2d6faf
Reviewed-on: https://go-review.googlesource.com/c/go/+/224581
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
We used to fall back to GetQueuedCompletionStatus if
GetQueuedCompletionStatus was not available, but as of Go 1.11 we
require Windows 7 or later, so GetQueuedCompletionStatusEx is always
available.
Fixes#37957
Change-Id: I7d8d49a92ab7b1f5afdc54a442f696aaf4a5168e
Reviewed-on: https://go-review.googlesource.com/c/go/+/225059
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reduce the length of time that other timer functions can see timerModifying.
In particular avoid system calls.
Fixes#38023
Change-Id: I1b61229c668e6085d9ee6dca9488a90055386c36
Reviewed-on: https://go-review.googlesource.com/c/go/+/224902
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
'go mod download' is equivalent to 'go mod download all'.
Fixes#38031
Change-Id: I7aec7e5a1370a3e248eba6daad9a75ec21f33a83
Reviewed-on: https://go-review.googlesource.com/c/go/+/225201
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reverts commit CL 222782.
Reason for revert: Reverting to see if 386 errors go away
Update #37881
Change-Id: I74f287404c52414db1b6ff1649effa4ed9e5cc0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/225218
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The callers expect negative errno values, so negate them when necessary.
No test because there is no reasonable way to make pipe/pipe2 fail.
This was reported on a system on which pipe2 returned ENOSYS.
Fixes#37997
Change-Id: I3ad6cbbc2521cf495f8df6ec991a3f781122b508
Reviewed-on: https://go-review.googlesource.com/c/go/+/224592
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
In Webpack, require("fs") will always be empty. This behavior throws an error: "fs.writeSync is not function". It happens when you did "fmt.Println".
This PR avoids such problem and use polyfill in wasm_exec.js on Webpack.
Change-Id: I55f2c75ce86b7f84d2d92e8e217b5decfbe3c8a1
GitHub-Last-Rev: aecc847e3f
GitHub-Pull-Request: golang/go#35805
Reviewed-on: https://go-review.googlesource.com/c/go/+/208600
Reviewed-by: Richard Musiol <neelance@gmail.com>
List test packages (when list is run with -e) even when the main package
has an error. This is useful to get complete data for go/packages.
Fixes#37971
Change-Id: If6ba0270a319ea5e003d1ed8b1ad39e479e95509
Reviewed-on: https://go-review.googlesource.com/c/go/+/224944
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This reverts CL 223754.
Reason for revert: new tests are failing on all longtest builders.
Change-Id: I2257d106c132f3a02c0af6b20061d4f9a8093c4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/225077
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
For each experiment that has been enabled in the toolchain, define a build tag
with the same name (but prefixed by "goexperiment.") which can be used for
compiling alternative files for the experiment. This allows changes for the
experiment, like extra struct fields in the runtime, without affecting the base
non-experiment code at all.
I use this capability in my CL for static lock ranking
(https://go-review.googlesource.com/c/go/+/207619), so that static lock ranking
can be fully enabled as a GOEXPERIMENT, but there is no overhead in the runtime
when the experiment is not enabled.
I added a test in cmd/go/testdata/scripts to make sure the build tags are being
defined properly. In order to implement the test, I needed to provide environment
variable GOEXPSTRING to the test scripts (with its value set from
objabi.Expstring(), so that it can determine the experiments baked into the
toolchain.
I filed https://github.com/golang/go/issues/37937 to make a builder with
GOEXPERIMENT set to 'staticlockranking'. This builder will ensure another variant
of GOEXPERIMENT is being tested regularly for this change, as well as checking
static lock ranking in the runtime.
Change-Id: Ieb4b86107238febd105558c1e639d30cfe57ab5c
Reviewed-on: https://go-review.googlesource.com/c/go/+/222925
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Rolling back portions of CL 222782 to see if that helps
issue #37881 any.
Update #37881
Change-Id: I9cc3ff8c469fa5e4b22daec715d04148033f46f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/224837
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This makes all modern public keys in the standard library implement a
common interface (below) that can be used by applications for better
type safety and allows for checking that public (and private keys via
Public()) are equivalent.
interface {
Equal(crypto.PublicKey) bool
}
Equality for ECDSA keys is complicated, we take a strict interpretation
that works for all secure applications (the ones not using the
unfortunate non-constant time CurveParams implementation) and fails
closed otherwise.
Tests in separate files to make them x_tests and avoid an import loop
with crypto/x509.
Fixes#21704
Change-Id: Id5379c96384a11c5afde0614955360e7470bb1c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/223754
Reviewed-by: Katie Hockman <katie@golang.org>
Discovered this after rebasing CL196679 (use poset bounds in prove).
Some tests fail with that CL applied:
codegen/smallintiface.go:11: linux/amd64/: opcode not found: "^LEAQ\\truntime.staticuint64s\\+8\\(SB\\)"
codegen/smallintiface.go:16: linux/amd64/: opcode not found: "^LEAQ\\truntime.staticuint64s\\+2024\\(SB\\)"
codegen/smallintiface.go:21: linux/amd64/: opcode not found: "^LEAQ\\truntime.staticuint64s\\+24\\(SB\\)"
The only difference in prove SSA dumps is that a single Lsh64x64
op with constant shift (<< 3) is marked as bounded. This triggers
a different rule matching sequence in lower, which manages to generate
worse code for the above testcases.
This CL fixes the above test after CL196679 is applied. Right now,
these rules never trigger (this CL passes toolstash -cmp), so I can't
write a test.
Change-Id: I353f1c79c1875cac1da82cd8afa1e05e42684f1c
Reviewed-on: https://go-review.googlesource.com/c/go/+/224877
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The existing Certificate.CreateCRL method generates non-conformant CRLs and
as such cannot be used for implementations that require standards
compliance. This change implements a new top level method, CreateCRL, which
generates compliant CRLs, and offers an extensible API if any
extensions/fields need to be supported in the future.
Here is an example Issuer/CRL generated using this change:
-----BEGIN CERTIFICATE-----
MIIBNjCB3aADAgECAgEWMAoGCCqGSM49BAMCMBIxEDAOBgNVBAMTB3Rlc3Rpbmcw
IhgPMDAwMTAxMDEwMDAwMDBaGA8wMDAxMDEwMTAwMDAwMFowEjEQMA4GA1UEAxMH
dGVzdGluZzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABLHrudbSM36sn1VBrmm/
OfQTyEsI4tIUV1VmneOKHL9ENBGCiec4GhQm2SGnDT/sZy2bB3c3yozh/roS6cZJ
UZqjIDAeMA4GA1UdDwEB/wQEAwIBAjAMBgNVHQ4EBQQDAQIDMAoGCCqGSM49BAMC
A0gAMEUCIQCoAYN6CGZPgd5Sw5a1rd5VexciT5MCxTfXj+ZfJNfoiAIgQVCTB8AE
Nm2xset7+HOgtQYlKNw/rGd8cFcv5Y9aUzo=
-----END CERTIFICATE-----
-----BEGIN X509 CRL-----
MIHWMH0CAQEwCgYIKoZIzj0EAwIwEjEQMA4GA1UEAxMHdGVzdGluZxgPMDAwMTAx
MDIwMDAwMDBaGA8wMDAxMDEwMzAwMDAwMFowFjAUAgECGA8wMDAxMDEwMTAxMDAw
MFqgHjAcMA4GA1UdIwQHMAWAAwECAzAKBgNVHRQEAwIBBTAKBggqhkjOPQQDAgNJ
ADBGAiEAjqfj/IG4ys5WkjrbTNpDbr+saHGO/NujLJotlLL9KzgCIQDm8VZPzj0f
NYEQgAW4nsiUzlvEUCoHMw0141VCZXv67A==
-----END X509 CRL-----
Fixes#35428
Change-Id: Id96b6f47698d0bed39d586b46bd12374ee6ff88f
GitHub-Last-Rev: c83a601716
GitHub-Pull-Request: golang/go#36945
Reviewed-on: https://go-review.googlesource.com/c/go/+/217298
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change modifies the implementation of (*mspan).countAlloc by
using OnesCount64 (which on many systems is intrinsified). It does so by
using an unsafe pointer cast, but in this case we don't care about
endianness because we're just counting bits set.
This change means we no longer need the popcnt table which was redundant
in the runtime anyway. We can also simplify the logic here significantly
by observing that mark bits allocations are always 8-byte aligned, so we
don't need to handle any edge-cases due to the fact that OnesCount64
operates on 64 bits at a time: all irrelevant bits will be zero.
Overall, this implementation is significantly faster than the old one on
amd64, and should be similarly faster (or better!) on other systems
which support the intrinsic. On systems which do not, it should be
roughly the same performance because OnesCount64 is implemented using a
table in the general case.
Results on linux/amd64:
name old time/op new time/op delta
MSpanCountAlloc/bits=64-4 16.8ns ± 0% 12.7ns ± 0% -24.40% (p=0.000 n=5+4)
MSpanCountAlloc/bits=128-4 23.5ns ± 0% 12.8ns ± 0% -45.70% (p=0.000 n=4+5)
MSpanCountAlloc/bits=256-4 43.5ns ± 0% 12.8ns ± 0% -70.67% (p=0.000 n=4+5)
MSpanCountAlloc/bits=512-4 59.5ns ± 0% 15.4ns ± 0% -74.12% (p=0.008 n=5+5)
MSpanCountAlloc/bits=1024-4 116ns ± 1% 23ns ± 0% -79.84% (p=0.000 n=5+4)
Change-Id: Id4c994be22224653af5333683a69b0937130ed04
Reviewed-on: https://go-review.googlesource.com/c/go/+/216558
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
This change adds a small microbenchmark for (*mspan).countAlloc, which
we're about to replace. Admittedly this isn't a critical piece of code,
but the benchmark was useful in understanding the performance change.
Change-Id: Iea93c00f571ee95534a42f2ef2ab026b382242b3
Reviewed-on: https://go-review.googlesource.com/c/go/+/224438
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently if we try to alias an intrinsic which hasn't been defined for
any architecture (such as by accidentally creating the alias before the
intrinsic is created with addF), then we'll just silently not apply any
intrinsics to those aliases.
Catch this particular case by panicking in alias if we try to apply the
alias and it did nothing.
Change-Id: I98e75fc3f7206b08fc9267cedb8db3e109ec4f5d
Reviewed-on: https://go-review.googlesource.com/c/go/+/224637
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Currently runtime/internal/sys bit-manipulation functions are aliased to
math/bits functions, which are intrinsified. Unfortunately these aliases
are declared before the intrinsified versions are generated, resulting
in the generic version of the code being copied over.
This change moves the aliases for bit operations in runtime/internal/sys
after the addF calls to generate those intrinsics in SSA, so that the
intrinsified SSA representation of those functions actually get copied
over.
This should improve the overall performance of the runtime (especially
the page allocator) since these bit operations will actually be
intrinsified now.
Change-Id: I4377da13f9a7bb6aee608e50df0297148bf8f806
Reviewed-on: https://go-review.googlesource.com/c/go/+/224437
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
In golang.org/cl/171464, we cleaned up generation of .stkobj linker
symbols, but we couldn't figure out why a similar cleanup to
.args_stackmap linker symbols caused problems.
The issue is that we only need/want to generate .args_stackmap for
functions that are implemented in assembly in the same package. When
"pulling" a function from another package via //go:linkname, we can
safely skip emitting .args_stackmap, because compiling that package
will have generated it, if necessary.
Fixes#31615.
Change-Id: If8680aa7dd5b4e8f268b6b032d746f1b8536c867
Reviewed-on: https://go-review.googlesource.com/c/go/+/223238
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
