Change-Id: I9d399db8ac26ad44adeace3bf1e5b11cbfe3e0d3
Reviewed-on: https://go-review.googlesource.com/41313
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Hopefully the last refactoring of TestRespectGroupSticky:
* Properly tested (+simplified) FreeBSD fix
* Tested on Darwin (10.12.4)
* Rename to TestRespectSetgidDir (I believe this is the accepted
terminology)
Fixesgolang/go#19596.
Change-Id: I8d689ac3e245846cb3f1338ea13e35be512ccb9c
Reviewed-on: https://go-review.googlesource.com/41430
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Change a few functions so that instead of
accepting a *types.Sym and calling Linksym
themselves, they accept an *obj.LSym.
Adapt the callsites.
Passes toolstash-check.
Change-Id: Ic5d3f306f2fdd3913281215a1f54d893a966bb1f
Reviewed-on: https://go-review.googlesource.com/41404
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This batch from reflect.go.
Changes made manually, since they are simple,
few, and typechecked by the compiler.
Passes toolstash-check.
Change-Id: I0030daab2dac8e7c95158678c0f7141fd90441f9
Reviewed-on: https://go-review.googlesource.com/41399
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The only remaining uses of duintxx
are in the implementation of duintNN.
I hope to inline those once I figure out why
CL 40864 is broken.
Note that some uses of duintxx with width Widthint
were converted into duintptr.
I did that, since #19954 is officially going to move forward.
Passes toolstash-check.
Change-Id: Id25253b711ea589d0199b51be9a3c18ca1af59ce
Reviewed-on: https://go-review.googlesource.com/41398
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is an automated refactoring to eliminate
all dxxx calls in gc/obj.go that accept types.Sym
instead of obj.LSym parameters.
The refactoring was of the form:
gorename -from '"cmd/compile/internal/gc".duintxx' -to Duintxx
gorename -from '"cmd/compile/internal/gc".duintxxLSym' -to DuintxxLSym
eg -t t.go -w cmd/compile/internal/gc
gofmt -r 'DuintxxLSym -> duintxxLSym' -w cmd/compile/internal/gc
where t.go looked like:
func before(s *types.Sym, off int, v uint64, wid int) int {
return gc.Duintxx(s, off, v, wid)
}
func after(s *types.Sym, off int, v uint64, wid int) int {
return gc.DuintxxLSym(s.Linksym(), off, v, wid)
}
The rename/gofmt shenanigans were to work around
limitations and bugs in eg and gorename.
The resulting code in reflect.go looks temporarily ugly,
but it makes refactoring and cleanup opportunities
much clearer.
Next step is to rename all the dxxx methods to rename the -LSym suffix
and clean up reflect.go.
The renaming is left for a separate CL to make the changes in
this CL more obvious, and thus hopefully easier to review.
Passes toolstash-check.
Change-Id: Ib31a2b6fd146ed03a855d20ecb0433f0f74e2f10
Reviewed-on: https://go-review.googlesource.com/41396
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
- Add new BranchStmt.Target field: It's the destination for break,
continue, or goto statements.
- When parsing with CheckBranches enabled, set the BranchStmt.Target
field. We get the information practically for free from the branch
checker, so keep it for further use.
- Fix a couple of comments.
- This could use a test, but the new Target field is currently not
used, and writing a test is tedious w/o a general tree visitor.
Do it later. For now, visually verified output from syntax dump.
Change-Id: Id691d89efab514ad885e19ac9759506106579520
Reviewed-on: https://go-review.googlesource.com/40988
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
ARM's udiv function is nosplit and it shouldn't be preemptied
(passing args in registers). It is in some sense like DUFFCOPY,
which we don't mark as safepoint.
Change-Id: I49f7c4e69e787ac364d0b0def0661e79a0ea9e69
Reviewed-on: https://go-review.googlesource.com/41370
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Currently one needs to refer to the sources to have a list of accepted
debug keys. We can copy what 'ssa/help' does and introspect the list of
debug keys to print a more detailed help:
$ go tool compile -d help
usage: -d arg[,arg]* and arg is <key>[=<value>]
<key> is one of:
append print information about append compilation
closure print information about closure compilation
disablenil disable nil checks
dclstack run internal dclstack check
gcprog print dump of GC programs
nil print information about nil checks
panic do not hide any compiler panic
slice print information about slice compilation
typeassert print information about type assertion inlining
wb print information about write barriers
export print export data
pctab print named pc-value table
ssa/help print help about SSA debugging
<value> is key-specific.
Key "pctab" supports values:
"pctospadj", "pctofile", "pctoline", "pctoinline", "pctopcdata"
For '-d help' to be discoverable, a hint is given in the -d flag
description.
A last thing, today at least one go file needs to be provided to get to
the code printing ssa/help.
$ go tool compile -d ssa/help foo.go
Add a check so one can just do '-d help' or '-d ssa/help'
Caught by trybot: I needed to update fmt_test.go as I'm introducing the
usage of %-*s in a format string.
Fixes#20041
Change-Id: Ib2858b038c1bcbe644aa3b1a371009710c6d957d
Reviewed-on: https://go-review.googlesource.com/41091
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
The experiment "clobberdead" clobbers all pointer fields that the
compiler thinks are dead, just before and after every safepoint.
Useful for debugging the generation of live pointer bitmaps.
Helped find the following issues:
Update #15936
Update #16026
Update #16095
Update #18860
Change-Id: Id1d12f86845e3d93bae903d968b1eac61fc461f9
Reviewed-on: https://go-review.googlesource.com/23924
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Currently TestSetGCPercent checks that NextGC is within 10 MB of the
expected value. For some reason it's much noisier on some of the
builders. To get these passing again, raise the threshold to 20 MB.
Change-Id: I14e64025660d782d81ff0421c1eb898f416e11fe
Reviewed-on: https://go-review.googlesource.com/41374
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
More steps towards simpler symbol handling:
- Pushdcl's incoming pos argument, saved in a newly pushed *Sym, was always
immediately overwritten by the Lastlineno value of the saved *Sym.
- Markdcl's incoming pos argument, saved in the stack mark *Sym, was not
restored when the stack mark was popped.
- Popdcl always maintained the most recent Lastlineno for a *Sym given
by package and name, making it unnecessary to save Lastlineno in the
first place. Removed Lastlineno from the set of fields that need saving,
and simplified Popdcl.
Change-Id: Ie93da1fbd780dcafc2703044e781c0c6298df569
Reviewed-on: https://go-review.googlesource.com/41390
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Currently SetGCPercent forces a GC in order to recompute GC pacing.
Since we can now recompute pacing on the fly using gcSetTriggerRatio,
change SetGCPercent (really runtime.setGCPercent) to go through
gcSetTriggerRatio and not trigger a GC.
Fixes#19076.
Change-Id: Ib30d7ab1bb3b55219535b9f238108f3d45a1b522
Reviewed-on: https://go-review.googlesource.com/39835
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
The current SetGCPercent test is, shall we say, minimal.
Expand it to check that the GC target is actually computed and updated
correctly.
For #19076.
Change-Id: I6e9b2ee0ef369f22f72e43b58d89e9f1e1b73b1b
Reviewed-on: https://go-review.googlesource.com/39834
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
This changes gcSetTriggerRatio so it can be called even during
concurrent mark or sweep. In this case, it will adjust the pacing of
the current phase, accounting for progress that has already been made.
To make this work for concurrent sweep, this introduces a "basis" for
the pagesSwept count, much like the basis we just introduced for
heap_live. This lets gcSetTriggerRatio shift the basis to the current
heap_live and pagesSwept and compute a slope from there to completion.
This avoids creating a discontinuity where, if the ratio has
increased, there has to be a flurry of sweep activity to catch up.
Instead, this creates a continuous, piece-wise linear function as
adjustments are made.
For #19076.
Change-Id: Ibcd76aeeb81ff4814b00be7cbd3530b73bbdbba9
Reviewed-on: https://go-review.googlesource.com/39833
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently, proportional sweep maintains its own count of how many
bytes have been allocated since the beginning of the sweep cycle so it
can compute how many pages need to be swept for a given allocation.
However, this requires a somewhat complex reimbursement scheme since
proportional sweep must be done before a span is allocated, but we
don't know how many bytes to charge until we've allocated a span. This
means that the allocated byte count used by proportional sweep can go
up and down, which has led to underflow bugs in the past (#18043) and
is going to interfere with adjusting sweep pacing on-the-fly (for #19076).
This approach also means we're maintaining a statistic that is very
closely related to heap_live, but has a different 0 value. This is
particularly confusing because the sweep ratio is computed based on
heap_live, so you have to understand that these two statistics are
very closely related.
Replace all of this and compute the sweep debt directly from the
current value of heap_live. To make this work, we simply save the
value of heap_live when the sweep ratio is computed to use as a
"basis" for later computing the sweep debt.
This eliminates the need for reimbursement as well as the code for
maintaining the sweeper's version of the live heap size.
For #19076.
Coincidentally fixes#18043, since this eliminates sweep reimbursement
entirely.
Change-Id: I1f931ddd6e90c901a3972c7506874c899251dc2a
Reviewed-on: https://go-review.googlesource.com/39832
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently, the computations that derive controls from the GC trigger
are spread across several parts of the mark termination code.
Consolidate computing the absolute trigger, the heap goal, and sweep
pacing into a single function called at the end of mark termination.
Unlike the code being consolidated, this has to be more careful about
negative gcpercent. Many of the consolidated code paths simply didn't
execute if GC was off.
This is a step toward being able to change the GC trigger ratio in the
middle of concurrent sweeping and marking. For this commit, we try to
stick close to the original structure of the code that's being
consolidated, so it doesn't yet support mid-cycle adjustments.
For #19076.
Change-Id: Ic5335be04b96ad20e70d53d67913a86bd6b31456
Reviewed-on: https://go-review.googlesource.com/39831
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
gcController.triggerRatio is the only field in gcController that
persists across cycles. As global mutable state, the places where it
written and read are spread out, making it difficult to see that
updates and downstream calculations are done correctly.
Improve this situation by doing two things:
1) Move triggerRatio to memstats so it lives with the other
trigger-related fields and makes gcController entirely transient
state.
2) Commit the new trigger ratio during mark termination when we
compute other next-cycle controls, including the absolute trigger.
This forces us to explicitly thread the new trigger ratio from
gcController.endCycle to mark termination, so we're not just pulling
it out of global state.
Change-Id: I6669932f8039a8c0ef46a3f2a8c537db72e578aa
Reviewed-on: https://go-review.googlesource.com/39830
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
heap_live is updated atomically without locking, so we should also use
atomic loads to read it. Fix the reads of heap_live that happen
outside of STW to be atomic.
Change-Id: Idca9451c348168c2a792a9499af349833a3c333f
Reviewed-on: https://go-review.googlesource.com/41371
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Change-Id: I0f8c695146b39cff72ca2374f861f3e9f72b0f77
Reviewed-on: https://go-review.googlesource.com/41314
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The inliner's ishairy passes a budget and a reason down through the
walk. Lift these into a visitor object and turn ishairy and its
helpers into methods.
This will make it easy to add more state.
Change-Id: Ic6ae246e1affd67ed283c3205f9595ae33e22215
Reviewed-on: https://go-review.googlesource.com/41151
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Response to code review feedback on CL 40693.
It is now only accessible by types.TypePkgLookup.
Passes toolstash-check.
Change-Id: I0c422c1a271f97467ae38de53af9dc33f4b31bdb
Reviewed-on: https://go-review.googlesource.com/41304
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Response to code review feedback on CL 40693.
Remove the final reference to it from package gc,
and manually unexport.
Passes toolstash-check.
Change-Id: I7fc48edd43263d8f7c56b47aeb7573408463dc22
Reviewed-on: https://go-review.googlesource.com/41303
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Previously, helpers were identified by entry PC, but this breaks if the
helper is inlined (as in notHelperCallingHelper). Instead, identify
helpers by function name (with package path). Now TestTBHelper and
TestTBHelperParallel pass with -l=4.
To keep the code unified, this change makes it so that the runner
is also identified by function name instead of entry PC.
Change-Id: I1b1987fc49d114e69d075fab56aeeacd5294982b
Reviewed-on: https://go-review.googlesource.com/41257
Run-TryBot: David Lazar <lazard@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
runtime.getcaller{pc,sp} expect their argument to be a pointer to the
caller's first function argument. This assumption breaks when the caller
is inlined. For example, with -l=4, calls to runtime.entersyscall (which
calls getcallerpc) are inlined and that breaks multiple cgo tests.
This change modifies the compiler to refuse to inline functions that
call runtime.getcaller{pc,sp}. Alternatively, we could mark these
functions //go:noinline but that limits optimization opportunities if
the calls to getcaller{pc,sp} are eliminated as dead code.
Previously TestCgoPprofPIE, TestCgoPprof, and TestCgoCallbackGC failed
with -l=4. Now all of the runtime tests pass with -l=4.
Change-Id: I258bca9025e20fc451e673a18f862b5da1e07ae7
Reviewed-on: https://go-review.googlesource.com/40998
Run-TryBot: David Lazar <lazard@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Austin Clements <austin@google.com>
On 32-bit architectures (or if we fail to map a 64-bit-style arena),
we try to map the heap arena just above the end of the process image.
While we can accept any address, using lower addresses is preferable
because lower addresses cause us to map less of the heap bitmap.
However, if a program is linked against C code that has global
constructors, those constructors may call brk/sbrk to allocate memory
(e.g., many C malloc implementations do this for small allocations).
The brk also starts just above the process image, so this may adjust
the brk past the beginning of where we want to put the heap arena. In
this case, the kernel will pick a different address for the arena and
it will usually be very high (at least, as these things go in a 32-bit
address space).
Fix this by consulting the current value of the brk and using this in
addition to the end of the process image to compute the initial arena
placement.
This is implemented only on Linux currently, since we have no evidence
that it's an issue on any other OSes.
Fixes#19831.
Change-Id: Id64b45d08d8c91e4f50d92d0339146250b04f2f8
Reviewed-on: https://go-review.googlesource.com/39810
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This makes the cmd/compile/internal/ssa package
compile much faster, and has no impact
on the speed of the compiler.
The chunk size was selected empirically,
in that at chunk size 10, the object
file was smaller than at chunk size 5 or 20.
name old time/op new time/op delta
SSA 7.33s ± 5% 5.64s ± 1% -23.10% (p=0.000 n=10+10)
name old user-time/op new user-time/op delta
SSA 9.70s ± 1% 8.04s ± 2% -17.17% (p=0.000 n=9+10)
name old obj-bytes new obj-bytes delta
SSA 9.82M ± 0% 8.28M ± 0% -15.67% (p=0.000 n=10+10)
Change-Id: Iab472905da3f0e82f3db2c93d06e2759abc9dd44
Reviewed-on: https://go-review.googlesource.com/41296
Reviewed-by: Keith Randall <khr@golang.org>
They are left over from the days before
we had BlockKindFirst and swapSuccessors.
Change-Id: I9259d53ac2821ca4d5de5dd520ca4b78f52ecad4
Reviewed-on: https://go-review.googlesource.com/41206
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
FreeBSD doesn't allow non-root users to enable the SetGID bit on
files or directories in /tmp, however it does allow this in
subdirectories, so create the test directory one level deeper.
Followup to golang/go#19596.
Change-Id: I30e71c6d6a156badc863e8068df10ef6ed817e26
Reviewed-on: https://go-review.googlesource.com/41216
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Brad noticed a bullet list was rendered as preformatted text because of
the indentation. One can use a unicode bullet as an ersatz for bullet
lists.
Fixes#20043
Change-Id: Iaed3582d14bd05920455669039a900d7155960d9
Reviewed-on: https://go-review.googlesource.com/41212
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Adding the Go Time podcast under the `Stay informed` section of the help
page on the website.
Change-Id: Ifb1c6bb20cbf640a91572d47f14a432f58439261
Reviewed-on: https://go-review.googlesource.com/41146
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
At VARKILLs, zero a variable if it is ambiguously live.
After the VARKILL anything this variable references
might be collected. If it were to become live again later,
the GC will see references to already-collected objects.
We don't know a variable is ambiguously live until very
late in compilation (after lowering, register allocation, ...),
so it is hard to generate the code in an arch-independent way.
We also have to be careful not to clobber any registers.
Fortunately, this almost never happens so performance is ~irrelevant.
There are only 2 instances where this triggers in the stdlib.
Fixes#20029
Change-Id: Ia9585a91d7b823fad4a9d141d954464cc7af31f4
Reviewed-on: https://go-review.googlesource.com/41076
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Make poolLocal size multiple of 128, so it aligns to CPU cache line
on the most common architectures.
This also has the following benefits:
- It may help compiler substituting integer multiplication
by bit shift inside indexLocal.
- It shrinks poolLocal size from 176 bytes to 128 bytes on amd64,
so now it fits two cache lines (or a single cache line on certain
Intel CPUs - see https://software.intel.com/en-us/articles/optimizing-application-performance-on-intel-coret-microarchitecture-using-hardware-implemented-prefetchers).
No measurable performance changes on linux/amd64 and linux/386.
Change-Id: I11df0f064718a662e77a85d88b8a15a8919f25e9
Reviewed-on: https://go-review.googlesource.com/40918
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
