[pull] master from golang:master#133
Merged
Merged
Conversation
This doesn't restore the round-about old behavior where we used to take an inverse of the inverse of the carry flag to move data from flags to GP. Instead it performs the addition or substraction directly between GP and flags which is faster by 1 cycle (TP & LAT & instructions) Fixes #80399 Change-Id: Id90d3089322ae78887ad9e839838402b70d61ae2 Reviewed-on: https://go-review.googlesource.com/c/go/+/800460 Reviewed-by: Keith Randall <khr@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> Auto-Submit: Jorropo <jorropo.pgm@gmail.com> Reviewed-by: Russ Cox <rsc@golang.org>
GoVersion split each tag with strings.Cut(tag, "go1."), which finds the separator anywhere in the string. A tag that merely contained "go1.", such as "notgo1.21", was parsed as a go1.N constraint, so GoVersion reported a Go version for a tag that names no version. Use strings.CutPrefix so that only a leading "go1." is treated as a version tag prefix. Fixes #80406 Change-Id: I1aa71a571e15b0ad44200b02b67b619d594b62ad GitHub-Last-Rev: c9a7eb3 GitHub-Pull-Request: #80407 Reviewed-on: https://go-review.googlesource.com/c/go/+/800600 Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
arm64 rules folded a-x*y < 0 into a no-overflow comparison. This could end up being absorbed into InvertFlags via canonicalization. But absorbing InvertFlags into a noov comparison swaps < and >, which assumes a-b < 0 iff b-a > 0. That is false for the min int. Something similar happens for ARM. The noov ops were introduced in CL 233097 (and CL 236637 for 32-bit arm) specifically to make these flag-based comparisons honor wrapping. Unfortunately, the InvertFlags absorption reintroduces a wrapping bug they were meant to prevent, and it has been there since that original CL. The boolean (CSET) was added in CL 526276 for #62469 and tuned again in CL 526237, and is unsound for the same reason. The test added after #62469 only checked that compilation succeeded, not that it yielded the correct result. There's no great way to rescue this optimization on arm64. The easiest fix is to decline to fold the asymmetric comparisons in the first place. Every other noov source is a commutative CMN, which canonicalization never reverses. This omission doesn't hurt much; no generated code in std/cmd changes. With no CMP-based noov remaining, the (dangerous) noov InvertFlags absorption rules are dead. Remove them. On arm, the optimizations fired in a few functions. Again, though, the work and code and risk to rescue it outweighs the impact of the optimization. Removing it in this commit adds an instruction or two to: bytes.(*Buffer).WriteByte bufio.(*Reader).Read crypto/tls.(*Conn).Read go/types.(*Scope).Innermost runtime.(*mcentral).uncacheSpan sync.runtime_notifyListWait encoding/gob.(*Decoder).ignoreStruct Instead of adding a new test file, strengthen the #62469 test to check the results and to increase test coverage. Updates #62469 Change-Id: I108855c67831d550a580f4ddd40ed7a9705400db Reviewed-on: https://go-review.googlesource.com/c/go/+/786520 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: Keith Randall <khr@google.com> Reviewed-by: Keith Randall <khr@golang.org> Auto-Submit: Keith Randall <khr@golang.org>
Change "a a destructive" to "a destructive" in simdV21Narrow2's introductory comment. Change-Id: I0d86e97f140b384ac871498e922c6b10c0545bc8 Reviewed-on: https://go-review.googlesource.com/c/go/+/799721 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Russ Cox <rsc@golang.org> Auto-Submit: 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>
Change "the the" to "the" and "The places" to "This places" in the Midway compiler pass introductory comment. Change-Id: I3a43d8f0583a2ff68e75dc298f6f988bdc791fdc Reviewed-on: https://go-review.googlesource.com/c/go/+/799740 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Keith Randall <khr@google.com> Auto-Submit: Keith Randall <khr@golang.org> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
crt0_64.o is hardcoded to /lib/crt0_64.o, while its neighbors crtcxa.o and crtdbase.o were resolved via the external linker's --print-file-name=. This breaks cross builds using a compiler with a non-standard sysroot, or when using custom path for crt0_64.o through LIBPATH. Built Go on AIX, then set a custom LIBPATH and validated that that path was used for crt0_64.o. Change-Id: If94ebd1302eaefaed68bd8a67600e7ffa3edf714 Reviewed-on: https://go-review.googlesource.com/c/go/+/800880 Reviewed-by: Ian Lance Taylor <iant@golang.org> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Russ Cox <rsc@golang.org> Auto-Submit: Ian Lance Taylor <iant@golang.org>
Correctly sext or zext based on needs. I am not sure if the add codepath actually cares, this might be latent, however I am writing new code which is definitely failing. Change-Id: I6610b63c83d70c6b1be2f7c83cdf15ab5ea9d62d Reviewed-on: https://go-review.googlesource.com/c/go/+/795080 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Russ Cox <rsc@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>
url.MustParse is to url.Parse as uuid.MustParse is to uuid.Parse. This function replaces the one-liners that programmers have been writing to initialize variables with valid URL string constants. Fixes #79946 Change-Id: I2f94c89e10d36d83da713579ca6659b5237604ca Reviewed-on: https://go-review.googlesource.com/c/go/+/799820 Reviewed-by: Sean Liao <sean@liao.dev> Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Damien Neil <dneil@google.com>
Clang from MSVC does not support this flag. Instead, just pass -pthread like other platforms, which seems to work just fine with both MSVC clang and MinGW clang. Fixes #80290. Change-Id: If94a4b06d98b9a2778ea22ad74c29bb506653590 Reviewed-on: https://go-review.googlesource.com/c/go/+/800300 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: Quim Muntal <quimmuntal@gmail.com>
…TP/LDP
The SSA pair pass (cmd/compile/internal/ssa/pair.go) runs before regalloc
and only sees source-level loads and stores. Spill/reload code that
regalloc inserts later for OpStoreReg/OpLoadReg becomes individual STR/LDR
instructions that never get a chance to be paired, even when two spills
target adjacent 8-byte stack slots.
Fuse those pairs as the final step of code generation, in the compiler
rather than the assembler. A new ssagen.ArchInfo.SSAGenFinish hook runs
after genssa has emitted all of a function's Progs, resolved its branch
and jump-table targets, and finalized the frame size in defframe (so the
register-argument spills defframe inserts participate too); on arm64 it
walks the Prog list and rewrites strictly-adjacent AMOVD spill pairs that
share a base register and have consecutive 8-byte offsets into a single
ASTP/ALDP. The second Prog is reduced to a 0-byte ANOP rather than
unlinked so that branch targets referencing it remain valid. The pass is
skipped under -N to keep unoptimized builds unoptimized. Doing this in
the compiler keeps the assembler a simple translator.
Fusion is gated on several safety and profitability conditions:
- same base register and same Addr.Name (AUTO or PARAM, the only
classes spill slots use), distinct destination registers (LDP with
Rt1 == Rt2 is CONSTRAINED UNPREDICTABLE), and no pre/post-index or
register-offset addressing
- the resolved offset must encode in LDP/STP's signed 7-bit scaled
immediate, [-512, 504]: the assembler rewrites an AUTO offset to
off+framesize+8 and a PARAM offset to off+framesize+24 or +32
depending on frame alignment (off+8 in a frameless leaf, which is
not decided until assembly, so a frameless PARAM must fit both).
Checking the resolved value against the final frame size both
admits deep spill slots in large frames, where spills are most
common, and refuses fusions that would need an
assembler-synthesized address (ADD + LDP), which is no smaller
than the original pair and serializes through REGTMP
- the first load of a pair must not write the base register: executed
sequentially, the second load computes its address from the
just-loaded value, while LDP computes both addresses from the
original base
- the second instruction is not a branch or jump-table target
(otherwise paths that jump directly to it would skip the work the
LDP/STP now does at the first instruction's position) and does not
carry a statement boundary: genssa promotes instructions it reuses
as inline marks to statements, and inline marks must never become
zero-sized, while plain statement boundaries must keep their line
table entries
The prologue's register-argument spills around morestack, which the
assembler inserts during preprocess, are already emitted as STP/LDP
pairs (CL 621556).
TestPairSpills in cmd/compile/internal/arm64 drives pairSpills directly
with hand-constructed Prog chains, asserting the fused operands and
covering each fusion path and each gating condition.
test/codegen/memcombine.go pins down the spill/reload pattern that the
SSA pair pass misses but this pass catches.
test/fixedbugs/spillreload_arm64_pair.go exercises the conditional-call-
with-adjacent-reloads pattern from runtime.schedule that miscompiled
before the branch-target check was added. BenchmarkSpillReloadPair in
cmd/compile/internal/test improves from about 1.92 to 1.78 ns/op on an
Apple M4 Max (~7%).
armlint reports that adjacent STR/LDR pairings drop from 4354 -> 235 on
gofmt (94.6% reduction) and 26022 -> 772 on cmd/go (97.0% reduction).
The text section shrinks by 16720 bytes (1.40%) on gofmt and 101312
bytes (1.52%) on cmd/go.
Change-Id: Ib16ce06efdb733af71a5909ba15fedc47061a656
GitHub-Last-Rev: b7405f7
GitHub-Pull-Request: #79689
Reviewed-on: https://go-review.googlesource.com/c/go/+/783660
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>
Reviewed-by: Russ Cox <rsc@golang.org>
Fixes #80310 Change-Id: Ia4a37a1f4327df5ebe7f7b5c764969a44be2774a Reviewed-on: https://go-review.googlesource.com/c/go/+/798400 Reviewed-by: Jorropo <jorropo.pgm@gmail.com> Reviewed-by: Florian Lehner <lhnr.flrn@gmail.com> Reviewed-by: Keith Randall <khr@google.com> Auto-Submit: Jorropo <jorropo.pgm@gmail.com> Reviewed-by: Russ Cox <rsc@golang.org> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
It isn't safe, as it depends on the body of wbMove not spilling its arguments. Fixes #80196 Change-Id: If42afa161fff70527e310256cdc619d5741b7753 Reviewed-on: https://go-review.googlesource.com/c/go/+/795380 Reviewed-by: Keith Randall <khr@google.com> 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> Auto-Submit: Keith Randall <khr@golang.org>
Follows up CL 774020. This should have no behavior change. Change-Id: I5f1070bc5740c477654de10393d9c498ce257728 Reviewed-on: https://go-review.googlesource.com/c/go/+/787760 Reviewed-by: David Chase <drchase@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>
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. I also had to fix a bug in the runtime where in one edge case we were restoring the framepointer to a zero value. 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: Iea5801a4e14fc76bee06b48973b929d3d7d67a83 Reviewed-on: https://go-review.googlesource.com/c/go/+/548317 Auto-Submit: 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: Russ Cox <rsc@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
For #5370 Change-Id: I04ca4e38e5e4d54814eb5ec50e698d190c67a652 Reviewed-on: https://go-review.googlesource.com/c/go/+/784280 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Russ Cox <rsc@golang.org> 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> Reviewed-by: Michael Pratt <mpratt@google.com>
(*CrossOriginProtection).AddTrustedOrigin fails if its argument contains a path, a query, and/or a fragment, but it doesn't check for the presence of a userinfo part. Such an omission doesn't affect security (because serialized origins don't contain any userinfo part), but it causes invalid origins to be silently accepted, which could create some confusion. With this CL, the method now also fails if its argument contains a userinfo part (even if it's empty). Change-Id: Ic570997dfe062284c51498272f61f7c743c8eb16 GitHub-Last-Rev: 232ab09 GitHub-Pull-Request: #79766 Reviewed-on: https://go-review.googlesource.com/c/go/+/785520 Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Viacheslav Danilin <viacheslavdanilin@gmail.com> Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org> 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>
Replace the isSIMDTag and isSIMD bool fields on types.Type with bits in the existing flags bitset. Widen flags from bitset8 to bitset16 to make room. Change-Id: I856c6d4e28feaf7cc9dc5ef7a99b613bf9784d31 Reviewed-on: https://go-review.googlesource.com/c/go/+/788580 Reviewed-by: Keith Randall <khr@golang.org> 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> Auto-Submit: Michael Pratt <mpratt@google.com> Reviewed-by: Russ Cox <rsc@golang.org>
Document that GORISCV64=rva23u64 defines hasZfa and hasZicond, matching the feature macros provided by runtime/asm_riscv64.h. Change-Id: I26e2d49ea8f74951ebf35002beeb812dde632a40 Reviewed-on: https://go-review.googlesource.com/c/go/+/796820 Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com> Auto-Submit: Joel Sing <joel@sing.id.au> Reviewed-by: Julian Zhu <jz531210@gmail.com> Reviewed-by: Meng Zhuo <mengzhuo1203@gmail.com> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Joel Sing <joel@sing.id.au>
Embedding types can override exported reflect methods, which can cause problems. Initially Gemini-authored, extensively revised. This addresses some of the issues mentioned in comments on CL 799481. Fixes #80332. Change-Id: Icbaf2ba83cf372e611a432d732094bcfe50a2e6c Reviewed-on: https://go-review.googlesource.com/c/go/+/801180 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Ian Lance Taylor <iant@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>
On strict-alignment targets, only combine loads and stores when the merged memory access is known to be aligned. Share pointer-alignment analysis between load and store combining. Keep it conservative by deriving alignment from typed base pointers and constant offsets, and by treating unknown pointer operations as minimally aligned. Add riscv64 codegen tests covering aligned and unaligned load and store memcombine cases. Fixes #71778 Change-Id: If6fbd040edb43fc02b7f343085fa20027695c77d Reviewed-on: https://go-review.googlesource.com/c/go/+/795260 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: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
EncodeToken currently rejects comment contents only when they contain "-->", which allows other occurrences of "--" and emits invalid XML. It also emits "--->" when comment content ends in a hyphen. Reject any "--" in Comment tokens, as required by XML 1.0, and insert a space before the closing delimiter when comment content ends in a hyphen. This matches encoding of struct fields tagged ",comment". Fixes #80155 Change-Id: I9ca9af44af15d58908cd471a9cf7ddd76baae33f GitHub-Last-Rev: 8e5beca GitHub-Pull-Request: #80395 Reviewed-on: https://go-review.googlesource.com/c/go/+/800361 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: Russ Cox <rsc@golang.org>
For Value.Set, when assigning to an interface, we use the receiver's storage as a scratch space for the implicit conversion to interface, which then assigns back to the receiver. The compiler does not know 1. this occurs only when assigning to an interface with an implicit conversion, 2. this is actually a self- assignment. So it treats the receiver escape. Mark the scratch space noescape to help the compiler. So now Value.Set does not leak the receiver. Change-Id: If904277fa15912653936fd4038173691170e8789 Reviewed-on: https://go-review.googlesource.com/c/go/+/799480 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alan Donovan <adonovan@google.com> Reviewed-by: David Chase <drchase@google.com>
mayCall treats division as potentially panicking, forcing it into a temporary before marshaling call arguments. That is necessary for integer division, but ordinary float division does not panic or call unless the target uses soft-float lowering. Handle float division like the other soft-float-sensitive arithmetic ops so hard-float targets can pass it directly as a call argument. Fixes #28698. Change-Id: I6b2377176255aef1ae31be16a578641f75e71f75 Reviewed-on: https://go-review.googlesource.com/c/go/+/782461 Reviewed-by: Russ Cox <rsc@golang.org> 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>
CL 248358 changed encoding/json to detect cyclic maps and slices and return an error, so jsValEscaper no longer risks unbounded recursion for these values. Add regression tests verifying that cyclic values produce a safely commented error followed by null, and remove the stale TODO. Change-Id: I83b6171b1d043c43d54890bd40983e2aef01ff4b Reviewed-on: https://go-review.googlesource.com/c/go/+/796121 Reviewed-by: Neal Patel <neal@golang.org> Reviewed-by: Neal Patel <nealpatel@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Russ Cox <rsc@golang.org> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Fixes #75968 Change-Id: I3ce84d01d0db6560ceb042915ec8fa11aa871a34 Reviewed-on: https://go-review.googlesource.com/c/go/+/714620 Auto-Submit: Sean Liao <sean@liao.dev> Reviewed-by: Eric Grosse <grosse@gmail.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Extend the rttype fold in (*ssa.Func).isFixedLoad / rewriteFixedLoad
to also handle abi.Type.TFlag, so that loads of *(*abi.Type)(p).TFlag
are constant-folded against the static type.
Store the abi.TFlag value as a cached field on types.Type, computed
on first use by (*Type).TFlag with no side effects, so the parallel
SSA backend can invoke it safely. TFlagGCMaskOnDemand is derived
directly from PtrDataSize without calling dgcsym, and the new
hasUncommon predicate covers TFlagUncommon without generating
method wrappers. hasUncommon requires typecheck.CalcMethods to
have run on the receiver base type and asserts a new
(*Type).MethodsComputed bit, set by CalcMethods, to catch missing
preparation.
compilecmp HEAD^ -> HEAD
HEAD^ (920fc8d5f3be): cmd/compile/internal/types: store SIMD flags in Type.flags bitset
HEAD (9053d82f4827): cmd/compile: optimize abi.Type.TFlag loads
file before after Δ %
asm 7220974 7220918 -56 -0.001%
cgo 6199238 6198918 -320 -0.005%
compile 35848643 35846357 -2286 -0.006%
cover 7907279 7895727 -11552 -0.146%
fix 12695300 12695124 -176 -0.001%
link 10090470 10088702 -1768 -0.018%
preprofile 3380078 3379006 -1072 -0.032%
vet 12295155 12288275 -6880 -0.056%
total 95637137 95613027 -24110 -0.025%
cmd/cgo/internal/test
cmd/cgo/internal/test.test45451 319 -> 255 (-20.06%)
cmd/compile/internal/base
cmd/compile/internal/base.registerFlags 3120 -> 3088 (-1.03%)
cmd/compile/internal/reflectdata
cmd/compile/internal/reflectdata.dcommontype 2079 -> 1886 (-9.28%)
cmd/compile/internal/ssa
cmd/compile/internal/ssa.isFixedLoad 702 -> 712 (+1.42%)
cmd/compile/internal/ssa.rewriteFixedLoad 2757 -> 2853 (+3.48%)
cmd/compile/internal/typecheck
cmd/compile/internal/typecheck.InitUniverse 3916 -> 3965 (+1.25%)
cmd/compile/internal/types
inserted cmd/compile/internal/types.(*Type).TFlag
inserted cmd/compile/internal/types.(*Type).TFlag.deferwrap1
inserted cmd/compile/internal/types.computeTFlag
inserted cmd/compile/internal/types.hasUncommon
cmd/gofmt
main.init 1100 -> 1068 (-2.91%)
main.rewriteFile 716 -> 696 (-2.79%)
main.rewriteFile.func1 538 -> 517 (-3.90%)
cmd/vendor/github.com/google/pprof/internal/driver
cmd/vendor/github.com/google/pprof/internal/driver.(*config).fieldPtr 133 -> 113 (-15.04%)
cmd/vendor/golang.org/x/telemetry/internal/crashmonitor
cmd/vendor/golang.org/x/telemetry/internal/crashmonitor.sentinel 72 -> 51 (-29.17%)
cmd/vendor/golang.org/x/tools/go/ast/astutil
cmd/vendor/golang.org/x/tools/go/ast/astutil.updateBasicLitPos 317 -> 302 (-4.73%)
cmd/vendor/golang.org/x/tools/internal/refactor/inline
cmd/vendor/golang.org/x/tools/internal/refactor/inline.clearPositions.func1 525 -> 505 (-3.81%)
cmd/vendor/golang.org/x/tools/internal/typesinternal
cmd/vendor/golang.org/x/tools/internal/typesinternal.ErrorCodeStartEnd 631 -> 599 (-5.07%)
cmd/vendor/golang.org/x/tools/internal/typesinternal.SetUsesCgo 185 -> 165 (-10.81%)
encoding/asn1
encoding/asn1.parseField 9101 -> 9037 (-0.70%)
encoding/gob
encoding/gob.(*Decoder).recvType 340 -> 325 (-4.41%)
encoding/gob.(*Encoder).sendActualType 1117 -> 1103 (-1.25%)
encoding/json/v2
encoding/json/v2.makeInterfaceArshaler.func1 2840 -> 2760 (-2.82%)
encoding/json/v2.marshalArrayAny 1463 -> 1431 (-2.19%)
encoding/json/v2.marshalObjectAny 2490 -> 2416 (-2.97%)
encoding/json/v2.unmarshalInlinedFallbackNext 2813 -> 2768 (-1.60%)
encoding/xml
encoding/xml.(*Decoder).unmarshal 8725 -> 8597 (-1.47%)
encoding/xml.(*Decoder).unmarshalAttr 1838 -> 1816 (-1.20%)
encoding/xml.(*printer).marshalSimple 1064 -> 1034 (-2.82%)
fmt
fmt.(*pp).fmtBytes 1716 -> 1690 (-1.52%)
internal/buildcfg
internal/buildcfg.ParseGOEXPERIMENT 2040 -> 2021 (-0.93%)
internal/buildcfg.expList 1350 -> 1306 (-3.26%)
net/http/internal/http2
net/http/internal/http2.StreamError.As 1208 -> 1112 (-7.95%)
reflect
reflect.Value.Seq.func1.1 229 -> 178 (-22.27%)
reflect.Value.Seq.func2 167 -> 134 (-19.76%)
reflect.Value.Seq.func3 208 -> 178 (-14.42%)
reflect.Value.Seq.func4 255 -> 221 (-13.33%)
reflect.Value.Seq2.func1.1 261 -> 217 (-16.86%)
reflect.Value.Seq2.func2 335 -> 293 (-12.54%)
reflect.Value.Seq2.func3 284 -> 245 (-13.73%)
reflect.Value.Seq2.func4 336 -> 284 (-15.48%)
runtime
runtime.stkobjinit 202 -> 165 (-18.32%)
testing
testing.(*F).Fuzz.func1.1 856 -> 824 (-3.74%)
text/template
text/template.(*state).evalArg 2252 -> 2228 (-1.07%)
text/template.(*state).evalCall 4188 -> 4156 (-0.76%)
text/template.(*state).evalCommand 1301 -> 1266 (-2.69%)
text/template.(*state).evalEmptyInterface 988 -> 955 (-3.34%)
text/template.(*state).evalField 3211 -> 3115 (-2.99%)
text/template.(*state).idealConstant 504 -> 445 (-11.71%)
text/template.(*state).validateType 1655 -> 1640 (-0.91%)
text/template.(*state).walkRange 3269 -> 3205 (-1.96%)
text/template.init 459 -> 431 (-6.10%)
Change-Id: I5ffefdc4e09586ab4f154e560b99b2d9cd30c2ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/701299
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: Keith Randall <khr@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
addNamedValue takes &loc of a local and stores it in the long-lived
f.Names and f.CanonicalLocalSlots, causing loc to escape on every call.
Allocate only on the first-sighting path, mirroring localSlotAddr.
Passes toolstash -cmp.
compilebench -alloc, allocs/op, linux/amd64 (-count 20):
│ /tmp/old.txt │ /tmp/new.txt │
│ allocs/op │ allocs/op vs base │
Template 820.9k ± 0% 816.2k ± 0% -0.58% (p=0.000 n=20)
Unicode 574.2k ± 0% 573.7k ± 0% -0.07% (p=0.000 n=20)
GoTypes 5.013M ± 0% 4.971M ± 0% -0.83% (p=0.000 n=20)
Compiler 702.2k ± 0% 698.6k ± 0% -0.51% (p=0.000 n=20)
SSA 47.02M ± 0% 46.50M ± 0% -1.12% (p=0.000 n=20)
Flate 823.9k ± 0% 813.8k ± 0% -1.23% (p=0.000 n=20)
GoParser 791.3k ± 0% 788.1k ± 0% -0.41% (p=0.000 n=20)
Reflect 2.318M ± 0% 2.302M ± 0% -0.69% (p=0.000 n=20)
Tar 937.7k ± 0% 930.1k ± 0% -0.81% (p=0.000 n=20)
XML 1.099M ± 0% 1.090M ± 0% -0.77% (p=0.000 n=20)
LinkCompiler 729.0k ± 0% 729.1k ± 0% ~ (p=0.101 n=20)
ExternalLinkCompiler 731.6k ± 0% 731.6k ± 0% ~ (p=0.449 n=20)
LinkWithoutDebugCompiler 143.3k ± 0% 143.3k ± 0% ~ (p=0.358 n=20)
geomean 1.187M 1.181M -0.54%
Fixes #79990
Change-Id: Ia0dfff98d4135912f4b008ea44a6e732f72c0d8a
Reviewed-on: https://go-review.googlesource.com/c/go/+/790300
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Podtserkovskii <michaelpo@meta.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
… int EncodedLen computes the encoded length with int arithmetic that can wrap for very large n, returning a negative or wrapped value. Callers that size an allocation from the result (EncodeToString, AppendEncode) then panic with no indication of the cause, or allocate an undersized buffer. CL 510635 and CL 512200 raised the overflow threshold of the unpadded branches but did not eliminate it, and the padded branches were unchanged: on 32-bit platforms base64.StdEncoding.EncodedLen(1610612736) still returns -2147483648. Make EncodedLen panic when the encoded length does not fit in an int, document the panic, and document the largest n that is guaranteed not to panic. DecodedLen needs no change: its result is at most n and cannot overflow. Fixes #20235 Change-Id: I963a55062a790017b42f49fc3a9178fc3bb024ef GitHub-Last-Rev: 2a7abbb GitHub-Pull-Request: #80373 Reviewed-on: https://go-review.googlesource.com/c/go/+/799800 Auto-Submit: Ian Lance Taylor <iant@golang.org> 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: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
…wline When a passing test in package list mode writes output that does not end in a newline, and that output is discarded (the default non-verbose, non-JSON case), cmd/go printed a spurious blank line before the "ok" summary line. The newline-before-"ok" guard tested out, a snapshot of the captured output taken before buf.Reset() discards it, but wrote the newline into buf, which Reset had already emptied. Test buf itself, so no newline is added when the captured output was discarded. Fixes #79786 Change-Id: Iab32de2d8fa9db85681e2e4f01d7af3f08a0f626 GitHub-Last-Rev: 86aaa23 GitHub-Pull-Request: #80352 Reviewed-on: https://go-review.googlesource.com/c/go/+/799182 Reviewed-by: Russ Cox <rsc@golang.org> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Sean Liao <sean@liao.dev> Reviewed-by: Michael Pratt <mpratt@google.com> Auto-Submit: Sean Liao <sean@liao.dev>
Use bytes.IndexByte insted of open-coding it in getString. Change-Id: I306074b092c879cc1f20cae2acb236d2e9775b59 Reviewed-on: https://go-review.googlesource.com/c/go/+/789540 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Russ Cox <rsc@golang.org> Auto-Submit: Tobias Klauser <tobias.klauser@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: Florian Lehner <lhnr.flrn@gmail.com>
Change-Id: I173f165dea17d21cb344a208bce9b70e6dd6a35a Reviewed-on: https://go-review.googlesource.com/c/go/+/767160 Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Sean Liao <sean@liao.dev> 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> Auto-Submit: Sean Liao <sean@liao.dev>
Change-Id: I619224cb379d917a2766dfce5ada9f9bf4617d81 Reviewed-on: https://go-review.googlesource.com/c/go/+/800500 Auto-Submit: Sean Liao <sean@liao.dev> Reviewed-by: Sean Liao <sean@liao.dev> Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Russ Cox <rsc@golang.org> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change-Id: I7eb441028af0f3e579c87f63458029d8125c4046 Reviewed-on: https://go-review.googlesource.com/c/go/+/790102 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Sean Liao <sean@liao.dev> Reviewed-by: Michael Pratt <mpratt@google.com> Auto-Submit: Sean Liao <sean@liao.dev> Reviewed-by: Russ Cox <rsc@golang.org>
The IsTrue comment says a value is 'true' when it is not the zero value of its type. That contradicts the implementation for structs, which are always true, and is wrong for a negative floating-point zero, which is untruthy without being 'the' zero of its type. Describe the rules the implementation actually applies instead. The comment is duplicated in html/template; update both copies. Fixes #28394 Change-Id: Idbbabec39ce46bddbbdd4877ac8cfe2b5d4a08dc GitHub-Last-Rev: 435105c GitHub-Pull-Request: #80338 Reviewed-on: https://go-review.googlesource.com/c/go/+/799063 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: Sean Liao <sean@liao.dev> Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Func.Names was []*LocalSlot, so every entry was a separately heap-allocated LocalSlot. Most named values are scalars that are never decomposed, yet each one still allocated a LocalSlot when it was first recorded in addNamedValue. Change Func.Names to []LocalSlot. addNamedValue now appends the slot by value and no longer eagerly fills CanonicalLocalSlots. The canonical *LocalSlot used for SplitOf chains and split deduplication is created lazily by localSlotAddr, so only slots that are actually decomposed pay for it. The names stay contiguous and the garbage collector has one fewer pointer per name to scan. This is a follow-up to CL 790300, the addNamedValue escape fix for issue #79990. That change stopped the slot escaping; this one removes the remaining per-name allocation for slots that are never decomposed. Passes toolstash -cmp; std and cmd build byte-identical. allocs/op via compilebench -alloc, linux/amd64 (-count 20); all packages ±0%, p<=0.001. "vs parent" is the win over CL 790300, "vs master" folds in CL 790300 too: allocs/op vs parent vs master Template 814.0k -0.16% -0.76% Unicode 573.7k -0.03% -0.10% GoTypes 4.956M -0.26% -1.08% SSA 46.32M -0.37% -1.51% Flate 811.6k -0.30% -1.45% GoParser 785.8k -0.23% -0.78% Reflect 2.293M -0.41% -1.11% Tar 927.5k -0.34% -1.02% XML 1.088M -0.32% -0.92% geomean 1.755M -0.27% -0.97% Updates #79990 Change-Id: Ib277056a47585d7805438955062c2f59bd77d42f Reviewed-on: https://go-review.googlesource.com/c/go/+/790920 Reviewed-by: Michael Podtserkovskii <michaelpo@meta.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: Michael Pratt <mpratt@google.com> Reviewed-by: Keith Randall <khr@google.com> Auto-Submit: Michael Pratt <mpratt@google.com>
go test -cover runs "go tool covdata", which links covdata, caches the
executable into $GOCACHE (CacheExecutable), and execs it from there via
runBuiltTool. When many packages are tested at once (go test ./...), the
covdata invocations share one build cache. A concurrent go process can
still hold a writable descriptor to the freshly written cached file when
this process execs it, so the exec fails with ETXTBSY ("text file busy"):
go tool covdata: fork/exec $GOCACHE/<hash>/covdata: text file busy
Every other exec-from-cache path already retries this race: base.RunStdin
and (*runTestActor).Act in cmd/go/internal/test both loop on ETXTBSY with
backoff (see #22220, #22315). runBuiltTool was the one path that did not.
The race became observable in 1.26 after CL 668035 (test barrier actions)
reordered the scheduling of test/coverage run actions; the underlying
missing-retry bug was latent before that.
Wrap toolCmd.Start() in the same bounded retry. Retrying Start() is safe:
once it succeeds the exec has happened (no ETXTBSY is possible after), and
on failure the child never ran, so re-execing has no side effects.
Fixes #78204
Change-Id: I77d0a5804e69d4fb386bbdb4aa9c5f3b179261b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/790840
Reviewed-by: Michael Matloob <matloob@golang.org>
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: Michael Pratt <mpratt@google.com>
Reviewed-by: Sean Liao <sean@liao.dev>
Auto-Submit: Michael Pratt <mpratt@google.com>
Since Go source code is in UTF-8, smart quotes should be part of the original source if they are desired. Characters important to programming languages should not be mangled. Fixes #54312 Fixes #76975 Change-Id: I20331669bb691eafe66ebcc3f8513c4f6a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/732420 Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Russ Cox <rsc@golang.org> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: kortschak <dan@kortschak.io> Reviewed-by: Rob Pike <r@golang.org>
On a clean GOCACHE, buildPkgsite returned a temporary path deleted by b.Close() before the command execution. Instead, this CL always returns the cached executable path even it is fresh built ensuring the existence of the doc executable. +test Fixes #80287 Change-Id: I949d01dceac817082fa5e38450d0b0b39fc325a3 Reviewed-on: https://go-review.googlesource.com/c/go/+/800720 Reviewed-by: Dmitri Shuralyov <dmitshur@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@golang.org> Reviewed-by: Michael Matloob <matloob@google.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
20 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )