This CL moves fixVariadicCall from mid-Walk of function calls to
early-Order, in preparation for moving it even earlier in the future.
Notably, rewriting variadic calls this early introduces two
compilation output changes:
1. Previously, Order visited the ODDDARG before the rest of the
arguments list, whereas the natural time to visit it is at the end of
the list (as we visit arguments left-to-right, and the ... argument is
the rightmost one). Changing this ordering permutes the autotmp
allocation order, which in turn permutes autotmp naming and stack
offsets.
2. Previously, Walk separately walked all of the variadic arguments
before walking the entire slice literal, whereas the more natural
thing to do is just walk the entire slice literal. This triggers
slightly different code paths for composite literal construction in
some cases.
Neither of these have semantic impact. They simply mean we're now
compiling f(a,b,c) the same way as we were already compiling
f([]T{a,b,c}...).
Change-Id: I40ccc5725697a116370111ebe746b2639562fe87
Reviewed-on: https://go-review.googlesource.com/c/go/+/229601
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Triggers a handful of times in std+cmd.
Change-Id: I9bb8ce9a5f8bae2547cb61157cd8f256e1b63e76
Reviewed-on: https://go-review.googlesource.com/c/go/+/229602
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Use a separate directory for TestBuildFortvOS test files.
Remove a bad comment in TestTrampoline.
Change-Id: I2dc07ae575ec3f73fb7cea26743094b11a41b464
Reviewed-on: https://go-review.googlesource.com/c/go/+/229619
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Allow go generate to process packages that contain invalid code. Ignore
errors when loading the package, but process only files which have a
valid package clause. Set $GOPACKAGE individually for each file, based
on the package clause.
Add test script for go generate and invalid packages.
Fixes#36422
Change-Id: I91ea088346a1548ccd6678b4595a527b948331ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/229097
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Recommend use of GOINSECURE over -insecure flang and clarify that GOINSECURE
environment variable does not also imply GONOSUMDB.
Updates #37519 by adding documentation as discussed.
Change-Id: Ia8ab6b3ed1aa559343b72e4ca76c372ee6bf1941
GitHub-Last-Rev: 8d86991f0c
GitHub-Pull-Request: golang/go#38572
Reviewed-on: https://go-review.googlesource.com/c/go/+/229223
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In mid-Walk, we rewrite calls to variadic functions to use explicit
slice literals; e.g., rewriting f(a,b,c) into f([]T{a,b,c}...).
However, it would be useful to do that rewrite much earlier in the
compiler, so that other compiler passes can be simplified.
This CL refactors the rewrite logic into a new fixVariadicCall
function, which subsequent CLs can more easily move into earlier
compiler passes.
Passes toolstash-check -race.
Change-Id: I408e655f2d3aa00446a2e6accf8765abc3b16a8a
Reviewed-on: https://go-review.googlesource.com/c/go/+/229486
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
ioutil.TempDir doesn't like path separators in its pattern. Modify
(*common).TempDir to replace path separators with underscores before
using the test name as a pattern for ioutil.TempDir.
Fixes#38465.
Change-Id: I9e8ae48b99648b2bf9f561762e845165aff01972
Reviewed-on: https://go-review.googlesource.com/c/go/+/229399
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
unsafe.Pointer safety rule #4 says "The compiler handles a Pointer
converted to a uintptr in the argument list of a call". Within escape
analysis, we've always required this be a single conversion
unsafe.Pointer->uintptr conversion, but the corresponding logic in
order is somewhat laxer, allowing arbitrary chains of OCONVNOPs from
unsafe.Pointer to uintptr.
This CL changes order to be stricter to match escape analysis.
Passes toolstash-check.
Change-Id: Iadd210d2123accb2020f5728ea2a47814f703352
Reviewed-on: https://go-review.googlesource.com/c/go/+/229578
Reviewed-by: Ian Lance Taylor <iant@golang.org>
With CL 228782, we've removed file I/O, but we're growing the memory too
much. This change will periodically flush the heap area to the mmapped
area (if possible).
Change-Id: I1622c738ee5a1a6d02bff5abb0a5751caf8095c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/229439
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Do not cancel rows during test. Only cancel the Tx.
Correct the referenced issue number on the test.
Fixes#38597
Change-Id: I0e8ba1bf2a8ba638d121c9c6938501fec1d5e961
Reviewed-on: https://go-review.googlesource.com/c/go/+/229478
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
golang.org/cl/147598 added the support for delta computation for mutex
and block profiles. In fact, this delta computation makes sense for
other types of profiles.
For example, /debug/pprof/allocs?seconds=x will provide how much allocation
was made during the specified period. /debug/pprof/goroutine?seconds=x will
provide the changes in the list of goroutines. This also makes sense for
custom profiles.
Update #23401
Update google/pprof#526
Change-Id: I45e9073eb001ea5b3f3d16e5a57f635193610656
Reviewed-on: https://go-review.googlesource.com/c/go/+/229537
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
In some slow environment, the goroutine for mutexHog2 may not run
within 1secs. So, try with increasing seconds parameters,
and declare failure if it still fails with the longest duration
parameter (32sec).
Also, relax the test condition - previously we expected the
profile's duration is within 0.5~2sec. But obviously, in some
slow environment, that's not even guaranteed. Just check we get
non-zero duration in the result.
Update #38544
Change-Id: Ia9b0d51429a2093e6c9eb92cf463ff6952ef3e10
Reviewed-on: https://go-review.googlesource.com/c/go/+/229498
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This CL optimizes code that uses a carry from a function such as
bits.Add64 as the condition in an if statement. For example:
x, c := bits.Add64(a, b, 0)
if c != 0 {
panic("overflow")
}
Rather than converting the carry into a 0 or a 1 value and using
that as an input to a comparison instruction the carry flag is now
used as the input to a conditional branch directly. This typically
removes an ADD LOGICAL WITH CARRY instruction when user code is
doing overflow detection and is closer to the code that a user
would expect to generate.
Change-Id: I950431270955ab72f1b5c6db873b6abe769be0da
Reviewed-on: https://go-review.googlesource.com/c/go/+/219757
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The code that runs as a part of loadlibfull converts the linker's
outer/sub state and sets the sym.Symbol AttrSubSymbol if a symbol has
both A) an outer sym, and B) is listed as a sub-symbol by some other
symbol.
Make sure that we have the same logic in the original loader method,
since we need to use it as part of dodata() prior to loadlibfull.
Change-Id: I200adab741d778a6ba821419e8ea131ad19375bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/229440
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Change the timing for preprocessing of integer/floating point constant
symbols so that we populate them with content at an earlier stage.
This is needed to allow them can be picked up by the loader-API
version of dodata().
Change-Id: Icf09f4f4b318b4f77e11d4a0f0a9cbecd76a1d6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/229438
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
This covers most of the lowering rules.
Passes
GOARCH=mips gotip build -toolexec 'toolstash -cmp' -a std
GOARCH=mipsle gotip build -toolexec 'toolstash -cmp' -a std
Change-Id: I9d00aaebecb36622e3bdaf556e5a9377670bf86b
Reviewed-on: https://go-review.googlesource.com/c/go/+/229102
Reviewed-by: Keith Randall <khr@golang.org>
Create a new version of the GCProg type + methods that use loader APIs
instead of sym.Symbol.
This code isn't actually used just yet, but will be needed once the
wavefront reaches dodata() and we need to convert that phase.
Change-Id: I087521832015818204fe5c2ac99c7bd3f61b2bf0
Reviewed-on: https://go-review.googlesource.com/c/go/+/229037
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Previously the connection pool would only count connections
expired in the background connectionCleaner goroutine towards the
MaxLifetimeClosed stat.
This change increments the stat correctly when checking for
expiry in when acquiring and releasing a connection.
Fixes#38058
Change-Id: Id707ddd40a42a4c38658d5f2931da131647d6c29
GitHub-Last-Rev: 0f205ede43
GitHub-Pull-Request: golang/go#38263
Reviewed-on: https://go-review.googlesource.com/c/go/+/227278
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Goroutines are directly associated with labels. It's relatively easy to
plumb those through without creating goroutine-locals in the wild.
This is accomplished by splitting out most of the code from the public
`runtime.GoroutineProfile` into a new unexported
`runtime.goroutineProfileWithLabels`, which then has a thin wrapper
linked into the `runtime/pprof` package as
`runtime_goroutineProfileWithLabels`. (mirroring the way labels get
associated with the `g` for a goroutine in the first place)
Per-#6104, OS-thread creation profiles are a bit useless, as `M`s tend
to be created be created by a background goroutine. As such, I decided
not to add support for capturing the labels at `M`-creation-time, since
the stack-traces seem to always come out `nil` for my simple test
binaries.
This change currently provides labels for debug=0 and debug=1, as
debug=2 is currently entirely generated by the runtime package and I
don't see a clean way of getting the `labelMap` type handled properly
within the `runtime` package.
Update the comment added in cl/131275 to mention goroutine support for
labels.
Updates #23458
Change-Id: Ia4b558893d7d10156b77121cd9b70c4ccd9e1889
Reviewed-on: https://go-review.googlesource.com/c/go/+/189318
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
This CL looks big but it only does formatting changes to arith_s390x.s.
The file was formatted using asmfmt(https://github.com/klauspost/asmfmt)
, so there should not be any functional impact. I verified that the
generated assembly of big.test file is identical.
Change-Id: I8b4035ef082a4d0357881869327e25253f2d8be1
Reviewed-on: https://go-review.googlesource.com/c/go/+/229302
Reviewed-by: Michael Munday <mike.munday@ibm.com>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
With old object files, when objdump an object file which, for
example, contains a call of fmt.Fprintf, it shows a symbol
reference like
R_CALL:fmt.Fprintf
With new object files, as the symbol reference is indexed, the
reference becomes
R_CALL:fmt.#33
The object file does not contain information of what symbol #33
in the fmt package is.
To make this more useful, print the index when dumping the symbol
definitions. This way, when dumping the fmt package, e.g.
"go tool nm fmt.a", it will print
6c705 T fmt.Fprintf#33
So we can find out what symbol #33 actually is.
Change-Id: I320776597d28615ce18dd0617c352d2b8180db49
Reviewed-on: https://go-review.googlesource.com/c/go/+/229246
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
It is no longer needed as we have converted the fieldtrack pass
to using the loader.
Also free loader.Reachparent after we are done with it.
Change-Id: Ibc4b29f282e1e4aea363a1b549755e31f84b0295
Reviewed-on: https://go-review.googlesource.com/c/go/+/229322
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Currently, we use a dense array to store symbol's sections. The
array element is a *sym.Section, which takes 8 bytes per symbol
on a 64-bit machine. And the array is created upfront.
To reduce memory usage, use a 16-bit index for sections, so we
store 2 bytes per symbol. The array is pointerless, reducing GC
work. Also create the array lazily.
This reduces some memory usage: linking cmd/compile,
name old alloc/op new alloc/op delta
Loadlib_GC 42.1MB ± 0% 36.2MB ± 0% -14.01% (p=0.008 n=5+5)
name old live-B new live-B delta
Loadlib_GC 16.8M ± 0% 15.4M ± 0% -8.36% (p=0.008 n=5+5)
Archive_GC 98.2M ± 0% 97.2M ± 0% -1.02% (p=0.008 n=5+5) # at the end
Change-Id: If8c41eded8859660bca648c5e6fdf5830810fbf6
Reviewed-on: https://go-review.googlesource.com/c/go/+/229306
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Originally on s390x, ADDE does not work when adding numbers from a memory location.
For example: ADDE (R3), R4 will result in a failure.
Since ADDC, ADD and ADDW already supports adding from memory location,
let's support that for ADDE as well.
Change-Id: I7cbe112ea154733a621b948c6a21bbee63fb0c62
Reviewed-on: https://go-review.googlesource.com/c/go/+/229304
Reviewed-by: Michael Munday <mike.munday@ibm.com>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This gives us better expected information for daylight savings time
transitions in year 2038 and beyond.
Fixes#36654
Change-Id: I5a39aed3c40b184e1d7bb7d6ce3aff5307c4c146
Reviewed-on: https://go-review.googlesource.com/c/go/+/215539
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Clean up the code a little bit to make it clearer:
Don't check throwsplit for a SI_USER signal.
If throwsplit is set for a SigPanic signal, always throw;
discard any other flags.
Fixes#36420
Change-Id: Ic9dcd1108603d241f71c040504dfdc6e528f9767
Reviewed-on: https://go-review.googlesource.com/c/go/+/228900
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Wait for Listeners to drop to zero too, not just conns.
Fixes#33313
Change-Id: I09350ae38087990d368dcf9302fbde3e95c02fcd
Reviewed-on: https://go-review.googlesource.com/c/go/+/213442
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hasit Bhatt <hasit.p.bhatt@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently markrootSpans, the scanning routine which scans span specials
(particularly finalizers) as roots, uses sweepSpans to shard work and
find spans to mark.
However, as part of a future CL to change span ownership and how
mcentral works, we want to avoid having markrootSpans use the sweep bufs
to find specials, so in this change we introduce a new mechanism.
Much like for the page reclaimer, we set up a per-page bitmap where the
first page for a span is marked if the span contains any specials, and
unmarked if it has no specials. This bitmap is updated by addspecial,
removespecial, and during sweeping.
markrootSpans then shards this bitmap into mark work and markers iterate
over the bitmap looking for spans with specials to mark. Unlike the page
reclaimer, we don't need to use the pageInUse bits because having a
special implies that a span is in-use.
While in terms of computational complexity this design is technically
worse, because it needs to iterate over the mapped heap, in practice
this iteration is very fast (we can skip over large swathes of the heap
very quickly) and we only look at spans that have any specials at all,
rather than having to touch each span.
This new implementation of markrootSpans is behind a feature flag called
go115NewMarkrootSpans.
Updates #37487.
Change-Id: I8ea07b6c11059f6d412fe419e0ab512d989377b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/221178
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Previously for a method value "x.M", we always flowed x directly to
the heap, which led to the receiver argument generally needing to be
heap allocated.
This CL changes it to flow x to the closure and M's receiver
parameter. This allows receiver arguments to be stack allocated as
long as (1) the closure never escapes, *and* (2) method doesn't leak
its receiver parameter.
Within the standard library, this allows a handful of objects to be
stack allocated instead. Listed here are diagnostics that were
previously emitted by "go build -gcflags=-m std cmd" that are no
longer emitted:
archive/tar/writer.go:118:6: moved to heap: f
archive/tar/writer.go:208:6: moved to heap: f
archive/tar/writer.go:248:6: moved to heap: f
cmd/compile/internal/gc/initorder.go:252:2: moved to heap: d
cmd/compile/internal/gc/initorder.go:75:2: moved to heap: s
cmd/go/internal/generate/generate.go:206:7: &Generator literal escapes to heap
cmd/internal/obj/arm64/asm7.go:910:2: moved to heap: c
cmd/internal/obj/mips/asm0.go:415:2: moved to heap: c
cmd/internal/obj/pcln.go:294:22: new(pcinlineState) escapes to heap
cmd/internal/obj/s390x/asmz.go:459:2: moved to heap: c
crypto/tls/handshake_server.go:56:2: moved to heap: hs
Thanks to Cuong Manh Le for help coming up with this solution.
Fixes#27557.
Change-Id: I8c85d671d07fb9b53e11d2dd05949a34dbbd7e17
Reviewed-on: https://go-review.googlesource.com/c/go/+/228263
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL refactors tagHole to handle all three call situations (unknown
function; known function in same analysis batch; known function in
previous analysis batch). This will make it somewhat easier to reuse
in a followup CL.
Passes toolstash-check.
Change-Id: I764d047a333dfc593d721a881361683e94b485df
Reviewed-on: https://go-review.googlesource.com/c/go/+/229059
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
During schedinit, these may occur in:
mProf_Malloc
stkbucket
newBucket
persistentalloc
persistentalloc1
mProf_Malloc
setprofilebucket
fixalloc.alloc
persistentalloc
persistentalloc1
These seem to be legitimate lock orderings.
Additionally, mheap.speciallock had a defined rank, but it was never
actually used. That is fixed now.
Updates #38474
Change-Id: I0f6e981852eac66dafb72159f426476509620a65
Reviewed-on: https://go-review.googlesource.com/c/go/+/228786
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
When generating code for unsigned equals (==) and not equals (!=)
comparisons we currently, on s390x, always use signed comparisons.
This mostly works well, however signed comparisons on s390x sign
extend their immediates and unsigned comparisons zero extend them.
For compare-and-branch instructions which can only have 8-bit
immediates this significantly changes the range of immediate values
we can represent: [-128, 127] for signed comparisons and [0, 255]
for unsigned comparisons.
When generating equals and not equals checks we don't neet to worry
about whether the comparison is signed or unsigned. This CL
therefore adds rules to allow us to switch signedness for such
comparisons if it means that it brings a constant into range for an
8-bit immediate.
For example, a signed equals with an integer in the range [128, 255]
will now be implemented using an unsigned compare-and-branch
instruction rather than separate compare and branch instructions.
As part of this change I've also added support for adding a name
to block control values using the same `x:(...)` syntax we use for
value rules.
Triggers 792 times when compiling cmd and std.
Change-Id: I77fa80a128f0a8ce51a2888d1e384bd5e9b61a77
Reviewed-on: https://go-review.googlesource.com/c/go/+/228642
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This reverts commit 1f0738c157.
Reason for revert: This May have caused issue 38567.
Change-Id: I2afa6a9d42cb29cfad09e706fb465c57e3774abd
Reviewed-on: https://go-review.googlesource.com/c/go/+/229301
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
