Previously, when emitting type switches without an explicit "case nil"
clause, we would emit:
if x == nil { goto Lnil }
...
Lnil: goto Ldefault
But we can instead just emit:
if x == nil { goto Ldefault }
Doesn't pass toolstash-check; seems like it causes some harmless
instruction scheduling changes.
Change-Id: Ie233dda26756911e93a08b3db40407ba38694c62
Reviewed-on: https://go-review.googlesource.com/c/go/+/199644
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, escape analysis is able to record at most one dereference
when a parameter leaks to the heap; that is, at call sites, it can't
distinguish between any of these three functions:
func x1(p ****int) { sink = *p }
func x2(p ****int) { sink = **p }
func x3(p ****int) { sink = ***p }
Similarly, it's limited to recording parameter leaks to only the first
4 parameters, and only up to 6 dereferences.
All of these limitations are due to the awkward encoding scheme used
at the moment.
This CL replaces the encoding scheme with a simple [8]uint8 array,
which can handle up to the first 7 parameters, and up to 254
dereferences, which ought to be enough for anyone. And if not, it's
much more easily increased.
Shrinks export data size geometric mean for Kubernetes by 0.07%.
Fixes#33981.
Change-Id: I10a94b9accac9a0c91490e0d6d458316f5ca1e13
Reviewed-on: https://go-review.googlesource.com/c/go/+/197680
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This CL better abstracts away the parameter leak info that was
directly encoded into the uint16 value. Followup CL will rewrite the
implementation.
Passes toolstash-check.
Updates #33981.
Change-Id: I27f81d26f5dd2d85f5b0e5250ca529819a1f11c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/197679
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
On wasm there is a special goroutine that handles asynchronous events.
Blocking this goroutine often causes a deadlock. However, the stack
trace of this goroutine was omitted when printing the deadlock error.
This change adds an exception so the goroutine is not considered as
an internal system goroutine and the stack trace gets printed, which
helps with debugging the deadlock.
Updates #32764
Change-Id: Icc8f5ba3ca5a485d557b7bdd76bf2f1ffb92eb3e
Reviewed-on: https://go-review.googlesource.com/c/go/+/199537
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
CL 170950 had a regression that makes the compiler produce
an invalid wasm binary if the data section is too large.
Loading such a binary gives the following error:
"LinkError: WebAssembly.instantiate(): data segment is out of bounds"
This change fixes the issue by ensuring that the minimum size of the
linear memory is larger than the end of the data section.
Fixes#34395.
Change-Id: I0c8629de7ffd0d85895ad31bf8c9d45fef197a57
Reviewed-on: https://go-review.googlesource.com/c/go/+/199358
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We're allowed to remove a write barrier when both the old
value in memory and the new value we're writing are not heap pointers.
Improve both those checks a little bit.
A pointer is known to not be a heap pointer if it is read from
read-only memory. This sometimes happens for loads of pointers
from string constants in read-only memory.
Do a better job of tracking which parts of memory are known to be
zero. Before we just kept track of a range of offsets in the most
recently allocated object. For code that initializes the new object's
fields in a nonstandard order, that tracking is imprecise. Instead,
keep a bit map of the first 64 words of that object, so we can track
precisely what we know to be zeroed.
The new scheme is only precise up to the first 512 bytes of the object.
After that, we'll use write barriers unnecessarily. Hopefully most
initializers of large objects will use typedmemmove, which does only one
write barrier check for the whole initialization.
Fixes#34723
Update #21561
Change-Id: Idf6e1b7d525042fb67961302d4fc6f941393cac8
Reviewed-on: https://go-review.googlesource.com/c/go/+/199558
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
For commuting ops, check whether the second argument is dead before
checking if the first argument is rematerializeable. Reusing the register
holding a dead value is always best.
Fixes#33580
Change-Id: I7372cfc03d514e6774d2d9cc727a3e6bf6ce2657
Reviewed-on: https://go-review.googlesource.com/c/go/+/199559
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
The current bytes test suit didn't come with endian based test
which causing #34549 can passed the try-bot.
This test will failed when little endian architecture simply using
load and compare uint.
Update #34549
Change-Id: I0973c2cd505ce21c2bed1deeb7d526f1e872118d
Reviewed-on: https://go-review.googlesource.com/c/go/+/198358
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Add test to check that struct type in reflectlite is mirror of reflect.
Note that the test does not check the field types, only check for number
of fields and field name are the same.
Updates #34486
Change-Id: Id5f9b26d35faec97863dd1fe7e5eab37d4913181
Reviewed-on: https://go-review.googlesource.com/c/go/+/199280
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
CL 197938 actually fixes those regression on Darwin as syscalls
are no longer labeled as always blocking and consume a thread.
Fixes#33953Fixes#32326
Change-Id: I82c98516c23cd36f762bc5433d7b71ea8939a0ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/199477
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
CL 191198 updated runtime rtype and mapType without adopting the changes
to reflectlite, causing mismatch between them.
This CL updates those changes to reflectlite.
Fixes#34486
Change-Id: I2bb043673d997f97bb0b12c4ad471474803b2160
Reviewed-on: https://go-review.googlesource.com/c/go/+/197559
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TestScripts/mod_get_svn would stop with the following prompt if the real
user didn't have vcs-test.golang.org in their known_hosts file:
The authenticity of host 'vcs-test.golang.org (35.184.38.56)' can't be established.
ECDSA key fingerprint is SHA256:[...]
Are you sure you want to continue connecting (yes/no/[fingerprint])?
This was bad because it relied on the user's real ssh known_hosts file.
Worse even, if the user didn't expert or notice the prompt, it could
hang a 'go test' run for quite a while.
Work around that by forcing svn to not use ssh at all. Other potentially
better approaches were tried, but none worked on svn 1.12.2 with openssh
8.0p1.
Fixes#33883.
Change-Id: I2f925fa892f2fa53c77d86b0034141162517ee69
Reviewed-on: https://go-review.googlesource.com/c/go/+/199142
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Updates #34388
Change-Id: I327a1c1557c47fa6c113c7a1a507a8e7355f9d1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/199277
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
When translating C types, cache the in-progress type under its parent
names, so that anonymous structs can also be translated for multiple
typedefs, without clashing.
Standalone types are not affected by this change.
Also updated the test for issue 9026 because the C struct name
generation algorithm has changed.
Fixes#31891
Change-Id: I00cc64852a2617ce33da13f74caec886af05b9f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/181857
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
add preceding "+" for a line that is certainly a statement,
and "_" for a line this is certainly not a statement.
Change-Id: I831435dbc2302f25db1320b99d3513fe61fe1fa2
Reviewed-on: https://go-review.googlesource.com/c/go/+/198737
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Two changes, one to cause the back end to not number ITab operations
(these tend to disappear and are also followed by something more robust)
and to explicitly unmark (as statements) all but the first bit of
the code generated to implement a type switch.
Change-Id: I9f7bf7cbf7ccc5d7eda57f7fb080e600eb312eb0
Reviewed-on: https://go-review.googlesource.com/c/go/+/198739
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Calls are code-generated in an alternate path that inherits
its positions from values, not from *SSAGenState. The
default position on *SSAGenState was marked as not-a-statement,
but this was not applied to the value itself, leading to
spurious "is statement" marks in the output (convention:
after code generation in the compiler, everything is either
definitely a statement or definitely not a statement, nothing
is in the undetermined state).
This CL causes a 35 statement regression in ssa/stmtlines_test.
This is down from the earlier 150 because of all the other
CLs preceding this one that deal with the root causes of the
missing lines (repeated lines on nested calls hid missing lines).
This also removes some line repeats from ssa/debug_test.
Change-Id: Ie9a507bd5447e906b35bbd098e3295211df2ae01
Reviewed-on: https://go-review.googlesource.com/c/go/+/188018
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
This was a cause of some statements being lost.
Change-Id: I81c95dcf3df6ed8a03b7578a27f9b21d33b3cf39
Reviewed-on: https://go-review.googlesource.com/c/go/+/198484
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
We replace the existing file with a thin wrapper around its target so
that we don't break anybody's workflow.
Updates #34388
Change-Id: I0d00371c483cb78f4be18fe987df33c79cd40f05
Reviewed-on: https://go-review.googlesource.com/c/go/+/198977
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This reduces the number of missing-statement lines.
Change-Id: Iefa56c2a253220d17d8b53210c8c6af78ee68756
Reviewed-on: https://go-review.googlesource.com/c/go/+/198483
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
This reverts CL 183841.
Fixes#34533
Reason for revert: Introduced a significant performance regression for repos with many incompatible-version tags.
Change-Id: I75d7fd76e6e1a0902b114b00167b38439e0f8221
Reviewed-on: https://go-review.googlesource.com/c/go/+/198699
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This was a cause of some statements being lost.
Change-Id: Ia4805c2dafd7a880d485a678a48427de8930d57e
Reviewed-on: https://go-review.googlesource.com/c/go/+/198482
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
In the local package build process, when -o is pointing to an existing folder, the object
the filename is generated from files listed on the command line like when the -o is
not pointing to a folder instead of using the `importPath` that is going to be `command-line-arguments`
Fixes#34535
Change-Id: I09a7609c17a2ccdd83da32f01247c0ef473dea1e
GitHub-Last-Rev: b3224226a3
GitHub-Pull-Request: golang/go#34562
Reviewed-on: https://go-review.googlesource.com/c/go/+/197544
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Statement markers on rematerializable values were getting lost in
register allocation. This checks for that case (rematerializable
input and using value share line number, but mark is on the input)
and preserves the mark.
When combined with other CLs in this series, this CL reduces the
"nostmt" count (a line appears in the assembly, but no statement
marker) for cmd/go from 413 to 277. The rematerialized input is
usually a LEAQ (on AMD64).
The cause is "complicated"; for example, a NilCheck originally has the
statement mark (a good thing, if the NilCheck remains) but the
NilCheck is removed and the mark floats to a Block end, then to a
SliceMake. The SliceMake decomposes and goes dead without preserving
its marker (its component values are elided in other rewrites and may
target inputs with different line numbers), but before deadcode
removes it from the graph it moves the mark to an input, which at that
time happens to be a LocalAddr. This eventually transforms to a LEAQ.
Change-Id: Iff91fc2a934357fb59ec46ac87b4a9b1057d9160
Reviewed-on: https://go-review.googlesource.com/c/go/+/198480
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
OpPhi nodes tend to disappear or get rearranged,
and cause statement marks to vanish.
Change-Id: I2f5a222903b7fcd0d1a72e8f6d7e156036b23f30
Reviewed-on: https://go-review.googlesource.com/c/go/+/198481
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
According to spec, the hash must be truncated, but crypto/dsa
does not do it. We can't fix it in crypto/dsa, because it would break
verification of previously generated signatures.
In crypto/x509 however, go can't generate DSA certs, only verify them,
so the fix here should be safe.
Fixes#22017
Change-Id: Iee7e20a5d76f45da8901a7ca686063639092949f
GitHub-Last-Rev: 8041cde8d2
GitHub-Pull-Request: golang/go#34630
Reviewed-on: https://go-review.googlesource.com/c/go/+/198138
Reviewed-by: Filippo Valsorda <filippo@golang.org>
'GOFLAGS=-mod=vendor' currently causes 'go get' to always fail unless
the '-mod' flag is explicitly overwritten. Moreover, as of CL 198319
we plan to set -mod=vendor by default if a vendor directory is
present, so all users with vendor directories will be affected — not
just those who set 'GOFLAGS' explicitly.
Similarly, an explicit '-mod=readonly' argument to 'go get' is
currently ignored as a special case, but the fact that it is ignored
(rather than rejected) can be very surprising.
Rather than adding more special cases, we should remove the '-mod'
flag from 'go get' entirely.
Fixes#30345Fixes#32502
Updates #33848
Change-Id: Iecd3233ca3ef580ca3a66bd5e6ee8d86d4cbd8a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/198438
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
CL 188317 introduced a compiler crash during dwarf generation which
was reported as Issue #34520. After CL 188217, the issue appears to be
fixed. Add a testcase to avoid future regressions.
Fixes#34520
Change-Id: I73544a9e9baf8dbfb85c19eb6d202beea05affb6
Reviewed-on: https://go-review.googlesource.com/c/go/+/198546
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Passing ambiguous patterns, ending in `.go`, to `go list` results in them
being interpreted as Go files despite potentially being package references.
This can then result in errors on other package references.
The parsing logic is modified to check for a locally present file
corresponding to any pattern ending in `.go`. If no such file is present
the pattern is considered to be a package reference.
We're also adding a variety of non-regression tests that fail with the
original parsing code but passes after applying the fix.
Fixes#32483Fixes#34653
Change-Id: I073871da0dfc5641a359643f95ac14608fdca09b
GitHub-Last-Rev: 5abc200103
GitHub-Pull-Request: golang/go#34663
Reviewed-on: https://go-review.googlesource.com/c/go/+/198459
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The existing text was hard to parse.
Shorten the sentences and simplify the text.
Change-Id: Ic16f486925090ea303c04e70969e5a4b27a60896
Reviewed-on: https://go-review.googlesource.com/c/go/+/198758
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Nilcheck would move statements from NilCheck values to others that
turned out were already dead, which leads to lost statements. Better
to eliminate the dead code first.
One "error" is removed from test/prove.go because the code is
actually dead, and the additional deadcode pass removes it before
prove can run.
Change-Id: If75926ca1acbb59c7ab9c8ef14d60a02a0a94f8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/198479
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
The earlier code was picking nodes that were "poor choices" and
thus sometimes losing statements altogether.
Change-Id: Ibe5ed800ffbd3c926c0ab1bc10c77d72d3042e45
Reviewed-on: https://go-review.googlesource.com/c/go/+/198478
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Aggregate-making nodes that are later decomposed
are poor choices for statements, because the decomposition
phase turns them into multiple sub-values, some of which may be
dead. Better to look elsewhere for a statement mark.
Change-Id: Ibd9584138ab3d1384548686896a28580a2e43f54
Reviewed-on: https://go-review.googlesource.com/c/go/+/198477
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The previous version used the position of the switch statement,
which makes for potentially jumpy stepping and introduces a large
number of statements repeating the line (tricky for inserting
breaks). It also shared a single OBREAK node and this was not
really a syntax "tree".
This improves both the nostmt test (by 6 lines) and
reduces the total badness score from dwarf-goodness (by about 200).
Change-Id: I1f71b231a26f152bdb6ce9bc8f95828bb222f665
Reviewed-on: https://go-review.googlesource.com/c/go/+/188218
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
1) An empty block is treated as not-a-statement unless its line differs
from at least one of its predecessors (it might make sense to
rearrange branches in predecessors, but that is a different issue).
2) When iterating forward to choose a "good" place for a statement,
actually check that the chosen place is in fact good.
3) Refactor same line and same file into methods on XPos and Pos.
This reduces the failure rate of ssa/stmtlines_test by 7-ish lines.
(And interacts favorably with later debugging CLs.)
Change-Id: Idb7cca7068f6fc9fbfdbe25bc0da15bcfc7b9d4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/188217
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Makes it easier to run
go build -a -gcflags=-d=export std |& grep ^BenchmarkExportSize
and get useful output for feeding into benchstat.
Change-Id: I2b52e8f5ff33b7ccb6c25b18e464513344bd9ad9
Reviewed-on: https://go-review.googlesource.com/c/go/+/198698
Reviewed-by: Robert Griesemer <gri@golang.org>
