[pull] master from golang:master (fix conflicts)#147
Merged
Conversation
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>
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 golang#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>
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 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>
…ions Change-Id: Ib8f93845bd0857d12cfacfe83ce8f57ea23a4ba1 Reviewed-on: https://go-review.googlesource.com/c/go/+/798100 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: Keith Randall <khr@google.com> 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>
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 golang#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>
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 golang#80472 Change-Id: If9532c01b66b8c4ceb47c932017569488b6143d5 GitHub-Last-Rev: 6e1374c GitHub-Pull-Request: golang#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>
For golang#63659 Change-Id: I0bb318f6bae169b04d39af2d295be0296a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/802680 Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org> Reviewed-by: Michael Matloob <matloob@google.com> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.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 golang#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>
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>
Change-Id: Icfe3063a5390105cf0796046348bcaa9e88bbfde GitHub-Last-Rev: 4df263e GitHub-Pull-Request: golang#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>
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>
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 golang#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>
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>
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>
|
|
There was a problem hiding this comment.
Pull request overview
This PR resolves conflicts from the golang:master sync (PR #146) and incorporates upstream changes across the toolchain/runtime/tests, including improved CLI flag help/usage output, compiler backend updates (notably for amd64 GOAMD64>=v3), and internal caching/behavior tweaks.
Changes:
- Update go command flag definitions to include meaningful usage strings and print flag defaults in usage output.
- Add/adjust compiler and codegen expectations for newer amd64 (MULXQ for GOAMD64>=3) and improve regalloc behavior around unavoidable calls.
- Refactor internal package caching to be loader-scoped (instead of a global cache), plus various runtime and test adjustments from upstream.
Reviewed changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/codegen/mathbits.go | Update amd64 multiply instruction expectations for GOAMD64>=v3. |
| test/codegen/condmove.go | Add riscv64 codegen expectations for constant-time select patterns. |
| test/codegen/arithmetic.go | Relax amd64 codegen expectations to allow MULQ/MULXQ. |
| src/sync/pool.go | Avoid redundant write to local private slot when empty. |
| src/sync/pool_test.go | Add benchmark for Pool.Get when the pool is empty. |
| src/runtime/sys_darwin_amd64.s | Fix mach_vm_region trampoline to pass 7th arg on stack with alignment. |
| src/runtime/iface.go | Ensure itab method table stores code pointers without write barriers. |
| src/debug/pe/file.go | Fix comment typo (io.ReadSeeker). |
| src/cmd/internal/obj/x86/obj6.go | Restrict LEAVE usage in asm preprocessing based on ABI safety. |
| src/cmd/go/testdata/script/mod_graph_version.txt | Adjust stderr expectations due to new usage output behavior. |
| src/cmd/go/testdata/script/list_parse_err.txt | Add coverage for wildcard patterns matching packages with invalid files. |
| src/cmd/go/testdata/script/list_goarch.txt | Add script test validating GOAMD64 build-tag selection behavior. |
| src/cmd/go/testdata/script/cover_build_pkg_select.txt | Add coverage test for -coverpkg limited to dependency; update module requirement. |
| src/cmd/go/testdata/script/build_buildvcs_auto.txt | Relax stderr expectations due to usage/flag output changes. |
| src/cmd/go/internal/workcmd/vendor.go | Add flag help strings and group vendor flag vars. |
| src/cmd/go/internal/workcmd/use.go | Add help string for -r. |
| src/cmd/go/internal/workcmd/edit.go | Add detailed help strings for go work edit flags. |
| src/cmd/go/internal/work/build.go | Add help strings for build/cover flags; improve flag.Value String methods. |
| src/cmd/go/internal/vet/vetflag.go | Improve vet tool flag plumbing and print flag defaults on usage errors. |
| src/cmd/go/internal/version/version.go | Add usage strings for version flags. |
| src/cmd/go/internal/test/testflag.go | Add usage strings for test flags and print defaults on usage errors. |
| src/cmd/go/internal/search/search.go | Use module index to detect packages during directory matching (with build tags). |
| src/cmd/go/internal/run/run.go | Add -exec help text; minor comment cleanup. |
| src/cmd/go/internal/modload/init.go | Move package cache into modload.Loader state with accessor. |
| src/cmd/go/internal/modget/get.go | Add help text; clarify deprecated/no-op flags. |
| src/cmd/go/internal/modcmd/why.go | Add help text for go mod why flags. |
| src/cmd/go/internal/modcmd/vendor.go | Add help text; minor cleanup; small local var style tweak. |
| src/cmd/go/internal/modcmd/tidy.go | Add help text for tidy flags (includes one typo to fix). |
| src/cmd/go/internal/modcmd/graph.go | Add help text for graph flags. |
| src/cmd/go/internal/modcmd/edit.go | Add help text for edit flags (includes one typo to fix). |
| src/cmd/go/internal/modcmd/download.go | Add help text for download flags and -x. |
| src/cmd/go/internal/load/pkg.go | Use loader-scoped package cache; adjust cover package selection logic placement. |
| src/cmd/go/internal/list/list.go | Import formatting and add help strings for list flags. |
| src/cmd/go/internal/generate/generate.go | Add help strings; simplify slice copy expression. |
| src/cmd/go/internal/envcmd/env.go | Add help strings for env flags. |
| src/cmd/go/internal/doc/pkgsite.go | Remove global package cache clear (cache is now loader-scoped). |
| src/cmd/go/internal/doc/doc.go | Print flag defaults in go doc usage output. |
| src/cmd/go/internal/clean/clean.go | Add help strings for clean flags. |
| src/cmd/go/internal/cfg/cfg.go | Add arch environment tag into build context tool tags. |
| src/cmd/go/internal/bug/bug.go | Add help text for go bug -v. |
| src/cmd/go/internal/base/flag.go | Add help strings for common flags (-n, -x, -C, -mod, mod common flags). |
| src/cmd/go/internal/base/base.go | Include flag defaults in command usage output; group exit status vars. |
| src/cmd/go/chdir_test.go | Update test to match new -C usage string. |
| src/cmd/dist/test.go | Simplify variant handling in dist tests and JSON package rewriting. |
| src/cmd/compile/internal/ssa/rewriteAMD64.go | Route Mul64uhilo rewrite through generated helper (GOAMD64-dependent). |
| src/cmd/compile/internal/ssa/regalloc.go | Free “doomed” register values across unavoidable calls in loop headers. |
| src/cmd/compile/internal/ssa/opGen.go | Add OpAMD64MULXQ opcode metadata. |
| src/cmd/compile/internal/ssa/_gen/AMD64Ops.go | Add MULXQ op definition; minor generator output adjustments. |
| src/cmd/compile/internal/ssa/_gen/AMD64.rules | Select MULXQ vs MULQU2 for Mul64uhilo based on GOAMD64 level. |
| src/cmd/compile/internal/amd64/ssa.go | Emit amd64 MULXQ instruction for OpAMD64MULXQ. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cmdTidy.Flag.BoolVar(&cfg.BuildV, "v", false, "print the names of packages as they are processed") | ||
| cmdTidy.Flag.BoolVar(&cfg.BuildX, "x", false, "print the commands") | ||
| cmdTidy.Flag.BoolVar(&tidyE, "e", false, "report errors but proceed anyway") | ||
| cmdTidy.Flag.BoolVar(&tidyDiff, "diff", false, "display the diff of the edits that would be mad") |
| cmdEdit.Flag.Var(flagFunc(flagExclude), "exclude", "add an exclude directive: `path@version`") | ||
| cmdEdit.Flag.Var(flagFunc(flagDropExclude), "dropexclude", "drop an exclude directive with the given `key`") | ||
| cmdEdit.Flag.Var(flagFunc(flagReplace), "replace", "add a replace directive: `old[@v]=new[@v]`") | ||
| cmdEdit.Flag.Var(flagFunc(flagDropReplace), "dropreplace", "drop a replace directive with the give `key`") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the merge conflicts in #146 so we can land the golang:master sync.
Conflicts resolved in src/cmd/go/internal/test/testflag.go and src/cmd/internal/obj/x86/obj6.go.
Local check: bash misc/gosentry/scripts/quickcheck.sh.