CL 89936 introduced restriction that Rows.Close() shouldn't modify dest
in previous Next().
CL 497675 for #60304 removed that restriction. But it didn't remove doc
about the restriction.
Change-Id: Idc0f16462fba4d2013ec069d251b8ebffbc4bdd1
GitHub-Last-Rev: f37493acd7aa3ad7863ad9c29b41290ad557d9cd
GitHub-Pull-Request: golang/go#73632
Reviewed-on: https://go-review.googlesource.com/c/go/+/670895
Auto-Submit: Sean Liao <sean@liao.dev>
Reviewed-by: Sean Liao <sean@liao.dev>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Variable uint64 arguments lowered through immJumpTable were converted
directly to uintptr. Since the generated table has 256 entries, values
above 255 could dispatch outside the table and corrupt control flow. With
-spectre=ret, the binary-search fallback instead selected case 255,
differing from constant lowering.
Convert uint64 indices to uint8 before either dispatch path, matching
imm8 constant encoding. Extend the ShiftAllRightConcatMod32 tests through
the first out-of-range value.
Tests:
- GOEXPERIMENT=simd ./bin/go test simd/...
- GOEXPERIMENT=simd ./bin/go test -short cmd/compile/...
- GOEXPERIMENT=simd ./bin/go vet cmd/compile/internal/ssagen simd/...
- linux/amd64 PoC, ordinary and -spectre=ret dispatch
Fixes#80522.
Change-Id: I4a663f91c561e8212509b155e10acfc7245a59a1
GitHub-Last-Rev: 18cbbde1817affdce75d96be0a10ec908e0a785a
GitHub-Pull-Request: golang/go#80526
Reviewed-on: https://go-review.googlesource.com/c/go/+/803273
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
When the coordinator's context deadline expires, there is a brief
window where ctx.Err() is set but fuzzCtx (a child context) has not
yet been canceled. If the coordinator's event loop observes <-doneC
during this window, the stop function compares the error against
fuzzCtx.Err() which is still nil, so the deadline error is not
suppressed and leaks out as a spurious test failure.
Fix this by also comparing against ctx.Err() in the suppression check.
ctx.Err() is guaranteed to be set before ctx.Done() is closed because
cancelCtx.cancel stores the error before closing the done channel.
Fixes#75804
Change-Id: I5262227c80b98387471a22e3cb8fe8491057e51e
GitHub-Last-Rev: a99ddf64fe05a99952516050c379673ed278a6f9
GitHub-Pull-Request: golang/go#79199
Reviewed-on: https://go-review.googlesource.com/c/go/+/774140
Auto-Submit: Sean Liao <sean@liao.dev>
Reviewed-by: Sean Liao <sean@liao.dev>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This is intended to simplify tuning and experiments.
Change-Id: I8ec2245b868fe4d508df1bc880c69568c90d8f93
Reviewed-on: https://go-review.googlesource.com/c/go/+/803700
Reviewed-by: Jakub Ciolek <jakub@ciolek.dev>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Unlike what CL 778622 assumed, the default for cmd/go is
whatever was set at the time of make.bash, not amd64.v1.
To test this properly, use a value that's higher than
what will reasonably be set.
Updates #73294Fixes#80525
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-goamd64v3
Change-Id: Ia8f9b9871ee9886201fa0a129c05a3636a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/804060
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jorropo <jorropo.pgm@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
The issue was fixed unintentionally by CL 767161.
Fixes#80520
Change-Id: I212af69f0d7ccdc8e48f9a17e207de57be85e9e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/804280
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
When parsing an ECHConfigList, length fields were previously evaluated
as a uint16. This would cause an infinite loop to occur when parsing a
65532 bytes long ECHConfig with a declared outer length header of 0.
Triggering this condition requires a payload of at least 65538 bytes. As
ECHConfigList payloads are typically delivered via protocols that limit
them to 65535 bytes (DNS HTTPS records and TLS extensions), regular
clients are safe from this issue.
Thank you to Nguyễn Hoàng Hải (facebookmark2022@gmail.com) for reporting
this issue.
Fixes#80513
Change-Id: I8c5011d3b375c24794dd38b915bb4dc06a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/804040
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LoweredZero on arm32 and s390x also take an end-of-zero-range
address. That input is also an address sink.
Also allow propagation of addresses from input to output of a value,
so that we handle at least simple address arithmetic. This is required
to fix riscv64 because it has no indexed load/store, and thus does
a bunch more explicit address arithmetic than other archs.
Fixes#80484
Change-Id: I126eec1e2cc6c2ab5a39611ccc4d3371dca28292
Reviewed-on: https://go-review.googlesource.com/c/go/+/803660
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: David du Colombier <0intro@gmail.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
POSIX says you follow the symlink, but some platforms don't.
Add OpenBSD to the list of ones that don't.
Fixes#80386
Change-Id: Ifcfed66c934ba1ec74836f883dfba3f86a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/800700
Reviewed-by: Nicholas Husin <nsh@golang.org>
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
We're going to use these for the run cgo action once it's cached
separately.
Change-Id: I8c1ae6db015f89739a7f21525abd79e06a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/799900
Reviewed-by: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
The disabledOnPlatform constant was hand-copied from
x/telemetry/internal/telemetry until a public API became available.
The vendored golang.org/x/telemetry/counter/countertest package now
exports SupportedPlatform, so use it and delete the copy.
This also picks up the mips/mipsle entries, which were added to
DisabledOnPlatform in x/telemetry after the constant was copied here.
For #66205
Change-Id: I9d4b6bb0939f0a32f78583d7a450167489a3d3be
Reviewed-on: https://go-review.googlesource.com/c/go/+/802520
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Auto-Submit: Hyang-Ah Hana Kim <hyangah@gmail.com>
At the header of a loop that makes an unavoidable call,
values that are unused before that call needn't be in registers.
The call will clobber them anyway,
and the back-edge will do pointless reloads.
Free them. They'll be loaded lazily again when needed.
This shrinks total generated code size for std a bit,
from -0.07% to -0.2% depending on GOARCH.
More interesting, I instrumented the toolchain to
count spills at runtime on amd64.
Using the compiler as a test case, this reduces
spills by 3.1% while building std+cmd.
Using a subset of std tests that have fairly
stable execution paths as a test case, spills drop 2%.
There are a few microbenchmarks that jump noticeably,
such as a 16% sort.StableInt1K speed-up on arm64,
and a few minor regressions,
but most I tried are either neutral or small improvements,
which is about what you'd expect from
an average 2% reduction in spills.
Change-Id: I8fa88a665fb51fdd2c1fd13695c37ed7d50c89d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/802740
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Add riscv64 asmcheck coverage for the conditional subtraction cases in condmove.go.
Verify that rva20u64 and rva22u64 keep using the existing SNEZ/NEG/AND/OR sequence. Verify that rva23u64 uses CZERONEZ/CZEROEQZ/OR without the older fallback sequence.
Co-authored-by: YuanSheng <yuansheng@isrc.iscas.ac.cn>
Change-Id: I7df5c57e36e57c6309ce0fb0efd2d53bd9e06b76
Reviewed-on: https://go-review.googlesource.com/c/go/+/801360
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Jorropo <jorropo.pgm@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Meng Zhuo <mengzhuo1203@gmail.com>
cmd/compile uses internal/buildcfg directly for reading the
arch specific envs, which only reads from the environment
and not cmd/go's GOENV config file.
Setting the build context build tag allows both build and list
operations to select the right files.
Fixes#73294
Change-Id: Ib1dd4c1d18c49d12de1a6c99d1a15be96a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/778622
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
No test sets omitVariant to true anymore, so !opts.omitVariant is
always true and the field is dead. Remove it, as its TODO suggests:
having timing information in ResultDB that corresponds directly with
dist test names is too valuable to give up.
Change-Id: I4b8e2d054770f5224dbb8e4c5ea136f058f2d48c
Reviewed-on: https://go-review.googlesource.com/c/go/+/802540
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Change-Id: Icfe3063a5390105cf0796046348bcaa9e88bbfde
GitHub-Last-Rev: 4df263e334c7011a0a08b7eb5d1722290302b15f
GitHub-Pull-Request: golang/go#65803
Reviewed-on: https://go-review.googlesource.com/c/go/+/565276
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Sean Liao <sean@liao.dev>
Auto-Submit: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Carlos Amedee <carlos@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Local wildcard matching checks whether each matched directory contains a
package. In module mode, answer that from the module index when available
instead of reading each directory again.
For typescript-go, the Windows local-wildcard go list query used by
gopls drops from about 4.3s to about 2.1s.
Change-Id: I7b6d8ee33031da61beec22926f1f8744d048a58e
Reviewed-on: https://go-review.googlesource.com/c/go/+/795741
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
When -coverpkg excludes the main package, SelectCoverPackages returns
before the existing regonly path can add the main package's coverage
registration hook. Allow unmatched main packages through that gate so
covered dependencies can emit metadata and counters.
Add a script regression for building with coverage limited to an
external dependency, and align the fixture's required module with its
imported package for deterministic cold-cache resolution.
Fixes#80307
Change-Id: I8867cd40069a897a52a89085acbfc743f531aec5
Reviewed-on: https://go-review.googlesource.com/c/go/+/801381
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Sean Liao <sean@liao.dev>
Reviewed-by: Carlos Amedee <carlos@golang.org>
itabInit filled m.Fun by storing code pointers through an
unsafe.Pointer slice, which makes the compiler emit a write barrier.
On wasm a code PC is a function index shifted left 16 bits, a small
value that can fall inside a live heap span, so the GC mistakes it
for a bad heap pointer and crashes.
Store through a uintptr slice instead so no write barrier is emitted,
and mark itabInit //go:nowritebarrier so the same mistake fails to
compile.
Fixes#80472
Change-Id: If9532c01b66b8c4ceb47c932017569488b6143d5
GitHub-Last-Rev: 6e1374c083518bb9f994092acca4f5ea28b38dfa
GitHub-Pull-Request: golang/go#80487
Reviewed-on: https://go-review.googlesource.com/c/go/+/803460
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Pool.Get currently clears the current P's private slot before checking whether the saved interface is nil. For empty or starved pools, this performs two unnecessary zero stores and a write-barrier check.
On amd64, private and the shared queue header occupy the first 32 bytes of poolLocal. When other Ps inspect shared queues while stealing, the redundant private write can cause cache-line contention.
Check the saved value first and clear private only when it is non-nil. This preserves typed-nil behavior because an interface containing a typed nil compares non-nil. Add BenchmarkPoolGetEmpty to cover the affected miss path.
Median results on linux/amd64:
name old time/op new time/op delta
PoolGetEmpty/P=1 7.802ns 7.445ns -4.58%
PoolGetEmpty/P=10 23.06ns 2.587ns -88.78%
PoolStarvation/P=10 3.696µs 2.987µs -19.18%
PoolOverflow/P=10 190.6ns 189.2ns -0.73%
Fixes#80486
Change-Id: I6c1794a27c1d848ed7e983f22ac4bd3f45857084
Reviewed-on: https://go-review.googlesource.com/c/go/+/803161
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Daniel Morsing <daniel.morsing@gmail.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Daniel Morsing <daniel.morsing@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
This is a second try for CL 548317.
When the go compiler compiles a function it saves the frame pointer
on the stack and pushes the stack pointer. Thus it is safe to restore
from the stack.
For assembly function still use the mathematical based
restore as users might use the frame pointer as scratch space.
The results are very good, file size down 0.8%
codesize down 2%.
Files:
file before after Δ %
addr2line 3950752 3919200 -31552 -0.799%
asm 7228164 7176284 -51880 -0.718%
buildid 3813377 3782545 -30832 -0.809%
cgo 6184696 6135728 -48968 -0.792%
compile 37066595 36734315 -332280 -0.896%
covdata 4529478 4497638 -31840 -0.703%
cover 7906781 7840805 -65976 -0.834%
dist 5329896 5285504 -44392 -0.833%
distpack 4028506 3996578 -31928 -0.793%
fix 12706126 12614110 -92016 -0.724%
link 10097646 10020534 -77112 -0.764%
nm 3924002 3896858 -27144 -0.692%
objdump 6570663 6523399 -47264 -0.719%
pack 3255175 3229463 -25712 -0.790%
pprof 20273692 20094988 -178704 -0.881%
preprofile 3373625 3351201 -22424 -0.665%
test2json 4563667 4524259 -39408 -0.864%
trace 18675260 18526132 -149128 -0.799%
vet 12297424 12200576 -96848 -0.788%
total 175775525 174350117 -1425408 -0.811%
Code: total 45787786 44851997 -935789 -2.044%
Change-Id: Ie20cb5a90b25e5dfe3b2ce2d6e73a6b049bc53a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/801860
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Jorropo <jorropo.pgm@gmail.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Add a MULXQ SSA op (BMI2 unsigned 64x64->128 multiply) and switch
the AMD64 lowering of Mul64uhilo to emit MULXQ when GOAMD64>=v3.
Unlike MULQ, MULXQ takes its first operand implicitly in RDX, places
both halves of the product in any general-purpose registers, and
does not affect the flags. The latter makes it interleavable with
ADCX/ADOX carry chains, which is the prerequisite for the
dual-carry-chain Montgomery multiplication.
The downside is that MULXQ ends up encoding as 5-6 bytes instead
of 3 bytes. However for bits.Mul64 it tends to balance out due
to needing less register shuffling.
On its own the change is modest but measurable on the fiat-based
P-384/P-521 paths in crypto/ecdsa, where bits.Mul64 dominates:
goos: linux
goarch: amd64
pkg: crypto/ecdsa
cpu: AMD Ryzen Threadripper 2950X 16-Core Processor
│ old │ new │
│ sec/op │ sec/op vs base │
Sign/P256-32 53.39µ ± 4% 53.23µ ± 5% ~ (p=1.000 n=6)
Sign/P384-32 284.3µ ± 3% 284.2µ ± 3% ~ (p=0.699 n=6)
Sign/P521-32 650.1µ ± 2% 663.1µ ± 2% +1.99% (p=0.015 n=6)
Verify/P256-32 76.27µ ± 2% 75.07µ ± 4% -1.57% (p=0.041 n=6)
Verify/P384-32 840.5µ ± 1% 793.1µ ± 2% -5.64% (p=0.002 n=6)
Verify/P521-32 2.147m ± 1% 2.091m ± 1% -2.65% (p=0.002 n=6)
GenerateKey/P256-32 16.79µ ± 3% 16.63µ ± 5% ~ (p=0.589 n=6)
GenerateKey/P384-32 183.1µ ± 3% 171.8µ ± 2% -6.20% (p=0.002 n=6)
GenerateKey/P521-32 445.7µ ± 2% 432.4µ ± 0% -2.98% (p=0.002 n=6)
geomean 230.8µ 226.1µ -2.07%
Change-Id: I40a9753ccc511bf6f55d3ca428a439807906c292
Reviewed-on: https://go-review.googlesource.com/c/go/+/781841
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Jorropo <jorropo.pgm@gmail.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Jorropo <jorropo.pgm@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
This fixes a bug in mach_vm_region_trampoline.
object_name is the 7th integer argument, so per the SysV AMD64 C ABI
it is passed on the stack rather than in a register,
but the trampoline only loaded it into R10 and never stored it,
handing libc_mach_vm_region an uninitialized argument.
I have no idea if this has negative impacts, but it would cause
corrupted BPs to be loaded which needs to be fixed for the LEAVE
commit comming later in that series.
Fixes#80439
Change-Id: I006f947c47ceda3c7b04e90ebccbee500493ec91
Reviewed-on: https://go-review.googlesource.com/c/go/+/801920
Auto-Submit: Jorropo <jorropo.pgm@gmail.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Different module loaders have different package graphs so we can't share
the package cache. This change moves the package cache from being a global
to being a field on the loader. This solves an old todo, but also fixes
a race we were running into when making changes to cmd/go where creating
a new package in swigIntSize would share dependency packages and setPackageFlags
would modify package structs that were being read from.
Change-Id: I7b744dbbd03a14edf4b54e5ea10b4c9d6a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/800520
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Alexander <jitsu@google.com>
Reviewed-by: Michael Matloob <matloob@google.com>
Currently, a method Value (aquired from e.g. Value.Method(i)) is
represented with its receiver and the index of the method, with
a special flag, instead of a regular func Value (as a method value
is a function). When the func value is needed (e.g. with
Value.Interface), it is lazily converted to the func Value
representation. The lazy conversion has a bad escape behavior: it
flows the Value to the a heap-allocated methodValue structure,
which causes the Value to escape. This in turn causes operations
like Value.Interface or Value.Convert escape its receiver to heap,
as they have a code path to handle the lazy conversion of method
Value, even if this code path is not taken.
To avoid the unnecessary escape, this CL changes the method Value
representation to eagerly creating the func Value. This eliminates
the code path with bad escape behavior in operations like
Value.Interface.
The lazy representation does have an advantage: Call on the method
Value is simple, as the method's code expects the receiver as the
first parameter, instead of a closure with the receiver pre-bound.
Keeping the receiver and the method index in the Value allows Call
to find the code pointer and pass the receiver directly. With the
func Value representation, Call has to unmarshal and re-marshal
the arguments in order to shift them by one. To avoid this problem,
we save the receiver and the method index in the closure structure
(i.e. methodValue structure), and set a special flag, so Call on
such a Value can still take the fast path, instead of going
through the closure. Benchmark result shows that Call on such a
Value is as fast as before, but creating the method Value is
slower as it involes an allocation. Creating a method Value and
then doing operations like Interface/Convert/Set is mostly
unchanged, as it just moves a lazy operation earlier.
Change-Id: I70beab0b816c8b0587a1b414e804ec3061c39e43
Reviewed-on: https://go-review.googlesource.com/c/go/+/799760
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
This CL vendors in CL 801700, making ProxyFromEnvironment follow the
recently updated behavior of x/net/http/httpproxy.FromEnvironment.
Fixes#79656
Change-Id: Id01c8c4011eb6cc8f51a81231622b7d16a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/803022
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The posetFlagUnsigned flag and the SetUnsigned method have had no
readers since CL 599795 removed the last ones; SetUnsigned was a no-op
and po.flags was only ever written. Remove the flag constant, the
method, the now-unused flags field, and the SetUnsigned calls (all of
which passed a value that was never observed), along with the dead
unsigned parameter of testPosetOps.
Change-Id: I3a113d85fbc4f99949a5c43d6b4cc3bd3e77b52c
Reviewed-on: https://go-review.googlesource.com/c/go/+/802821
TryBot-Bypass: David Chase <drchase@google.com>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
This CL adds ResolveFile, which performs deprecated ast.Ident
resolution on a File syntax tree produced by the parser
in SkipObjectResolution mode.
As with all ast.Object-related declarations, it is deprecated;
it is provided to ease migration away from ast.Object.
Fixes#79802
+ test, api, relnote
Change-Id: Ia9ba7baf0e546fc8abb7410eb18027777f1c85ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/794420
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
We currently look for all uses of the address of a local, to find the
maximum extent of the lifetime of a local. But the analysis doesn't
handle the case where a use of the address is itself just a copy or
pointer arithmetic, and the result of that use is then subsequently
used outside the lifetime we currently compute.
Maybe we could track down uses of uses, etc. But we do the safer thing
here which is just to consider only uses that we know aren't propagating
the address anywhere. Any other uses are considered bad and abort
merging for the affected local. This is conservative but easy to reason
about. In practice, these address copies don't come up all that often,
as they are usually folded into load/store ops. The only typical use
of an explicit address is initial zeroing (which this CL handles).
Fixes#80127
Change-Id: Ia722a8314c1ace1b883ecedec1a3e06a2957a6b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/796001
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Add general package documentation for cmd/compile/internal/noder.
The existing documentation in cmd/compile/internal/noder/doc.go is not
actually supposed to be doc comments, so it has been moved to the
bottom of the file.
For #80397
Change-Id: Iae2b3e39c95f8237917588e1fe6962f3b6a8f2e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/800440
Reviewed-by: Mark Freeman <markfreeman@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Auto-Submit: Mark Freeman <markfreeman@google.com>
We're going to be adding subpackages to ssa so look in subdirectories (other
than _gen and testata) for generated files.
For #80409
Change-Id: Id921dd8c7065418f087ae600b7ab82e16a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/801040
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
If the edge being removed has a phi associated with it, removing that
edge may change the behavior of the program.
The other phases currently don't generate any code that hits this.
However I managed to create the situation when working on the prove
pass.
Change-Id: I57d8f918d7af45cc2506a36d71f3e17a6a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/801160
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Fix a bug in the runtime where in one edge case we were
restoring the frame pointer to a zero value. mstart1 recorded the
caller's PC and SP into gp.sched but not the frame pointer.
I don't think this cause any issues except maybe inaccurate data
in pprof however it is required to make the LEAVE commit work
correctly later in that series.
Change-Id: I00a761095a42fdd9edd75d2c2fea84c089ad0ae6
Reviewed-on: https://go-review.googlesource.com/c/go/+/801921
Auto-Submit: Jorropo <jorropo.pgm@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
The ciphertext tail loads and stores were assuming that the ciphertext
would always be followed by at least 16 bytes of tag, which could be
read and masked or written with zeroes innocuously.
Annoyingly, we support short GCM tags, so that assumption does not hold.
Repurposed the plaintext tail load/store assembly, which was making no
such assumption, to be used for the ciphertext tail as well.
Added tests in cryptotest.TestAEAD that surround the buffers with
inaccessible pages, so that any out-of-bounds access will fault. In the
process, normalized how these tests are done across the crypto tree.
Fixes#80288
Change-Id: Id334c3803a593e18c9607d9b0d2b78616a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/801600
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
This allows known bits's debug output to be traversal order independant
(when not considering loops which known bits currently do not know
how to deal with).
Fixes#80451
Change-Id: I33e91bbed27122400f65422a86ab29a5a3d6d277
Reviewed-on: https://go-review.googlesource.com/c/go/+/802141
Auto-Submit: Jorropo <jorropo.pgm@gmail.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
I have found some of theses in the wild and we are relying on
known bits to fold theses right now.
Doing in opt allows prove to do a better job.
Change-Id: I30dc6b8acf66356726d50782a95a07a18f1ed1ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/801980
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Jorropo <jorropo.pgm@gmail.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
The -toolexec build flag wraps each toolchain program (compile, asm,
link, ...) in a user-supplied program, e.g. to profile them or swap in
alternates.
To know when to rebuild a cgo package, the go command records the C,
C++, and Fortran compiler versions by probing each compiler:
cc -### -x c -c -
That probe was run under -toolexec, but the real cgo compiles never
are, so the wrapper saw the probe and none of the actual compiles.
Probe under -toolexec only for gccgo, a Go toolchain compiler treated
like cmd/compile. Leave the C, C++, and Fortran probes unwrapped.
Fixes#64580
Change-Id: I2161d6d4a8d49e87e5854dfe17b9eb83db059376
GitHub-Last-Rev: f48be7eda7db9a8e5cd87d46ead5bf65df5fee9a
GitHub-Pull-Request: golang/go#80153
Reviewed-on: https://go-review.googlesource.com/c/go/+/794361
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
This CL should have been part of CL 796480, but was mistakenly omitted.
Updates #77485.
Change-Id: Ib18dbbcd01ad2080d14c64494e2c71421a1bd2f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/801960
Reviewed-by: Richard Miller <millerresearch@gmail.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: David Chase <drchase@google.com>
I filed #49331 about five years ago, suggesting that the compiler
should re-associate some computations to support ILP.
There was general consensus that it was a decent idea,
and I believe that Ryan Berger may have attempted it,
but so far, nothing has landed.
For md5 in particular, which was the motivating example,
we can get the desired parallelism by adjusting the source code.
So do that. We can always revert if/when the compiler pass lands.
Each MD5 step is of the form
arg0 = arg1 + RotateLeft32(F(arg1,arg2,arg3) + arg0 + x + k, s)
where arg1 is the output of the previous step.
The generated code has the form a+(b+(c+d)) rather than (a+b)+(c+d),
which has a long serial dependency chain.
This is a significant source of the performance difference between
the generic Go code and the handwritten assembly.
Adjust the generator to impose the association we desire.
Group the arg1-independent terms as F + (arg0+x+k).
This lets the compiler schedule the inner sum in parallel with F,
trimming the chain to a single add after F is calculated.
Add two further expression tweaks.
In round 2, use the direct selection form (x&z)|(y&^z)
instead of the XOR-trick form ((x^y)&z)^y.
The direct form puts arg1 on a 2-op path to F instead of 3,
which reduces serialization on the critical path and shortens variable lifetimes.
On arm64, both forms are 3 logical ops.
On default amd64, this adds an instruction,
but it's off the critical path.
And BMI1 is available at GOAMD64 >= v3.
In round 3, associate arg1^arg2^arg3 as arg1^(arg2^arg3), again for ILP.
Before this commit, asm was about ~30% faster than purego on my arm64
machine, and ~37% on the amd64 machine I have access to (no GOAMD64).
This commit narrows the gap to ~13% and ~20%, respectively.
I have follow-up work planned that shrinks the gap further
and should enable deletion of the md5 assembly.
Updates #49248
Updates #49331
Change-Id: Ia317e20cd73c88c4fb9303b31f5b6b441bfa6b63
Reviewed-on: https://go-review.googlesource.com/c/go/+/801240
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
We don't support binary-only packages, and haven't for a while. We would
only check for binary only packages when actually executing the action
graph. But it seems more natural to report the error when loading
packages. The only difference is the reporting of staleness but it
doesn't really make sense to report staleness if we don't even support
building those packages.
While we're here remove the unused needStale need bit.
This also slightly simplifies the logic in checkCacheForBuild.
For #28152
Change-Id: I2bad74152d2c4323026c7513537265a36a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/799002
Auto-Submit: Michael Matloob <matloob@golang.org>
TryBot-Bypass: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Hongxiang Jiang <hxjiang@golang.org>
