Make typedmemmove, typedmemclr, typedmemclrpartial look more like other
callers of bulkBarrierPreWrite.
Change-Id: Ic47030d88bf07d290f91198b7810ffc16d9769e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/227541
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The cgo build tag is not necessary for root_darwin_arm64.go. We can't
build for darwin/arm64 without cgo, and even if we did 1) this code
would work fine 2) the no-cgo code that shells out to
/usr/bin/security would not work.
(Suggested by Filippo.)
Change-Id: I98cac2ea96ec5ac1ae60b7e32d195d5e86e2bd66
Reviewed-on: https://go-review.googlesource.com/c/go/+/227583
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This test is currently flaky in the builders.
Skip it while we investigate.
For #37404
Change-Id: I53721d383a4cafbe8d031ed25a3b1be2ae8b4285
Reviewed-on: https://go-review.googlesource.com/c/go/+/227587
Reviewed-by: David Chase <drchase@google.com>
This removes all conditions and conditional code (that I could find)
that depended on darwin/386.
Fixes#37610.
Change-Id: I630d9ea13613fb7c0bcdb981e8367facff250ba0
Reviewed-on: https://go-review.googlesource.com/c/go/+/227582
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This removes all files that are only used on darwin/386 and cleans up
build tags in files that are still used on other platforms.
Updates #37610.
Change-Id: If246642476c12d15f59a474e2b91a29c0c02fe75
Reviewed-on: https://go-review.googlesource.com/c/go/+/227581
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This removes all conditions and conditional code (that I could find)
that depended on darwin/arm.
Fixes#35439 (since that only happened on darwin/arm)
Fixes#37611.
Change-Id: Ia4c32a5a4368ed75231075832b0b5bfb1ad11986
Reviewed-on: https://go-review.googlesource.com/c/go/+/227198
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This removes all files that are only used on darwin/arm and cleans up
build tags in files that are still used on other platforms.
Updates #37611.
Change-Id: Ic9490cf0edfc157c6276a7ca950c1768b34a998f
Reviewed-on: https://go-review.googlesource.com/c/go/+/227197
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Adds a GOMODCACHE environment variable that's used by cmd/go to determine the
location of the module cache. The default value of GOMODCACHE will be
GOPATH[0]/pkg/mod, the default location of the module cache before this change.
Replace the cmd/go/internal/modfetch.PkgMod variable which previously held the
location of the module cache with the new cmd/go/internal/cfg.GOMODCACHE
variable, for consistency with many of the other environment variables that
affect the behavior of cmd/go. (Most of the changes in this CL are due to
moving/renaming the variable.)
The value of cfg.GOMODCACHE is now set using a variable initializer. It was
previously set in cmd/go/internal/modload.Init.
The location of GOPATH/pkg/sumdb is unchanged by this CL. While it was
previously determined using the value of PkgMod, it now is determined
independently dirctly from the value of GOPATH[0].
Fixes#34527
Change-Id: Id4d31d217b3507d6057c8ef7c52af1a0606603e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/219538
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Don't emit an error for undefined symbol in trampoline
generation pass, which will be duplicate as we'll emit a better
one later.
Fix TestUndefinedRelocErrors on PPC64.
Change-Id: I964d4bd63ec4b4c6eb5d98caf68db93ce6488bf5
Reviewed-on: https://go-review.googlesource.com/c/go/+/227617
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
TestPingPongHog tests properties of the scheduler.
But the race detector intentionally does randomized scheduling,
so the test is not applicable.
Fixes#38266
Change-Id: Ib06aa317b2776cb1faa641c4e038e2599cf70b2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/227344
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
I somehow messed up my local environment for the pre-commit hook.
Fix the format.
Change-Id: I6161ad1fce7714e8f311be5315d854f8c9bd7664
Reviewed-on: https://go-review.googlesource.com/c/go/+/227579
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Implement text address assignment and trampoline generation using
the loader.
Note: the trampoline insertion part doesn't actually work. It
also needs to propagate Aux symbols for external symbols in
LoadFull. But it won't be needed after converting pclntab
generation, so I'll leave it out for now. This could break
linking large binaries on PPC64 and ARM.
Change-Id: Ie46a35b25d7c027983dd877207cfa8f67c32530b
Reviewed-on: https://go-review.googlesource.com/c/go/+/227482
Reviewed-by: Than McIntosh <thanm@google.com>
SymFile, derived from sym.Symbol.File, is supposed to return the
package path, instead of the file name (arguably the name is
confusing). Make it so, and rename it to SymPkg.
Change-Id: I67bcd12f67cea271f2a2ce3c5724e5d228f5b2f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/227481
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
TestTryAdd is particularly brittle because it tests some real cases by
constructing fake sample stack frames. If those frames don't correctly
represent what the runtime would generate then they may fail to catch
regressions.
Instead, call runtime.Callers at the bottom of real function calls to
generate real frames as a base for truncation, etc in tests. Several of
these tests still have to fake parts of the frames to test the right
thing, but this is a bit less fragile.
Change-Id: I62522a9ded5544b06d1bf28550af5400f3af667b
Reviewed-on: https://go-review.googlesource.com/c/go/+/227484
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
This change adds CFA information to the assembly function 'crosscall1'
and reorgnizes its code to establish well-formed prologue and epilogue.
It will fix an infinite callstack issue when debugging cgo program with
GDB on arm64.
Brief root cause analysis:
GDB's aarch64 unwinder parses prologue to determine current frame's size
and previous PC&SP if CFA information is not available.
The unwinder parses the prologue of 'crosscall1' to determine a frame size
of 0x10, then turns to its next frame trying to compute its previous PC&SP
as they are not saved on current frame's stack as per its 'traditional frame
unwind' rules, which ends up getting an endless frame chain like:
[callee] : pc:<pc0>, sp:<sp0>
crosscall1: pc:<pc1>, sp:<sp0>+0x10
[caller] : pc:<pc1>, sp:<sp0>+0x10+0x10
[caller] : pc:<pc1>, sp:<sp0>+0x10+0x10+0x10
...
GDB fails to detect the 'caller' frame is same as 'crosscall1' and terminate
unwinding since SP increases everytime.
Fixes#37238
Change-Id: Ia6bd8555828541a3a61f7dc9b94dfa00775ec52a
Reviewed-on: https://go-review.googlesource.com/c/go/+/226999
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
I took some of the infrastructure from Austin's lock logging CR
https://go-review.googlesource.com/c/go/+/192704 (with deadlock
detection from the logs), and developed a setup to give static lock
ranking for runtime locks.
Static lock ranking establishes a documented total ordering among locks,
and then reports an error if the total order is violated. This can
happen if a deadlock happens (by acquiring a sequence of locks in
different orders), or if just one side of a possible deadlock happens.
Lock ordering deadlocks cannot happen as long as the lock ordering is
followed.
Along the way, I found a deadlock involving the new timer code, which Ian fixed
via https://go-review.googlesource.com/c/go/+/207348, as well as two other
potential deadlocks.
See the constants at the top of runtime/lockrank.go to show the static
lock ranking that I ended up with, along with some comments. This is
great documentation of the current intended lock ordering when acquiring
multiple locks in the runtime.
I also added an array lockPartialOrder[] which shows and enforces the
current partial ordering among locks (which is embedded within the total
ordering). This is more specific about the dependencies among locks.
I don't try to check the ranking within a lock class with multiple locks
that can be acquired at the same time (i.e. check the ranking when
multiple hchan locks are acquired).
Currently, I am doing a lockInit() call to set the lock rank of most
locks. Any lock that is not otherwise initialized is assumed to be a
leaf lock (a very high rank lock), so that eliminates the need to do
anything for a bunch of locks (including all architecture-dependent
locks). For two locks, root.lock and notifyList.lock (only in the
runtime/sema.go file), it is not as easy to do lock initialization, so
instead, I am passing the lock rank with the lock calls.
For Windows compilation, I needed to increase the StackGuard size from
896 to 928 because of the new lock-rank checking functions.
Checking of the static lock ranking is enabled by setting
GOEXPERIMENT=staticlockranking before doing a run.
To make sure that the static lock ranking code has no overhead in memory
or CPU when not enabled by GOEXPERIMENT, I changed 'go build/install' so
that it defines a build tag (with the same name) whenever any experiment
has been baked into the toolchain (by checking Expstring()). This allows
me to avoid increasing the size of the 'mutex' type when static lock
ranking is not enabled.
Fixes#38029
Change-Id: I154217ff307c47051f8dae9c2a03b53081acd83a
Reviewed-on: https://go-review.googlesource.com/c/go/+/207619
Reviewed-by: Dan Scales <danscales@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Extend CL 220417 (which removed the integer Greater and Geq ops) to
floating point comparisons. Greater and Geq can always be
implemented using Less and Leq.
Fixes#37316.
Change-Id: Ieaddb4877dd0ff9037a1dd11d0a9a9e45ced71e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/222397
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Convert the ppc64 architecture's version of gentext to use the new
loader APIs.
Change-Id: Ib4af2608f4b246cb6dde07ceaa4a1f7ced45a700
Reviewed-on: https://go-review.googlesource.com/c/go/+/227021
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In the commit message of CL 212360, I wrote:
> This new intrinsic ... generates MOVB+TESTB+NE.
> (It is possible that MOVBQZX+TESTQ+NE would be better.)
I should have tested. MOVBQZX+TESTQ+NE does in fact appear to be better.
For the benchmark in #36196, on my machine:
name old time/op new time/op delta
FMA-8 0.86ns ± 6% 0.70ns ± 5% -18.79% (p=0.000 n=98+97)
NonFMA-8 0.61ns ± 5% 0.60ns ± 4% -0.74% (p=0.001 n=100+97)
Interestingly, these are both considerably faster than
the measurements I took a couple of months ago (1.4ns/2ns).
It appears that CL 219131 (clearing VZEROUPPER in asyncPreempt) helped a lot.
And FMA is now once again slower than NonFMA, although this change
helps it regain some ground.
Updates #15808
Updates #36351
Updates #36196
Change-Id: I8a326289a963b1939aaa7eaa2fab2ec536467c7d
Reviewed-on: https://go-review.googlesource.com/c/go/+/227238
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The test was not preserving temporary directory flags leading to a
failure on windows with:
mkdir C:\WINDOWS\go-build315158903: Access is denied.
Fixes#38251
Change-Id: I6ee31b31e84b7f6e75ea6ee0f3b8c094835bf5d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/227497
Reviewed-by: David Chase <drchase@google.com>
The code in setArchsyms that sets up TOC symbols was buggy; it was
kicking in only for aix-ppc64 and not linux-ppc64. These symbols are
required for both ABIs, so change the guard in question from
"ctx.IsAIX()" to "ctxt.IsPPC64()". Also, the code to create versioned
".TOC." syms was not passing the correct symbol version to the loader
(now fixed).
Change-Id: I356071e528beadad20f61d067059eaf26f06e06b
Reviewed-on: https://go-review.googlesource.com/c/go/+/227257
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
On s390x, we already have MVCIN opcode in asmz.go,
but we did not use it. This CL uses that opcode and adds MVCIN
instruction.
MVCIN instruction can be used to move data from one storage location
to another while reversing the order of bytes within the field. This
could be useful when transforming data from little-endian to big-endian.
Change-Id: Ifa1a911c0d3442f4a62f91f74ed25b196d01636b
Reviewed-on: https://go-review.googlesource.com/c/go/+/227478
Reviewed-by: Michael Munday <mike.munday@ibm.com>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Remove some extra sanity-checking code from the loader's RelocVariant
method, since it was yielding a slowdown of 1-2% linking kubernetes
hyperkube (once again a reminder that relocation processing is a very
performance-sensitive part of the linker).
Change-Id: Ifbc0662f3f96c5f54131103ce6f7439ecfb9b9dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/227477
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
This CL extracts some error handling code into a common method for
presenting errors encountered when loading package data.
Fixes#36087Fixes#36762
Change-Id: I87c8d41e3cc6e6afa152d9c067bc60923bf19fbe
Reviewed-on: https://go-review.googlesource.com/c/go/+/210938
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The darwin/arm port is removed in Go 1.15. Setting GOOS=darwin
GOARCH=arm will fail, therefore "go test cmd/link" on macOS will
fail (in non -short mode). Remove this test point.
Updates #37611.
Change-Id: Ia9531c4b4a6692a0c49153517af9fdddd1f3e0bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/227341
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Complete a long-standing TODO in the code.
Exit blocks are cold code, so we lay them out at the end of the function.
Blocks that are post-dominated by exit blocks are also ipso facto exit blocks.
Treat them as such.
Implement using a simple loop, because there are generally very few exit blocks.
In addition to improved instruction cache, this empirically yields
better register allocation.
Binary size impact:
file before after Δ %
cgo 4812872 4808776 -4096 -0.085%
fix 3370072 3365976 -4096 -0.122%
vet 8252280 8248184 -4096 -0.050%
total 115052984 115040696 -12288 -0.011%
This also appears to improve compiler performance
(-0.15% geomean time/op, -1.20% geomean user time/op),
but that could just be alignment effects.
Compiler benchmarking hasn't been super reliably recently,
and there's no particular reason to think this should
speed up the compiler that much.
Change-Id: I3d262c4f5cb80626a67a5c17285e2fa09f423c00
Reviewed-on: https://go-review.googlesource.com/c/go/+/227217
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Multiple instances of testDWARF run in parallel, with a shared
backing store of the env input slice. Do modification of the
environment locally, instead of on the shared slice.
Fixes#38265.
Change-Id: I22a7194c8cd55ba22c9d6c47ac47bf7e710a7027
Reviewed-on: https://go-review.googlesource.com/c/go/+/227342
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
'go get' will now check absolute paths without wildcards the same way
it checks relative paths. modload.DirImportPath may be used for both
without converting path separators.
Fixes#38038
Change-Id: I453299898ece58f3b5002a5e80021d6bfe815fdd
Reviewed-on: https://go-review.googlesource.com/c/go/+/226857
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Add support to the loader for getting/setting the 'variant' property
of a symbol relocation. The variant property handles unusual or
infrequently used relocations that have both a type and a variant of
that type (this is needed for S390).
In the sym.Symbol world, a relocation variant is a field on the
'relocExt' extension that is part of sym.Reloc. In this new
implementation for the loader, reloc variants are stored in a side
table (a map) in the loader, and accessed via loader methods.
Change-Id: I62bf54ae7ff6d500c0ea8d2dbe759b2431087378
Reviewed-on: https://go-review.googlesource.com/c/go/+/227018
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Begin the job of converting the linker's "gentext" phase over to use
loader APIs. This patch includes most architectures except for s390x
and PPC (these will be added in subsequent patches, since they require
a couple of loader changes first).
Change-Id: Ic7f55c207dcdbbba657330ef007a72ff7c837416
Reviewed-on: https://go-review.googlesource.com/c/go/+/227017
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The auxint value was being printed in LongString() but not LongHTML().
Fixes#38250.
Change-Id: I28e819feef8710f912bee424d1b900eb07f3abb8
Reviewed-on: https://go-review.googlesource.com/c/go/+/227160
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Previously we stopped the timer and then reset it. With the current
timer implementation that is no longer required.
Change-Id: Ie7aba61ad53ce835f6fcd0b6bce7fe0a15b10e24
Reviewed-on: https://go-review.googlesource.com/c/go/+/227180
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
If the final pass(es) are identical during ssa.html generation,
they are persisted in-memory as "pendingPhases" but never get
written as a column in the html. This change flushes those
in-memory phases.
Fixes#38242
Change-Id: Id13477dcbe7b419a818bb457861b2422ba5ef4bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/227182
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Replace HTMLWriter's Logger field with a *Func. Implement Fatalf method
for HTMLWriter which gets the Frontend() from the Func and calls down
into it's Fatalf method, passing the msg and args along. Replace
remaining calls to the old Logger with calls to logging methods on
the Func.
Change-Id: I966342ef9997396f3416fb152fa52d60080ebecb
Reviewed-on: https://go-review.googlesource.com/c/go/+/227277
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 201783 enable -d=checkptr when -race or -msan is specified
everywhere but windows.
But, now that all unsafe pointer conversions in the standard
library are fixed, enable -d=checkptr even on windows.
Updates #34964
Updates #34972
Change-Id: Id912fa83b0d5b46c6f1c134c742fd94d2d185835
Reviewed-on: https://go-review.googlesource.com/c/go/+/227003
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This only removes the ability to build it, and removes it as a
src/buildall.bash target (which uses go tool dist list).
Now:
$ go tool dist list | grep ^darwin
darwin/amd64
darwin/arm64
After this, remaining is removing leftover port--specific code in the
tree.
Updates #37610
Updates #37611
Change-Id: I00f03b2355c2e152f75e57abd3063be243529d2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/226985
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Before using some CPU instructions, we must check for their presence.
We use global variables in the runtime package to record features.
Prior to this CL, we issued a regular memory load for these features.
The downside to this is that, because it is a regular memory load,
it cannot be hoisted out of loops or otherwise reordered with other loads.
This CL introduces a new intrinsic just for checking cpu features.
It still ends up resulting in a memory load, but that memory load can
now be floated to the entry block and rematerialized as needed.
One downside is that the regular load could be combined with the comparison
into a CMPBconstload+NE. This new intrinsic cannot; it generates MOVB+TESTB+NE.
(It is possible that MOVBQZX+TESTQ+NE would be better.)
This CL does only amd64. It is easy to extend to other architectures.
For the benchmark in #36196, on my machine, this offers a mild speedup.
name old time/op new time/op delta
FMA-8 1.39ns ± 6% 1.29ns ± 9% -7.19% (p=0.000 n=97+96)
NonFMA-8 2.03ns ±11% 2.04ns ±12% ~ (p=0.618 n=99+98)
Updates #15808
Updates #36196
Change-Id: I75e2fcfcf5a6df1bdb80657a7143bed69fca6deb
Reviewed-on: https://go-review.googlesource.com/c/go/+/212360
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
We still disallow inlining for an immediately-recursive function, but allow
inlining if a function is in a recursion chain.
If all functions in the recursion chain are simple, then we could inline
forever down the recursion chain (eventually running out of stack on the
compiler), so we add a map to keep track of the functions we have
already inlined at a call site. We stop inlining when we reach a
function that we have already inlined in the recursive chain. Of course,
normally the inlining will have stopped earlier, because of the cost
function.
We could also limit the depth of inlining by a simple count (say, limit
max inlining of 10 at any given site). Would that limit other
opportunities too much?
Added a test in test/inline.go. runtime.BenchmarkStackCopyNoCache() is
also already a good test that triggers the check to stop inlining
when we reach the start of the recursive chain again.
For the bent benchmark suite, the performance improvement was mostly not
statistically significant, but the geomean averaged out to: -0.68%. The text size
increase was less than .1% for all bent benchmarks. The cmd/go text size increase
was 0.02% and the cmd/compile text size increase was .1%.
Fixes#29737
Change-Id: I892fa84bb07a947b3125ec8f25ed0e508bf2bdf5
Reviewed-on: https://go-review.googlesource.com/c/go/+/226818
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
CL 221427 added TestExitCode. This test is failing
on Plan 9 because ExitCode is always equal to 1
on error since Plan 9 use error strings.
This change fixes TestExitCode by checking that
ExitCode is equal to 1 on error instead of the
specific value.
Fixes#38237.
Change-Id: Ie269722e731e275e5bfc51644c1fa6be76525f1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/227158
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
