Austin Clements [Fri, 21 Oct 2022 19:18:14 +0000 (15:18 -0400)]
misc/cgo/test: simplify for module mode
When we were first introducing module mode, CL 163418 moved many of
the tests in misc/cgo/test into their own test binary under testdata
so misc/cgo/test continued to work in both GOPATH mode and module
mode. This introduce a somewhat complicated test driver into
misc/cgo/test. Since the misc/cgo/test test had to invoke "go test" as
a subprocess, this required care to thread any build flags down into
the subprocess. The output from any failures of the sub-process was
also less than ideal.
Now that we don't have to worry about running these in GOPATH mode any
more, this CL moves all of the tests back into misc/cgo/test and drops
the test driver.
There are two slight complications:
- Test41761 was added after this split and has a C type "S" that's
also present in misc/cgo/test itself. We rename that to keep that
test working.
- TestCgo in go/internal/srcimporter now fails to import misc/cgo/test
because misc/cgo/test now contains imports of other "misc" module
packages and the importer it sets up isn't configured to allow that.
We fix this by setting up a build context that's configured for
this.
Cherry Mui [Fri, 4 Nov 2022 00:56:21 +0000 (20:56 -0400)]
cmd/compile/internal/pgo: use slices for in/out edges
Currently in the pprof Graph, a Node's in/out edges are
represented as maps, keyed by the source/destination Nodes. For a
Node it usually has a very small number of edges, so linear search
would be generally faster than map operations. Use slices and
linear search instead.
Change-Id: I5ab06be0c019373cd3a845b9f3282731372e9c4c
Reviewed-on: https://go-review.googlesource.com/c/go/+/447802 Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cherry Mui [Fri, 4 Nov 2022 00:33:34 +0000 (20:33 -0400)]
cmd/compile/internal/pgo: remove some unused fields
We intentionally don't use file path so it is resilient to code
moving. OrigName and Objfile are also not used currently. Remove
them. (We can add them back if it turns out to be useful.)
Change-Id: I7975d78c874bc21475b9119301088452a4426cb9
Reviewed-on: https://go-review.googlesource.com/c/go/+/447801 Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
David Chase [Fri, 4 Nov 2022 15:23:12 +0000 (11:23 -0400)]
cmd/compile: change the multiple (gossa)hash separator from ; to /
Semicolon has bad copy-paste ergonomics; it requires quoting.
Slash is okay, and won't be a separator ever in debug strings
because it is already used in e.g. ssa/phase/debug=etc.
Change-Id: I493360e9282666eea1a342971a77df2ebd6c92ad
Reviewed-on: https://go-review.googlesource.com/c/go/+/447975 Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cherry Mui [Fri, 4 Nov 2022 17:54:59 +0000 (13:54 -0400)]
cmd/compile: fix PGO cross-package inlining
With CL 447015, we identify hot callees from edge weights, but
the code only traverses edges for calls from the current package.
If the callee is in a different package, when compiling that
package, the edge was not visited, so the callee was not actually
marked inline candidate. This CL fixes it by traversing all hot
edges.
Change-Id: If668c1a16ebe34e3474376b88ab3a84be76b8562
Reviewed-on: https://go-review.googlesource.com/c/go/+/448015
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
cuiweixie [Fri, 4 Nov 2022 02:48:39 +0000 (10:48 +0800)]
api: add newline to 55301.txt
Change-Id: I9bcaba0ca9c30b204bf126dbfec4cd48d9ebe2ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/447896 Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Than McIntosh <thanm@google.com>
Run-TryBot: xie cui <523516579@qq.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Eli Bendersky [Thu, 3 Nov 2022 19:16:06 +0000 (12:16 -0700)]
text/template/parse: fix interaction of break/continue keywords and functions
Fixes a bug that was introduced in CL 410414; in that CL, to avoid
a race condition in the initialization of the lexer, the setting
of the breakOK and continueOK options was moved to before
Tree.funcs was populated from parameters. As a result, the parser
missed the fact that 'break' and 'continue' were defined as functions.
Following CL 421883 race conditions are no longer an issue, so
the simplest fix is just to move the initialization where it was
before - in startParse, after t.funcs has been set.
Fixes #56538
Change-Id: I3b99fe9ad12255a4f6eb9a00eb3f64529ff055c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/447775
Run-TryBot: Eli Bendersky <eliben@golang.org>
Auto-Submit: Eli Bendersky <eliben@golang.org> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Eli Bendersky <eliben@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
cuiweixie [Sun, 23 Oct 2022 00:41:38 +0000 (08:41 +0800)]
net: add ControlContext to Dialer
Fixes #55301
Change-Id: Ie8abcd383eee9af75038bde908ac638f43d33b7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/444955 Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: xie cui <523516579@qq.com>
Matthew Dempsky [Thu, 3 Nov 2022 18:02:51 +0000 (11:02 -0700)]
os/exec: allow NUL in environment variables on Plan 9
Plan 9 uses NUL as os.PathListSeparator, so it's almost always going
to appear in the environment variable list. Exempt GOOS=plan9 from the
check for NUL in environment variables.
For #56284.
Fixes #56544.
Change-Id: I23df233cdf20c0a9a606fd9253e15a9b5482575a
Reviewed-on: https://go-review.googlesource.com/c/go/+/447715 Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Damien Neil <dneil@google.com>
Ian Lance Taylor [Thu, 3 Nov 2022 20:22:37 +0000 (13:22 -0700)]
testing: add Get method for -test.v option
There is existing code that calls flag.Lookup("test.v") and inspects
the value. That stopped working as of CL 443596. Make code like that
continue to work at least for the case where we aren't using
-test.v=test2json.
Change-Id: Idb30b149b48ee3987a201e349cf4d9bfe9ddee56
Reviewed-on: https://go-review.googlesource.com/c/go/+/447796
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Paul E. Murphy [Mon, 12 Jul 2021 21:53:22 +0000 (16:53 -0500)]
cmd/link: support PPC64 prefixed relocations for power10
Handle emitting (to ld) or resolving commonly used ELFv2 1.5
relocations. The new ISA provides PC relative addressing with
34 bit signed addresses, and many other relocations which can
replace addis + d-form type relocations with a single prefixed
instruction.
Updates #44549
Change-Id: I7d4f4314d1082daa3938f4353826739be35b0e81
Reviewed-on: https://go-review.googlesource.com/c/go/+/355149 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
Matthew Dempsky [Thu, 3 Nov 2022 18:56:43 +0000 (11:56 -0700)]
cmd/compile: allow ineffectual //go:linkname in -lang=go1.17 and older
Prior to Go 1.18, ineffectual //go:linkname directives (i.e.,
directives referring to an undeclared name, or to a declared type or
constant) were treated as noops. In Go 1.18, we changed this into a
compiler error to mitigate accidental misuse.
However, the x/sys repo contained ineffectual //go:linkname directives
up until go.dev/cl/274573, which has caused a lot of user confusion.
It seems a bit late to worry about now, but to at least prevent
further user pain, this CL changes the error message to only apply to
modules using "go 1.18" or newer. (The x/sys repo declared "go 1.12"
at the time go.dev/cl/274573 was submitted.)
This needs to be as low as possible while not breaking priority
assumptions of other scores to correctly schedule carry chains.
Prior to the arm64 changes, it was set below ReadTuple. At the time,
this prevented the MulHiLo implementation on PPC64 from occluding
the scheduling of a full carry chain.
Memory scores can also prevent better scheduling, as can be observed
with crypto/internal/edwards25519/field.feMulGeneric.
Fixes #56497
Change-Id: Ia4b54e6dffcce584faf46b1b8d7cea18a3913887
Reviewed-on: https://go-review.googlesource.com/c/go/+/447435 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Michael Matloob [Wed, 2 Nov 2022 18:36:51 +0000 (14:36 -0400)]
go/build: don't add unparsable non-Go files to InvalidGoFiles
go/build attempts to parse comments at the beginning of non-Go files
looking for //go:build or //+go build comments. Before this change,
if the beginning of the non-Go file failed to parse (perhaps because
it is in a format that isn't even meant to be built with Go code) the
file would be added to InvalidGoFiles. The comment for InvalidGoFiles
states that it contains Go files, so this is clearly incorrect
behavior.
Further, if there was a directory that only contained these unparsable
non-Go files, it would have a non-zero number of InvalidGoFiles, and
the matching code in cmd/go/internal/search/search.go in
(*Match).MatchDirs would treat it as a directory containing (invalid)
Go files and would match the directory as a Go package. This incorrect
behavior is also fixed by this CL.
Fixes #56509
Change-Id: Id0d905827c71f7927f7c2fa42b236181950af7e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/447357
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
David Chase [Wed, 28 Sep 2022 21:10:59 +0000 (17:10 -0400)]
cmd/compile: if GOGC is not set, temporarily boost it for rapid starting heap growth
Benchmarking suggests about a 14-17% reduction in user build time,
about 3.5-7.8% reduction for wall time. This helps most builds
because small packages are common. Latest benchmarks (after the last
round of improvement):
(The number of compiler workers is capped at min(4, GOMAXPROCS))
An earlier, similar version of this CL at one point observed a 27%
reduction in user build time (building 40+ benchmarks, 20 times), but
the current form is judged to be the most reliable; it may be
profitable to tweak the numbers slightly later, and/or to adjust the
number of compiler workers.
We've talked about doing this in the past, the "new"(ish) metrics
package makes it a more tractable proposition.
The method here is:
1. If os.Getenv("GOGC") is empty, then increase GOGC to a large value,
calculated to grow the heap to 32 + 4 * compile_parallelism before a
GC occurs (e.g., on a >= 4 processor box, 64M). In practice,
sometimes GC occurs before that, but this still results in fewer GCs
and saved time. This is "heap goal".
2. Use a finalizer to approximately detect when GC occurs, and use
runtime metrics to track progress towards the goal heap size,
readjusting GOGC to retarget it as necessary. Reset GOGC to 100 when
the heap is "close enough" to the goal.
One feared failure mode of doing this is that the finalizer will be
slow to run and the heap will grow exceptionally large before GOGC is
reset; I monitored the heap size at reset and exit across several
boxes with a variety of processor counts and extra noise
(including several builds in parallel, including a laptop with a busy
many-tabs browser running) and overshoot effectively does not occur.
In some cases the compiler's heap grows so rapidly that estimated live
exceeds the GC goal, but this is not delayed-finalizer overshoot; the
compiler is just using that much memory. In a small number of cases
(3% of GCs in make.bash) the new goal is larger than predicted by as
much as 38%, so check for that and redo the adjustment.
I considered instead using the maximum heap size limit +
GC-detecting-finalizer + reset instead, but to me that seemed like it
might have a worse bad-case outcome; if the reset is delayed, it's
possible the GC would start running frequently, making it harder to
run the finalizer, reach 50% utilization, and the extra GCs would
lose the advantage. This might also perform badly in the case that a
rapidly growing heap outruns goal. In practice, this sort of
overshoot hasn't been observed, and a goal of 64M is small enough to
tolerate plenty of overshoot anyway.
This version of the CL includes a comment urging anyone who sees the
code and thinks it would work for them, to update a bug (to be
created if the CL is approved) with information about their
situation/experience, so that we may consider creating some more
official and reliable way of obtaining the same result.
Change-Id: I45df1c927c1a7d7503ade1abd1a3300e27516633
Reviewed-on: https://go-review.googlesource.com/c/go/+/436235
Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
David Chase [Fri, 21 Oct 2022 22:10:23 +0000 (18:10 -0400)]
cmd/compile: add debug-hash flag for fused-multiply-add
This adds a -d debug flag "fmahash" for hashcode search for
floating point architecture-dependent problems. This variable has no
effect on architectures w/o fused-multiply-add.
This was rebased onto the GOSSAHASH renovation so that this could have
its own dedicated environment variable, and so that it would be
cheap (a nil check) to check it in the normal case.
The intended use is in conjunction with github.com/dr2chase/gossahash,
which will probably acquire a flag "-fma" to streamline its use. This
tool+use was inspired by an ad hoc use of this technique "in anger"
to debug this very problem. This is also a dry-run for using this
same technique to identify code sensitive to loop variable
lifetime/capture, should we make that change.
Example intended use, with current search tool (using old environment
variable), for a test example:
gossahash -e GOFMAHASH GOMAGIC=GOFMAHASH go run fma.go
Trying go args=[...], env=[GOFMAHASH=1 GOMAGIC=GOFMAHASH]
go failed (81 distinct triggers): exit status 1
Trying go args=[...], env=[GOFMAHASH=11 GOMAGIC=GOFMAHASH]
go failed (39 distinct triggers): exit status 1
Trying go args=[...], env=[GOFMAHASH=011 GOMAGIC=GOFMAHASH]
go failed (18 distinct triggers): exit status 1
Trying go args=[...], env=[GOFMAHASH=0011 GOMAGIC=GOFMAHASH]
Trying go args=[...], env=[GOFMAHASH=1011 GOMAGIC=GOFMAHASH]
...
Trying go args=[...], env=[GOFMAHASH=0110111011 GOMAGIC=GOFMAHASH]
Trying go args=[...], env=[GOFMAHASH=1110111011 GOMAGIC=GOFMAHASH]
go failed (2 distinct triggers): exit status 1
Trigger string is 'GOFMAHASH triggered math.qzero:427 111111101010011110111011', repeated 6 times
Trigger string is 'GOFMAHASH triggered main.main:20 010111010000101110111011', repeated 1 times
Trying go args=[...], env=[GOFMAHASH=01110111011 GOMAGIC=GOFMAHASH]
go failed (1 distinct triggers): exit status 1
Trigger string is 'GOFMAHASH triggered main.main:20 010111010000101110111011', repeated 1 times
Review GSHS_LAST_FAIL.0.log for failing run
FINISHED, suggest this command line for debugging:
GOSSAFUNC='main.main:20 010111010000101110111011' \
GOFMAHASH=01110111011 GOMAGIC=GOFMAHASH go run fma.go
Change-Id: Ifa22dd8f1c37c18fc8a4f7c396345a364bc367d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/394754
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Cherry Mui [Tue, 1 Nov 2022 16:33:58 +0000 (12:33 -0400)]
cmd/compile: use CDF to determine PGO inline threshold
Currently in PGO we use a percentage threshold to determine if a
callsite is hot. This CL uses a different method -- treating the
hottest callsites that make up cumulatively top X% of total edge
weights as hot (X=95 for now). This default might work better for
a wider range of profiles. (The absolute threshold can still be
changed by a flag.)
For #55022.
Change-Id: I7e3b6f0c3cf23f9a89dd5994c10075b498bf14ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/447016
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Zeke Lu [Wed, 2 Nov 2022 16:55:51 +0000 (16:55 +0000)]
cmd/go/internal/vcs: also check file mode when identifying VCS root
Currently, FromDir identifies a VCS checkout directory just by checking
whether it contains a specified file. This is not enough. For example,
although there is a ".git" file (a plain file, not a directory) in a
git submodule directory, this directory is not a git repository root.
This change takes the file mode into account. As of now, the filename
and file mode for the supported VCS tools are:
Alexander Scheel [Wed, 2 Nov 2022 16:19:23 +0000 (16:19 +0000)]
crypto/x509: create CRLs with Issuer.RawSubject
Per discussion with Roland Shoemaker, this updates
x509.CreateRevocationList to mirror the behavior of
x509.CreateCertificate, creating an internal struct for the ASN.1
encoding of the CRL. This allows us to switch the Issuer field type to
asn1.RawValue, bypassing the round-tripping issues of pkix.Name in most
scenarios.
Per linked ticket, this resolves issues where a non-Go created
certificate can be used to create CRLs which aren't correctly attested
to that certificate. In rare cases where the CRL issuer is validated
against the certificate's issuer (such as the linked JDK example), this
can result in failing to check this CRL for the candidate certificate.
Fixes #53754
Change-Id: If0adc053c081d6fb0b1ce47324c877eb2429a51f
GitHub-Last-Rev: 033115dd5eb93295330eb151ff8557df5bffbec8
GitHub-Pull-Request: golang/go#53985
Reviewed-on: https://go-review.googlesource.com/c/go/+/418834 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Filippo Valsorda [Thu, 25 Aug 2022 10:19:06 +0000 (12:19 +0200)]
crypto/ecdh: update ECDH docs and add tests for edge cases
Two edge cases that were mentioned in the docs are actually impossible:
* For NIST curves, ECDH can't fail, since the zero scalar is rejected
by NewPrivateKey, the identity point is rejected by NewPublicKey,
and NIST curves are a prime-order group.
Let's call the inputs to scalar multiplication k and P, and the
order of the group q. If k[P] is the identity, and also q[P] is the
identity by definition, then P's order is a divisor of q-k, because
k[P] + [q-k]P = q[P] = I
P's order is either 1 or q, and can only be a divisor of q-k if it's
1 (so P is the identity), or if k is zero.
* For X25519, PrivateKey.PublicKey can't return the all-zero value,
since no value is equivalent to zero after clamping.
Clamping unsets the lowest three bit, sets the second-to-highest
bit, and unsets the top bit; this means that a scalar equivalent to
zero needs to be a multiple of 8*q, and needs to be between 2**254
and 2**255-1, but 8*p > 2**255-1.
Tests for other exotic edge cases such as non-canonical point encodings,
clamping, points on the twist, and low-order components are covered by
x/crypto/wycheproof.
Bryan C. Mills [Wed, 2 Nov 2022 13:09:24 +0000 (09:09 -0400)]
os/exec: allow open descriptors to be closed during TestPipeLookPathLeak
In https://build.golang.org/log/d2eb315305bf3d513c490e7f85d56e9a016aacd2,
we observe a failure in TestPipeLookPathLeak due to an additional
descriptor (7) that was open at the start of the test being closed while
the test executes.
I haven't dug much into the failure, but it seems plausible to me that the
descriptor may have been opened by libc for some reason, and may have been
closed due to some sort of idle timeout or the completion of a background
initialization routine.
Since the test is looking for a leak, and closing an existing descriptor
does not indicate a leak, let's not fail the test if an existing descriptor
is unexpectedly closed.
Updates #5071.
Change-Id: I03973ddff6592c454cfcc790d6e56accd051dd52
Reviewed-on: https://go-review.googlesource.com/c/go/+/447235 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cherry Mui [Tue, 1 Nov 2022 15:40:29 +0000 (11:40 -0400)]
cmd/compile: use edge weights to decide inlineability in PGO
Currently, with PGO, the inliner uses node weights to decide if a
function is inlineable (with a larger budget). But the actual
inlining is determined by the weight of the call edge. There is a
discrepancy that, if a callee node is hot but the call edge is not,
it would not inlined, and marking the callee inlineable would of
no use.
Instead of using two kinds of weights, we just use the edge
weights to decide inlineability. If a function is the callee of a
hot call edge, its inlineability is determined with a larger
threshold. For a function that exceeds the regular inlining budget,
it is still inlined only when the call edge is hot, as it would
exceed the regular inlining cost for non-hot call sites, even if
it is marked inlineable.
For #55022.
Change-Id: I93fa9919fc6bcbb394e6cfe54ec96a96eede08f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/447015
Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
We don't have a formatter for these files, so check here that
they are in the right form to allow 'cat next/*.txt >go1.X.txt'
at the end of each cycle.
Ian Lance Taylor [Mon, 31 Oct 2022 22:58:06 +0000 (15:58 -0700)]
text/template: correct assignment, not declaration, in range
We were mishandling {{range $i = .}}, treating it as though it were
{{range $i := .}}. That happened to work if $i were the most recently
declared variable, but not otherwise.
Fixes #56490
Change-Id: I222a009d671d86c06a980a54388e05f12101c00b
Reviewed-on: https://go-review.googlesource.com/c/go/+/446795
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Zeke Lu [Thu, 27 Oct 2022 01:43:16 +0000 (01:43 +0000)]
net: store IPv4 returned from cgo resolver as 4-byte slice net.IP
net.IP states that a 16-byte slice can still be an IPv4 address.
But after netip.Addr is introduced, it requires extra care to keep
it as an IPv4 address when converting it to a netip.Addr using
netip.AddrFromSlice.
To address this issue, let's change the cgo resolver to return
4-byte net.IP for IPv4. The change will save us 12 bytes too.
Please note that the go resolver already return IPv4 as 4-byte
slice.
The test TestResolverLookupIP has been modified to cover this
behavior. So no new test is added.
Fixes #53554.
Change-Id: I0dc2a59ad785c0c67a7bc22433105529f055997f
GitHub-Last-Rev: bd7bb2f17bd8e07ea5b39e4a24512ed35d316bb8
GitHub-Pull-Request: golang/go#53638
Reviewed-on: https://go-review.googlesource.com/c/go/+/415580
Auto-Submit: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
cuiweixie [Sat, 22 Oct 2022 14:07:07 +0000 (22:07 +0800)]
regexp: add ErrLarge error
For #56041
Change-Id: I6c98458b5c0d3b3636a53ee04fc97221f3fd8bbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/444817
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: xie cui <523516579@qq.com>
David Chase [Fri, 14 Oct 2022 16:04:52 +0000 (12:04 -0400)]
cmd/compile: renovate GOSSAHASH
Randomized feature enable/disable might be something we use to
help users debug any problems with changed loop variable capture,
and there's another CL that would like to use it to help in
locating places where "fused" multiply add instructions change
program behavior.
This CL:
- adds the ability to include an integer parameter (e.g. line number)
- replumbed the environment variable into a flag to simplify go build cache management
- but added an environment variable to allow flag setting through the environment
- which adds the possibility of switching on a different variable
(if there's one built-in for variable capture, it shouldn't be GOSSAHASH)
- cleaned up the checking code
- adds tests for all the intended behavior
- removes the case for GSHS_LOGFILE; TBD whether we'll need to put that back
or if there is another way.
Change-Id: I8503e1bb3dbc4a743aea696e04411ea7ab884787
Reviewed-on: https://go-review.googlesource.com/c/go/+/443063 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: David Chase <drchase@google.com>
math/big: use Montgomery for z.Exp(x, y, m) even for even m
Montgomery multiplication can be used for Exp mod even m
by splitting it into two steps - Exp mod an odd number and
Exp mod a power of two - and then combining the two results
using the Chinese Remainder Theorem.
For more details, see Ç. K. Koç, “Montgomery Reduction with Even Modulus”,
IEE Proceedings: Computers and Digital Techniques, 141(5) 314-316, September 1994.
http://www.people.vcu.edu/~jwang3/CMSC691/j34monex.pdf
Thanks to Guido Vranken for suggesting that we use a faster algorithm.
Russ Cox [Thu, 27 Oct 2022 02:16:52 +0000 (22:16 -0400)]
cmd/link: remove unnecessary use of sync.Once alongside sync.Mutex
There does not seem to be any point to this sync.Once.
I noticed because I was surveying uses of sync.Once to
understand usage patterns. This seems to be a dreg left over
from some earlier instance of the code.
Mateusz Poliwczak [Tue, 1 Nov 2022 08:53:10 +0000 (08:53 +0000)]
net: support no-reload option for unix go resolver
It adds support for no-reload option, as specified in resolv.conf(5):
no-reload (since glibc 2.26)
Sets RES_NORELOAD in _res.options. This option
disables automatic reloading of a changed
configuration file.
Change-Id: I11182c5829434503f719ed162014f2301e3ba8d4
GitHub-Last-Rev: 7ae44be2d5d5e3242dc39e733f154a64724dbedd
GitHub-Pull-Request: golang/go#56489
Reviewed-on: https://go-review.googlesource.com/c/go/+/446555 Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Ian Lance Taylor [Tue, 1 Nov 2022 19:56:12 +0000 (12:56 -0700)]
net: drop unused _C_ai_addrlen function
Fixes AIX build.
Change-Id: Icbb33896017bbcc488a8baff20e10eb0e14ea4b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/447095 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
The global ListOfHotCallSites set is used to communicate between
CanInline and InlineCalls the set of call sites that InlineCalls may
increase the budget for.
CanInline clears this map on each call, thus assuming that
InlineCalls(x) is called immediately after CanInline(x). This assumption
is false, as CanInline (among other cases) is recursive (CanInline ->
hairyVisitor.doNode -> inlCallee -> CanInline).
When this assumption proves false, we will lose the opportunity to
inline hot calls.
This CL is the least invasive fix for this. ListOfHotCallSites is
actually just a subset of the candHotEdgeMap, with CallSiteInfo.Callee
cleared. candHotEdgeMap doesn't actually need to distinguish based on
Callee, so we can drop callee from candHotEdgeMap as well and just use
that directly [1].
Later CLs should do more work to remove the globals entirely.
For cmd/compile, this inceases the number of PGO inlined functions by
~50% for one set of PGO parameters. I have no evaluated performance
impact.
[1] This is something that we likely want to change in the future.
For #55022.
Change-Id: I57735958d651f6dfa9bd296499841213d20e1706
Reviewed-on: https://go-review.googlesource.com/c/go/+/446755
Auto-Submit: Michael Pratt <mpratt@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Bryan C. Mills [Tue, 1 Nov 2022 13:24:54 +0000 (09:24 -0400)]
runtime: check for ErrWaitDelay in runBuiltTestProg
ErrWaitDelay is not expected to occur in this test, but if it does
it indicates a failure mode very different from the “failed to start”
catchall that we log for other non-ExitError errors.
Bryan C. Mills [Tue, 1 Nov 2022 12:26:58 +0000 (08:26 -0400)]
internal/testenv: adjust timeout calculations in CommandContext
I noticed some test failures in the build dashboard after CL 445597
that made me realize the grace period should be based on the test
timeout, not the Context timeout: if the test itself sets a short
timeout for a command, we still want to give the test process enough
time to consume and log its output.
I also put some more thought into how one might debug a test hang, and
realized that in that case we don't want to set a WaitDelay at all:
instead, we want to leave the processes in their stuck state so that
they can be investigated with tools like `ps` and 'lsof'.
Ian Lance Taylor [Sat, 29 Oct 2022 00:23:58 +0000 (17:23 -0700)]
go/build: ignore files by extension before matching on name
Otherwise given a file like defs_nacl_amd64p32.go.~1~ we will add
"nacl" and "amd64p32" to AllTags. This was causing the
cmd/go/internal/modindex tests to fail on my system, since I had
an old editor backup file lying around.
Change-Id: Ib1c5d835e4871addae6dc78cee07c9839bb880e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/446395 Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
The go1 benchmark suite does a lot of work at package init time, which
makes it take quite a while to run even if you're not running any of
the benchmarks, or if you're only running a subset of them. This leads
to an awkward workaround in dist test to compile but not run the
package, unlike roughly all other packages. It also reduces isolation
between benchmarks by affecting the starting heap size of all
benchmarks.
Fix this by initializing all data required by a benchmark when that
benchmark runs, and keeping it local so it gets freed by the GC and
doesn't leak between benchmarks. Now, none of the benchmarks depend on
global state.
Re-initializing the data on each benchmark run does add overhead to an
actual benchmark run, as each benchmark function is called several
times with different values of b.N. A full run of all benchmarks at
the default -benchtime=1s now takes ~10% longer; higher -benchtimes
would be less. It would be quite difficult to cache this data between
invocations of the same benchmark function without leaking between
different benchmarks and affecting GC overheads, as the testing
package doesn't provide any mechanism for this.
This reduces the time to run the binary with no benchmarks from 1.5
seconds to 10 ms, and also reduces the memory required to do this from
342 MiB to 17 MiB.
To make sure data was not leaking between different benchmarks, I ran
the benchmarks with -shuffle=on. The variance remained low: mostly
under 3%. A few benchmarks had higher variance, but in all cases it
was similar to the variance between this change.
This CL naturally changes the measured performance of several of the
benchmarks because it dramatically changes the heap size and hence GC
overheads. However, going forward the benchmarks should be much better
isolated.
Austin Clements [Tue, 1 Nov 2022 14:00:25 +0000 (10:00 -0400)]
runtime: skip TestArenaCollision on failed reservation
If TestArenaCollision cannot reserve the address range it expects to
reserve, it currently fails somewhat mysteriously. Detect this case
and skip the test. This could lead to test rot if we wind up always
skipping this test, but it's not clear that there's a better answer.
If the test does fail, we now also log what it thinks it reserved so
the failure message is more useful in debugging any issues.
Check for and reject environment variables containing NULs.
The conventions for passing environment variables to subprocesses
cause most or all systems to interpret a NUL as a separator. The
syscall package rejects environment variables containing a NUL
on most systems, but erroniously did not do so on Windows. This
causes an environment variable such as "FOO=a\x00BAR=b" to be
interpreted as "FOO=a", "BAR=b".
Check for and reject NULs in environment variables passed to
syscall.StartProcess on Windows.
Add a redundant check to os/exec as extra insurance.
Than McIntosh [Fri, 28 Oct 2022 17:46:32 +0000 (13:46 -0400)]
cmd/cover: fix buglet causing differences in -m output
Use a slightly different line number pragma when emitting instrumented
code, so as to ensure that we don't get any changes in the
"-gcflags=-m" output for coverage vs non-coverage.
Fixes #56475.
Change-Id: I3079171fdf83c0434ed6ea0ce3eb2797c2280c55
Reviewed-on: https://go-review.googlesource.com/c/go/+/446259
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Than McIntosh [Wed, 26 Oct 2022 20:56:48 +0000 (16:56 -0400)]
cmd/go: revamp and simplify logic in PrepareForCoverageBuild
Change the 'PrepareForCoverageBuild' helper function to provide more
sensible defaults in the case where Go packages are listed on the
command line (e.g. cases such as "go run -cover mumble.go").
With the old implementation, if module mode was enabled, we would only
instrument packages in the main module, meaning that if you did
something like this:
$ ls go.mod
go.mod
$ GOCOVERDATA=/tmp/cov go run -cover testdata/somefile.go
$
no coverage profiles would be generated at all. This is due to the
fact that the pseudo-package created by the Go command internally when
building "somefile.go" is not considered part of the main module.
This patch moves the default to "packages explicitly mentioned on the
command line, plus packages in the main module", which will make more
sense to users passing specific packages and *.go files on the command
line. Examples:
// Here cmd/compile is part the Go standard library + commands
// (which we exclude from instrumentation by default), but since
// 'cmd/compile' is mentioned on the command line, we will instrument
// just that single package (not any of its deps).
$ cd $GOROOT/src
$ go build -o gc.exe -cover cmd/compile
$ GOCOVERDATA=/tmp/cov ./gc.exe ...
...
$
// Here we're running a Go file named on the command line, where
// the pseudo-package for the command line is not part of the
// main module, but it makes sense to instrument it anyhow.
$ cd ~/go/k8s.io/kubernetes
$ GOCOVERDATA=/tmp/cov go run -cover test/typecheck/testdata/bad/bad.go
...
$
This patch also simplifies the logic and improves flow/comments in
in the helper function PrepareForCoverageBuild.
Change-Id: Id8fc8571157dac8c09e44cc73baa05aeba1640ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/445918
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
This patch fixes a typo/bug introduced in CL 441858 where when pattern
matching a coverage counter access we were looking at an assingment
node instead of the assignment LHS, and fixes a similar problem in
atomic counter update pattern matching introduced in CL 444835. In
both of these cases the bug was not caught because the test intended
to lock down the behavior was written incorrectly (wasn't
instrumenting what the test author thought it was instrumenting,
ouch).
Change-Id: I6e6ac3beacf12ef1a817de5527340b639f0bb044
Reviewed-on: https://go-review.googlesource.com/c/go/+/446258 Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
Than McIntosh [Wed, 26 Oct 2022 15:33:28 +0000 (11:33 -0400)]
cmd/{go,cover}: fix for -coverprofile path capture with local pkg
When coverage testing a local package (defined by a relative import
path such as "./foo/bar") the convention when "-coverprofile" is used
has been to capture source files by full pathname, as opposed to
recording the full import path or the invented import path
("command-line-arguments/") created by the go command in the case of
building named Go files. Doing this makes it much easier to use
collected profiles with "go tool -cover -html=<profile>".
The support for this feature/convention wound up being inadvertantly
dropped during the GOEXPERIMENT=coverageredesign implementation; this
patch restores it.
Russ Cox [Fri, 28 Oct 2022 14:54:37 +0000 (10:54 -0400)]
net: use libc (not cgo) for DNS on macOS
Change the macOS implementation to use libc calls.
Using libc calls directly is what we do for all the runtime and os syscalls.
Doing so here as well improves consistency and also makes it possible
to cross-compile (from non-Mac systems) macOS binaries that use the
native name resolver.
Fixes #12524.
Change-Id: I011f4fcc5c50fbb5396e494889765dcbb9342336
Reviewed-on: https://go-review.googlesource.com/c/go/+/446178
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Michael Matloob [Tue, 25 Oct 2022 21:51:12 +0000 (14:51 -0700)]
cmd/go: add move test for goroot
This an end-to-end test that sets GOROOT to a symlink
to the distribution, approximating copying it to a new
location, and checks that packages in the standard library
are not stale, as they would be if paths were embedded
in artifacts.
For #47257
Change-Id: I4ed04df36656ad946a2f6f5ce3194e76e06372e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/445358
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Keith Randall [Tue, 18 Oct 2022 23:07:36 +0000 (16:07 -0700)]
cmd/compile: add cache of sizeable objects so they can be reused
We kind of have this mechanism already, just normalizing it and
using it in a bunch of places. Previously a bunch of places cached
slices only for the duration of a single function compilation. Now
we can reuse slices across a whole compiler run.
Use a sync.Pool of powers-of-two sizes. This lets us use not
too much memory, and avoid holding onto memory we're no longer
using when a GC happens.
There's a few different types we need, so generate the code for it.
Generics would be useful here, but we can't use generics in the
compiler because of bootstrapping.
Change-Id: I6cf37e7b7b2e802882aaa723a0b29770511ccd82
Reviewed-on: https://go-review.googlesource.com/c/go/+/444820
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Keith Randall [Fri, 28 Oct 2022 21:14:24 +0000 (14:14 -0700)]
cmd/compile: add rule for post-decomposed growslice optimization
The recently added rule only works before decomposing slices.
Add a rule that works after decomposing slices.
The reason we need the latter is because although the length may
be a constant, it can be hidden inside a slice that is not constant
(its pointer or capacity might be changing). By applying this
optimization after decomposing slices, we can find more cases
where it applies.
Fixes #56440
Change-Id: I0094e59eee3065ab4d210defdda8227a6e897420
Reviewed-on: https://go-review.googlesource.com/c/go/+/446277
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Michael Pratt [Fri, 28 Oct 2022 21:34:43 +0000 (17:34 -0400)]
cmd/compile/internal/pgo: remove most global state
Since pgo is a new package, it is reasonably straightforward to
encapsulate its state into a non-global object that we pass around,
which will help keep it isolated.
There are no functional changes in this CL, just packaging up the
globals into a new object.
There are two major pieces of cleanup remaining:
1. reflectdata and noder have separate InlineCalls calls for method
wrappers. The Profile is not plumbed there yet, but this is not a
regression as the globals were previously set only right around the
main inlining pass in gc.Main.
2. pgo.ListOfHotCallSites is still global, as it will require more work
to clean up. It is effectively a local variable in InlinePackage,
except that it assumes that InlineCalls is immediately preceded by a
CanInline call for the same function. This is not necessarily true
due to the recursive nature of CanInline. This also means that some
InlineCalls calls may be missing the list of hot callsites right now.
For #55022.
Change-Id: Ic1fe41f73df96861c65f8bfeecff89862b367290
Reviewed-on: https://go-review.googlesource.com/c/go/+/446303 Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
Ian Lance Taylor [Mon, 31 Oct 2022 20:41:59 +0000 (13:41 -0700)]
os: in TestDirFS only check returned path for unexpected string
The test added in CL 446115 was failing on Plan 9, on which the error
returned by a failed open includes the path that failed.
Change-Id: If27222596c3cb0366a030bb49ae41c4c869c3db2
Reviewed-on: https://go-review.googlesource.com/c/go/+/446641 Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Michael Pratt [Fri, 28 Oct 2022 17:52:43 +0000 (13:52 -0400)]
cmd/compile/internal/pgo: remove ConvertLine2Int
Parts of package pgo fetch the line number of a node by parsing the
number out of the string returned from ir.Line().
This is indirect and inefficient, so it should be replaced with a more
direct lookup. It is also potentially buggy: ir.Line uses
ctxt.OutermostPos, i.e., the line number where an inlined node in
inlined. We want ctxt.InnermostPos, because that is the line number used
in pprof profiles that we are matching against (See comments on
OutermostPos and InnermostPos).
I'm not sure whether this was an active, as we use ir.Line before and
during inlining. I think we could see CALL nodes with OutermostPos !=
InnermostPos during midstack inlining, but I am not sure. Regardless,
explicitly using the desired position is clearer.
For #55022.
Change-Id: Ic640761c9e1d01cacbf91f3aaeaf284ad7e38dbd
Reviewed-on: https://go-review.googlesource.com/c/go/+/446302 Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Bryan C. Mills [Wed, 26 Oct 2022 15:44:34 +0000 (11:44 -0400)]
internal/testenv: remove RunWithTimout
For most tests, the test's deadline itself is more appropriate than an
arbitrary timeout layered atop of it (especially once #48157 is
implemented), and testenv.Command already adds cleaner timeout
behavior when a command would run too close to the test's deadline.
That makes RunWithTimeout something of an attractive nuisance. For
now, migrate the two existing uses of it to testenv.CommandContext,
with a shorter timeout implemented using context.WithTimeout.
As a followup, we may want to drop the extra timeouts from these
invocations entirely.
Bryan C. Mills [Wed, 26 Oct 2022 16:18:13 +0000 (12:18 -0400)]
internal/testenv: add CommandContext and Command
This adds a testenv.CommandContext function, with timeout behavior
based on the existing logic in cmd/go.TestScript: namely, the command
is terminated with SIGQUIT (if supported) with an arbitrary grace
period remaining until the test's deadline.
If the test environment does not support executing subprocesses,
CommandContext skips the test.
If the command is terminated due to the timout expiring or the test
fails to wait for the command after starting it, CommandContext marks
the test as failing.
For tests where a shorter timeout is desired (such as for fail-fast
behavior), one may be supplied using context.WithTimeout.
The more concise Command helper is like CommandContext but without
the need to supply an explicit Context.
The test sleeps for 1 millisecond to give the cancellation a moment
to take effect. This is flaky because the request can finish before
the cancellation of the context is seen. It's easy to verify by adding
time.Sleep(2*time.Millisecond)
after https://github.com/golang/go/blob/0a6c4c87404ecb018faf002919e5d5db04c69ee2/src/net/http/transport.go#L2619.
With this modification, the test fails about 5 times out of 10 runs.
The fix is easy. We just need to block the handler of the second
request until this request is cancelled. I have verify that the
updated test can uncover the issue fixed by CL 257818.
Russ Cox [Mon, 31 Oct 2022 19:42:13 +0000 (15:42 -0400)]
encoding/xml: reduce depth limit on wasm
Wasm can't handle the recusion for XML nested to depth 10,000.
Cut it off at 5,000 instead. This fixes TestCVE202228131 on trybots
in certain conditions.
Also disable TestCVE202230633 to fix 'go test -v encoding/xml' on gomotes.
Also rename errExeceededMaxUnmarshalDepth [misspelled and unwieldy]
to errUnmarshalDepth.
Bryan C. Mills [Mon, 31 Oct 2022 15:14:35 +0000 (11:14 -0400)]
cmd/compile/internal/types2: fix tests on js/wasm
The js/wasm builder started failing as of CL 432535 due to needing
'go build' to import standard-library packages that are no longer
installed to GOROOT/pkg. Since js/wasm can't exec subprocesses,
it cannot run 'go build' to generate the export data needed for
these tests.
Archana R [Fri, 28 Oct 2022 07:42:39 +0000 (02:42 -0500)]
internal/bytealg: fix bug in index function for ppc64le/power9
The index function was not handling certain corner cases where there
were two more bytes to be examined in the tail end of the string to
complete the comparison. Fix code to ensure that when the string has
to be shifted two more times the correct bytes are examined.
Also hoisted vsplat to V10 so that all paths use the correct value.
Some comments had incorrect register names and corrected the same.
Added the strings that were failing to strings test for verification.
erifan01 [Tue, 25 Oct 2022 04:08:05 +0000 (12:08 +0800)]
cmd/compile: enable address folding for global symbols of shared library
Address folding is disabled in CL42172, the commit message of which
said that "In shared library, load/store of global is rewritten to
using GOT and temp register, which conflicts with the use of temp
register for assembling large offset.". Actually this doesn't happen
because the sequence of instructions when rewritten to use Got looks
like this:
MOVD $sym, Rx becomes
MOVD sym@GOT, Rx
If there is an offset off, there will be one more instruction:
ADD $off, Rx, Rx
And MOVD sym, Rx becomes
MOVD sym@GOT, REGTMP
MOVx (REGTMP), Ry
If there is a small offset off, it becomes:
MOVD sym@GOT, REGTMP
MOVx (REGTMP)off, Ry
If off is very large, it becomes:
MOVD sym@GOT, REGTMP
MOVD $off, Rt
ADD Rt, REGTMP
MOVx (REGTMP), Ry
We can see that the address can be calculated correctly, and testing
on darwin/arm64 confirms this.
Removing this restriction is beneficial to further optimize the sequence
of "ADRP+ADD+LD/ST" to "ADRP+LD/ST(offset), so this CL removes it.
Change-Id: I0e9f7bc1723e0a027f32cf0ae2c41cd6df49defe
Reviewed-on: https://go-review.googlesource.com/c/go/+/445535 Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Eric Fang <eric.fang@arm.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Guoqi Chen [Tue, 28 Jun 2022 07:46:00 +0000 (15:46 +0800)]
cmd/compile: add missing tail calls flag for linux/loong64
Set the value of the variable tailCall to true and prevent
allocating or clobber the linker register.
Change-Id: I4ec19c67056cb99196911aa7c0054be89ab7eb8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/414954 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org> Reviewed-by: WANG Xuerui <git@xen0n.name>
Riccardo Gerosa [Mon, 24 Oct 2022 22:41:02 +0000 (22:41 +0000)]
math/big: improve performance of Binomial
This change improves the performance of Binomial by implementing an
algorithm that produces smaller intermediate values at each step.
Working with smaller big.Int values has the advantage that fewer allocations
and computations are required for each mathematical operation.
The algorithm used is the Multiplicative Formula, which is a well known
way of calculating the Binomial coefficient and is described at:
https://en.wikipedia.org/wiki/Binomial_coefficient#Multiplicative_formula
https://en.wikipedia.org/wiki/Binomial_coefficient#In_programming_languages
In addition to that, an optimization has been made to remove a
redundant computation of (i+1) on each loop which has a measurable
impact when using big.Int.
Performance improvement measured on an M1 MacBook Pro
running the existing benchmark for Binomial:
name old time/op new time/op delta
Binomial-8 589ns ± 0% 435ns ± 0% -26.05% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
Binomial-8 1.02kB ± 0% 0.08kB ± 0% -92.19% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
Binomial-8 38.0 ± 0% 5.0 ± 0% -86.84% (p=0.000 n=10+10)
Change-Id: I5a830386dd42f062e17af88411dd74fcb110ded9
GitHub-Last-Rev: 6b2fca07de4096accb02f66c313dff47c2303462
GitHub-Pull-Request: golang/go#56339
Reviewed-on: https://go-review.googlesource.com/c/go/+/444315
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Michael Matloob [Wed, 21 Sep 2022 19:51:27 +0000 (15:51 -0400)]
cmd/go: don't install most GOROOT .a files in pkg
Packages in GOROOT that don't use cgo will not be installed in
GOROOT/pkg, and will instead be cached as usual like other Go
packages.
- add a internal/buildinternal package to hold the identities of the
five packages that use cgo
- update dist's test code to do a go build std cmd before checking
staleness on builders. Because most of those packages no longer have
install locations, and have dependencies that don't either, the
packages need to be cached to not be stale.
- fix index_test to import packages with the path "." when preparing
the "want" values to compare the indexed data to. (the module index
matches the behavior of build.ImportDir, which always passes in "."
as the path.
- In both the index and go/build Importers, don't set
PkgObj for GOROOT packages which will no longer have install
targets. PkgTargetRoot will still be set to compute target paths,
which will still be needed in buildmode=shared.
- "downgrade" all install actions that don't have a target to build
actions. (The target should already not be set for packages that
shouldn't be installed).
For #47257
Change-Id: Ia5aee6b3b20b58e028119cf0352a4c4a2f10f6b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/432535
Run-TryBot: Michael Matloob <matloob@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Ian Lance Taylor [Fri, 28 Oct 2022 19:33:49 +0000 (12:33 -0700)]
testing: change Error to Errorf in comment
Fixes #56479
Change-Id: I1d97eb3ea97304e429e178ad05cb9f861b2bce84
Reviewed-on: https://go-review.googlesource.com/c/go/+/446275
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Michael Matloob [Fri, 28 Oct 2022 00:51:04 +0000 (17:51 -0700)]
cmd/go: don't substitute '$WORK' for work directory in -x heredocs
When writing the set of commands excuted for go build -x, $WORK is
substituted for the work directory in all the commnands. But this
includes the cat <<EOF commands used to create a file with the given
contents. While we can expect the shell to substitute $WORK properly,
commands that read input files, such as importcfgs won't do that
substitution.
This is necessary to fix the build_dash_x script test for CL 432535
because it removes .a files from the traditional stdlib install
locations. The test can pass even with importcfg packagefiles in $WORK
because all transitive imports are in the stdlib, and the compiler can
fall back to finding stdlib .a files in their traditional places, but
once they're gone the packagefile paths in $WORK will have paths that
contain the string $WORK, and os.Open will fail to open them for
reading. And since the fallback is gone the test will fail.
For #47257
Change-Id: I5db0066de6ed3ccf97927a78ce0939e3eb14aebe
Reviewed-on: https://go-review.googlesource.com/c/go/+/446116
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>