Previously, we would recognize &(T{...}) expressions during type
checking, rewrite them into (*T){...}, and then do a lot of extra work
to make sure the user doesn't write (*T){...} themselves and
resynthesizing the OPTRLIT later on.
This CL simply handles &T{...} directly in the straight forward
manner, by changing OADDR directly to OPTRLIT when appropriate.
While here, match go/types's invalid composite literal type error
message.
Passes toolstash-check.
Change-Id: I902b14c7e2cd9fa93e6915dd58272d2352ba38f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/197120
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Now that OpSliceMake is called by runtime.makeslice callers,
prove can see and record the actual length and cap of each
slice being constructed.
This small patch is enough to remove 260 additional bound checks
from cmd+std.
Thanks to Martin Möhrmann for pointing me to CL141822 that
I had missed.
Updates #24660
Change-Id: I14556850f285392051f3f07d13b456b608b64eb9
Reviewed-on: https://go-review.googlesource.com/c/go/+/196784
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This allows for `LD 4(X5), X6' rather than `LD $4, X5, X6'. Similar for other
load and store instructions. It is worth noting that none of these are likely
to be used directly once the MOV pseudo-instructions are implemented.
Updates #27532
Change-Id: Ie043c2dedd2cdaceb258b27976cfb3f74aa1cc1d
Reviewed-on: https://go-review.googlesource.com/c/go/+/196842
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Add support for assembling control transfer instructions.
Based on the riscv-go port.
Updates #27532
Change-Id: I205d3ccd0a48deeaace0f20fca8516f382a83fae
Reviewed-on: https://go-review.googlesource.com/c/go/+/196841
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Add support for assembling AUIPC and LUI instructions.
Based on the riscv-go port.
Updates #27532
Change-Id: I178868b6dcc6fdc6b8527454569a3538ed50723e
Reviewed-on: https://go-review.googlesource.com/c/go/+/196840
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
RFC 7230 is clear about headers with a space before the colon, like
X-Answer : 42
being invalid, but we've been accepting and normalizing them for compatibility
purposes since CL 5690059 in 2012.
On the client side, this is harmless and indeed most browsers behave the same
to this day. On the server side, this becomes a security issue when the
behavior doesn't match that of a reverse proxy sitting in front of the server.
For example, if a WAF accepts them without normalizing them, it might be
possible to bypass its filters, because the Go server would interpret the
header differently. Worse, if the reverse proxy coalesces requests onto a
single HTTP/1.1 connection to a Go server, the understanding of the request
boundaries can get out of sync between them, allowing an attacker to tack an
arbitrary method and path onto a request by other clients, including
authentication headers unknown to the attacker.
This was recently presented at multiple security conferences:
https://portswigger.net/blog/http-desync-attacks-request-smuggling-reborn
net/http servers already reject header keys with invalid characters.
Simply stop normalizing extra spaces in net/textproto, let it return them
unchanged like it does for other invalid headers, and let net/http enforce
RFC 7230, which is HTTP specific. This loses us normalization on the client
side, but there's no right answer on the client side anyway, and hiding the
issue sounds worse than letting the application decide.
Fixes CVE-2019-16276
Fixes#34540
Change-Id: I6d272de827e0870da85d93df770d6a0e161bbcf1
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/549719
Reviewed-by: Brad Fitzpatrick <bradfitz@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/197503
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
If the parent context passed to WithCancel or WithTimeout
is a known context implementation (one created by this package),
we attach the child to the parent by editing data structures directly;
otherwise, for unknown parent implementations, we make a
goroutine that watches for the parent to finish and propagates
the cancellation.
A common problem with this scheme, before this CL, is that
users who write custom context implementations to manage
their value sets cause WithCancel/WithTimeout to start
goroutines that would have not been started before.
This CL changes the way we map a parent context back to the
underlying data structure. Instead of walking up through
known context implementations to reach the *cancelCtx,
we look up parent.Value(&cancelCtxKey) to return the
innermost *cancelCtx, which we use if it matches parent.Done().
This way, a custom context implementation wrapping a
*cancelCtx but not changing Done-ness (and not refusing
to return wrapped keys) will not require a goroutine anymore
in WithCancel/WithTimeout.
For #28728.
Change-Id: Idba2f435c81b19fe38d0dbf308458ca87c7381e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/196521
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
There are currently two edges in the lock cycle graph caused by
scavenge.lock: with sched.lock and mheap_.lock. These edges appear
because of the call to ready() and stack growths respectively.
Furthermore, there's already an invariant in the code wherein
mheap_.lock must be acquired before scavenge.lock, hence the cycle.
The fix to this is to bring scavenge.lock higher in the lock cycle
graph, such that sched.lock and mheap_.lock are only acquired once
scavenge.lock is already held.
To faciliate this change, we move scavenger waking outside of
gcSetTriggerRatio such that it doesn't have to happen with the heap
locked. Furthermore, we check scavenge generation numbers with the heap
locked by using gopark instead of goparkunlock, and specify a function
which aborts the park should there be any skew in generation count.
Fixes#34047.
Change-Id: I3519119214bac66375e2b1262b36ce376c820d12
Reviewed-on: https://go-review.googlesource.com/c/go/+/191977
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The macOS filesystem seems to have gotten significantly flakier as of
macOS 10.14, so this causes frequently flakes in the 10.14 builders.
We have no reason to believe that it will be fixed any time soon, so
rather than trying to detect the specific macOS version, we'll apply
the same workarounds that we use on Windows: classifying (and
retrying) the errors known to indicate flakiness and relaxing the
success criteria for renameio.TestConcurrentReadsAndWrites.
Fixes#33041
Change-Id: I74d8c15677951d7a0df0d4ebf6ea03e43eebddf9
Reviewed-on: https://go-review.googlesource.com/c/go/+/197517
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Prior to this CL conditional branches on s390x always used an
extended mnemonic such as BNE, BLT and so on to represent branch
instructions with different condition code masks. This CL adds
support for numeric condition code masks to the s390x SSA backend
so that we can encode the condition under which a Block's
successor is chosen as a field in that Block rather than in its
type.
This change will be useful as we come to add support for combined
compare-and-branch instructions. Rather than trying to add extended
mnemonics for every possible combination of mask and compare-and-
branch instruction we can instead use a single mnemonic for each
instruction.
Change-Id: Idb7458f187b50906877d683695c291dff5279553
Reviewed-on: https://go-review.googlesource.com/c/go/+/197178
Reviewed-by: Keith Randall <khr@golang.org>
This switches the linker over to using the new debug_lines data
generated in the compiler.
Change-Id: If8362d6fcea7db60aaebab670ed6f702ab1c4908
Reviewed-on: https://go-review.googlesource.com/c/go/+/191968
Run-TryBot: Jeremy Faller <jeremy@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Replace usage of yyerror with yyerrorl in checkdefergo and copytype in
typecheck.go.
All covered error messages already appear in the tests and the yyerror
replacement did not lead to any tests failing.
Passes toolstash-check
Updates #19683
Change-Id: I735e83bcda7ddc6a14afb22e50200bcbb9192fc4
Reviewed-on: https://go-review.googlesource.com/c/go/+/69910
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 192518 was a minimal simplification to get sendfile
on Windows to work with chunked files, but as I had mentioned,
I would add even more improvements.
This CL improves it by:
* If the reader is not an *io.LimitedReader, since the underlying
reader is anyways an *os.File, we fallback and stat that
file to determine the file size and then also invoke the chunked
sendFile on the underlying reader. This issue existed even
before the prior CL.
* Extracting the chunked TransmitFile logic and moving it directly
into internal/poll.SendFile.
Thus if the callers of net.sendFile don't use *io.LimitedReader,
but have a huge file (>2GiB), we can still invoke the chunked
internal/poll.SendFile on it directly.
The test case is not included in this patch as it requires
creating a 3GiB file, but that if anyone wants to view it, they
can find it at
https://go-review.googlesource.com/c/go/+/194218/13/src/net/sendfile_windows_test.go
Updates #33193.
Change-Id: I97a67c712d558c84ced716d8df98b040cd7ed7f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/194218
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Change the comment to make more conformable to the function implementation.
Change-Id: I8461e2f09824c50e16223a27d0f61070f04bd21b
GitHub-Last-Rev: c25a8493d3
GitHub-Pull-Request: golang/go#27404
Reviewed-on: https://go-review.googlesource.com/c/go/+/132477
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Add a bunch of extra tests and benchmarks for defer, in preparation for new
low-cost (open-coded) implementation of defers (see #34481),
- New file defer_test.go that tests a bunch more unusual defer scenarios,
including things that might have problems for open-coded defers.
- Additions to callers_test.go actually verifying what the stack trace looks like
for various panic or panic-recover scenarios.
- Additions to crash_test.go testing several more crash scenarios involving
recursive panics.
- New benchmark in runtime_test.go measuring speed of panic-recover
- New CGo benchmark in cgo_test.go calling from Go to C back to Go that
shows defer overhead
Updates #34481
Change-Id: I423523f3e05fc0229d4277dd00073289a5526188
Reviewed-on: https://go-review.googlesource.com/c/go/+/197017
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The branch-relative-on-condition (BRC) instruction allows us to use
an immediate to specify under what conditions the branch is taken.
For example, `BRC $7, L1` is equivalent to `BNE L1`. It is sometimes
useful to specify branches in this way when either we don't have
an extended mnemonic for a particular mask value or we want to
generate the condition code mask programmatically.
The new load-on-condition (LOCR and LOCGR) and compare-and-branch
(CRJ, CGRJ, CLRJ, CLGRJ, CIJ, CGIJ, CLIJ and CLGIJ) instructions
provide the same flexibility for conditional loads and combined
compare and branch instructions.
Change-Id: Ic6f5d399b0157e278b39bd3645f4ee0f4df8e5fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/196558
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Inline scavenging causes significant performance regressions in tail
latency for k8s and has relatively little benefit for RSS footprint.
We disabled inline scavenging in Go 1.12.5 (CL 174102) as well, but
we thought other changes in Go 1.13 had mitigated the issues with
inline scavenging. Apparently we were wrong.
This CL switches back to only doing foreground scavenging on heap
growth, rather than doing it when allocation tries to allocate from
scavenged space.
Fixes#32828.
Change-Id: I1f5df44046091f0b4f89fec73c2cde98bf9448cb
Reviewed-on: https://go-review.googlesource.com/c/go/+/183857
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Currently, we map and grow the heap a whole arena (64MB) at a time.
Unfortunately, in order to fix#32828, we need to switch from
scavenging inline with allocation back to scavenging on heap growth,
but heap-growth scavenging happens in large jumps because we grow the
heap in large jumps.
In order to prepare for better heap-growth scavenging, this CL
separates mapping more space for the heap from actually "growing" it
(tracking the new space with spans). Instead, growing the heap keeps
track of the "current arena" it's growing into. It track that with new
spans as needed, and only maps more arena space when the current arena
is inadequate. The effect to the user is the same, but this will let
us scavenge on much smaller increments of heap growth.
There are two slightly subtleties to this change:
1. If an allocation requires mapping a new arena and that new arena
isn't contiguous with the current arena, we don't want to lose the
unused space in the current arena, so we have to immediately track
that with a span.
2. The mapped space must be accounted as released and idle, even
though it isn't actually tracked in a span.
For #32828, since this makes heap-growth scavenging far more
effective, especially at small heap sizes. For example, this change is
necessary for TestPhysicalMemoryUtilization to pass once we remove
inline scavenging.
Change-Id: I300e74a0534062467e4ce91cdc3508e5ef9aa73a
Reviewed-on: https://go-review.googlesource.com/c/go/+/189957
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
This change makes it so that the scavenge goal is defined primarily in
terms of heap_inuse at the end of the last GC rather than next_gc. The
reason behind this change is that next_gc doesn't take into account
fragmentation, and we can fall into situation where the scavenger thinks
it should have work to do but there's no free and unscavenged memory
available.
In order to ensure the scavenge goal still tracks next_gc, we multiply
heap_inuse by the ratio between the current heap goal and the last heap
goal, which describes whether the heap is growing or shrinking, and by
how much.
Finally, this change updates the documentation for scavenging and
elaborates on why the scavenge goal is defined the way it is.
Fixes#34048.
Updates #32828.
Change-Id: I8deaf87620b5dc12a40ab8a90bf27932868610da
Reviewed-on: https://go-review.googlesource.com/c/go/+/193040
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
This code is not currently compiling, the asm vet checks fail. When running race.bash on ppc64le, I get:
runtime/race_ppc64le.s:104:1: [ppc64le] RaceReadRange: wrong argument size 24; expected $...-16
runtime/race_ppc64le.s:514:1: [ppc64le] racecallbackthunk: unknown variable cmd; offset 0 is arg+0(FP)
runtime/race_ppc64le.s:515:1: [ppc64le] racecallbackthunk: unknown variable ctx
I'm also not sure why it ever worked; it looks like it is writing
the arguments to racecallback in the wrong place (the race detector
itself probably still works, it would just have trouble symbolizing
any resulting race report).
At a meta-level, we should really add a ppc64le/race builder.
Otherwise this code will rot, as evidenced by the rot this CL fixes :)
Update #33309
Change-Id: I3b49c2442aa78538fbb631a143a757389a1368fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/197337
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When the -json flag is passed to go mod download,
the sumdb error is embedded in the json Error field.
Other errors for the same command behave this way as
well such as module not found. The fix is done by changing
base.Fatalf into proper error returns.
Fixes#34485
Change-Id: I2727a5c70c7ab03988cad8661894d0f8ec71a768
Reviewed-on: https://go-review.googlesource.com/c/go/+/197062
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This CL implements several optimizations for the escape analysis flow
graph:
1. Instead of recognizing heapLoc specially within Escape.outlives,
set heapLoc.escapes = true and recognize any location with escapes
set. This allows us to skip adding edges from the heap to escaped
variables in two cases:
1a. In newLoc, if the location is for a variable or allocation too
large to fit on the stack.
1b. During walkOne, if we discover that an object's address flows
somewhere that naturally outlives it.
2. When recording edges in Escape.flow, if x escapes and we're adding
an edge like "x = &y", we can simply mark that y escapes too.
3. During walkOne, if we reach a location that's marked as escaping,
we can skip visiting it again: we've either already walked from it, or
it's in queue to be walked from again.
On average, reduces the number of visited locations by 15%. Reduces
time spent in escape analysis for particularly hairy packages like
runtime and gc by about 8%. Reduces escape.go's TODO count by 22%.
Passes toolstash-check.
Change-Id: Iaf86a29d76044e4b4c8ab581b916ef5bb5df4437
Reviewed-on: https://go-review.googlesource.com/c/go/+/196811
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The old escape analysis code used to repeatedly walk the entire flow
graph until it reached a fixed point. With escape.go, I wanted to
avoid this if possible, so I structured the walking code with two
constraints:
1. Always walk from the heap location last.
2. If an object escapes, ensure it has flow edge to the heap location.
This works, but it precludes some graph construction
optimizations. E.g., if there's an assignment "heap = &x", then we can
immediately tell that 'x' escapes without needing to visit it during
the graph walk. Similarly, if there's a later assignment "x = &y", we
could immediately tell that 'y' escapes too. However, the natural way
to implement this optimization ends up violating the constraints
above.
Further, the constraints above don't guarantee that the 'transient'
flag is handled correctly. Today I think that's handled correctly
because of the order that locations happen to be constructed and
visited based on the AST, but I've felt uneasy about it for a little
while.
This CL changes walkAll to use a proper work queue (technically a work
stack) to track locations that need to be visited, and allows walkOne
to request that a location be re-visited.
Passes toolstash-check.
Change-Id: Iaa6f4d3fe4719c04d67009fb9a2a3e4930b3d7c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/196958
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
n.SetLikely(false) is probably mean to indicate that the branch is
"unlikely", but it has the real effect of not marking branch as likely.
So invert the test condition, we can use more meaningful n.SetLikely(true).
Before:
if l2 < 0 {
panicmakeslicelen()
}
After:
if l2 >= 0 {
} else {
panicmakeslicelen
}
Fixes#32486
Change-Id: I156fdba1f9a5d554a178c8903f1a391ed304199d
Reviewed-on: https://go-review.googlesource.com/c/go/+/195197
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
In CL 188317, we generate the debug_lines in the compiler, and created a
new symbol to hold the line table. Here we modify the object file format
to output the file table.
Change-Id: Ibee192e80b86ff6af36467a0b1c26ee747dfee37
Reviewed-on: https://go-review.googlesource.com/c/go/+/191167
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, when we create an OPTRLIT node, it defaults to the
OCOMPLIT's final element's position. But it improves error messages to
use the OCOMPLIT's own position instead.
Change-Id: Ibb031f543c7248d88d99fd0737685e01d86e2500
Reviewed-on: https://go-review.googlesource.com/c/go/+/197119
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This is better handled by tools like cmd/gofmt, which can
automatically rewrite the source code and already supports a syntactic
version of this simplification. (go/types can be used if
type-sensitive simplification is actually necessary.)
Change-Id: I51332a8f3ff4ab3087bc6b43a491c6d92b717228
Reviewed-on: https://go-review.googlesource.com/c/go/+/197118
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This commit just adds a regress test for a few of the important corner
cases that I identified in #27557, which turn out to not be tested
anywhere.
While here, annotate a few of the existing test cases where we could
improve escape analysis.
Updates #27557.
Change-Id: Ie57792a538f7899bb17915485fabc86100f469a3
Reviewed-on: https://go-review.googlesource.com/c/go/+/197137
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
When using a '.' constant literal as a reflect.Value variadic argument,
idealConstant would incorrectly result in a float64. This is because
rune literals can be represented as a float64, and contain a period,
which tricked the logic into thinking the literal must have been a
floating point number.
This also happened with other characters that can be part of a floating
point number, such as 'e' or 'P'.
To fix these edge cases, exit the case sooner if the literal was a rune,
since that should always go to the int case instead.
Finally, add test cases that verify that they behave properly. These
would error before, since eq would receive a mix of int and float64,
which aren't comparable.
Fixes#34483.
Change-Id: Icfcb7803bfa0cf317a1d1adacacad3d69a57eb42
Reviewed-on: https://go-review.googlesource.com/c/go/+/196808
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tom Payne <tom@airmap.com>
Reviewed-by: Rob Pike <r@golang.org>
Extends the built-in eq function to support all Go
comparable types.
Fixes#33740
Change-Id: I522310e313e251c4dc6a013d33d7c2034fe2ec8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/193837
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Add support for assembling various single-precision and double-precision
floating point instructions.
Based on the riscv-go port.
Updates #27532
Change-Id: Iac1aec9b03bb6cbf116b229daeef944d4df550fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/196839
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 192937 introduced some changes which weren't properly gofmt'ed. Do so
now.
Change-Id: I2d2d57ea8a79fb41bc4ca59fa23f12198d615fd8
Reviewed-on: https://go-review.googlesource.com/c/go/+/196812
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In CL 192980, I tend to think that canSSAType can be used as replacement
for isfat. It is not the truth as @khr points me out that isfat has very
different purpose.
So this CL adds documentation for isfat, also remove outdated TODO.
Change-Id: I15954d638759bd9f6b28a6aa04c1a51129d9ae7d
Reviewed-on: https://go-review.googlesource.com/c/go/+/196499
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is mostly a copy-paste jobs from the linker to generate the debug
information in the compiler instead of the linker. The new data is
inserted into the debug line numbers symbol defined in CL 188238.
Generating the debug information BEFORE deadcode results in one subtle
difference, and that is that the state machine needs to be reset at the
end of every function's debug line table. The reasoning is that
generating the table AFTER dead code allows the producer and consumer of
the table to agree on the state of the state machine, and since these
blocks will (eventually) be concatenated in the linker, we don't KNOW
the state of the state machine unless we reset it. So,
generateDebugLinesSymbol resets the state machine at the end of every
function.
Right now, we don't do anything with this line information, or the file
table -- we just populate the symbols.
Change-Id: If9103eda6cc5f1f7a11e7e1a97184a060a4ad7fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/188317
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
As we move the debug_line generation into the compiler, we need to
upgrade the notion of compilationUnit to not just be on a per package
basis. That won't be the case as it will be impossible for all
compilationUnits to have the same set of files names used to build the
debug_lines table. (For example, assembled files in a package don't know
about any files but themselves, so the debug_lines table could only
reference themseves. As such, we need to break the 1:1 relationship
between compUnit and package.)
Change-Id: I2e517bb6c01de0115bbf777af828a2fe59c09ce8
Reviewed-on: https://go-review.googlesource.com/c/go/+/189618
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
In CL 173017, I changed the package-to-module query logic to query all
possible module paths in parallel in order to reduce latency. (For
long package paths, most such paths will not exist and will fail with
little overhead.)
The module resolution algorithm treats various kinds of non-existence
as “soft errors”, to be reported only if package resolution fails, but
treats any remaining errors as hard errors that should fail the query.
Unfortunately, that interacted badly with the +incompatible version
validation added in CL 181881, causing a regression in the 'direct'
fetch path for modules using the “major branch” layout¹ with a post-v1
version on the repository's default branch. Because we did not
interpret a mismatched module path as “no such module”, a go.mod file
specifying the path 'example.com/foo/v2' would cause the search for
module 'example.com/foo' to error out. (That regression was not caught
ahead of time due to a lack of test coverage for 'go get' on a package
within a /vN module.)
The promotion of hard errors during parallel search also made the 'go'
command less tolerant of servers that advertise 'go-import' tags for
nonexistent repositories. CL 194561 mitigated that problem for HTTP
servers that return code 404 or 410 for a nonexistent repository, but
unfortunately a few servers in common use (notably GitLab and
pre-1.9.3 releases of Gitea) do not.
This change mitigates both of those failure modes by ignoring
“miscellaneous” errors from shorter module paths if the requested
package pattern was successfully matched against a module with a
longer path.
¹https://research.swtch.com/vgo-module#from_repository_to_modules
Updates #34383
Updates #34094
Change-Id: If37dc422e973eba13f3a3aeb68bc7b96e2d7f73d
Reviewed-on: https://go-review.googlesource.com/c/go/+/197059
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
On modern 64bit CPUs a SHR, SHL or AND instruction take 1 cycle to execute.
A pair of shifts that operate on the same register will take 2 cycles
and needs to wait for the input register value to be available.
Large constants used to mask the high bits of a register with an AND
instruction can not be encoded as an immediate in the AND instruction
on amd64 and therefore need to be loaded into a register with a MOV
instruction.
However that MOV instruction is not dependent on the output register and
on many CPUs does not compete with the AND or shift instructions for
execution ports.
Using a pair of shifts to mask high bits instead of an AND to mask high
bits of a register has a shorter encoding and uses one less general
purpose register but is slower due to taking one clock cycle longer
if there is no register pressure that would make the AND variant need to
generate a spill.
For example the instructions emitted for (x & 1 << 63) before this CL are:
48c1ea3f SHRQ $0x3f, DX
48c1e23f SHLQ $0x3f, DX
after this CL the instructions are the same as GCC and LLVM use:
48b80000000000000080 MOVQ $0x8000000000000000, AX
4821d0 ANDQ DX, AX
Some platforms such as arm64 already have SSA optimization rules to fuse
two shift instructions back into an AND.
Removing the general rule to rewrite AND to SHR+SHL speeds up this benchmark:
var GlobalU uint
func BenchmarkAndHighBits(b *testing.B) {
x := uint(0)
for i := 0; i < b.N; i++ {
x &= 1 << 63
}
GlobalU = x
}
amd64/darwin on Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz:
name old time/op new time/op delta
AndHighBits-4 0.61ns ± 6% 0.42ns ± 6% -31.42% (p=0.000 n=25+25):
'go run run.go -all_codegen -v codegen' passes with following adjustments:
ARM64: The BFXIL pattern ((x << lc) >> rc | y & ac) needed adjustment
since ORshiftRL generation fusing '>> rc' and '|' interferes
with matching ((x << lc) >> rc) to generate UBFX. Previously
ORshiftLL was created first using the shifts generated for (y & ac).
S390X: Add rules for abs and copysign to match use of AND instead of SHIFTs.
Updates #33826
Updates #32781
Change-Id: I5a59f6239660d53c029cd22dfb44ddf39f93a56c
Reviewed-on: https://go-review.googlesource.com/c/go/+/196810
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When 'git fetch' is passed the '--unshallow' flag, it assumes that the
local and remote refs are equal.¹ However, we were fetching an
expanded set of refs explicitly in the same command, violating that
assumption.
Now we first expand the set of refs, then unshallow the repo in a
separate fetch. Empirically, this seems to work, whereas the opposite
order does not.
¹4c86140027/transport.c (L1303-L1309)Fixes#34266
Change-Id: Ie97eb7c1223f944003a1e31d0ec9e69aad0efc0d
Reviewed-on: https://go-review.googlesource.com/c/go/+/196961
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Function sizes are computed to determine whether a function
can be kept on one line or should be split to several lines. Part of the
computation is the function header from the FUNC token and until the
opening { token.
Prior to this change, the function header size used distance from the
original source position of the current token, which led to issues when
the source between FUNC and the original source position was rewritten
(such as whitespace being collapsed). Now we take the current output
position into account, so that header size represents the reformatted
source rather than the original source.
The following files in the Go repository are reformatted with this
change:
* strings/strings_test.go
* cmd/compile/internal/gc/fmt.go
In both cases the reformatting is minor and seems to be correct given
the heuristic to single-line functions longer than 100 columns to
multiple lines.
Fixes#28082
Change-Id: Ib737f6933e09b79e83715211421d5262b366ec93
Reviewed-on: https://go-review.googlesource.com/c/go/+/188818
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
