Instead of populating the aux symbol
of CALLudiv during rewrite rules,
populate it during genssa.
This simplifies the rewrite rules.
It also removes all remaining calls
to ctxt.Lookup from any rewrite rules.
This is a first step towards removing
ctxt from ssa.Cache entirely,
and also a first step towards converting
the obj.LSym.Version field into a boolean.
It should also speed up compilation.
Also, move func udiv into package runtime.
That's where it is anyway,
and it lets udiv look and act like the rest of
the runtime support functions.
Change-Id: I41462a632c14fdc41f61b08049ec13cd80a87bfe
Reviewed-on: https://go-review.googlesource.com/41191
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
As of CL 39998, it is no longer necessary.
Fixes#19699
Change-Id: Ie1c49c8468073c6ddeb96c03668705cf81d40c98
Reviewed-on: https://go-review.googlesource.com/41051
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The dclstack is now a proper stack and thus we can implement it
using a slice rather than a linked list.
Change-Id: I200e85621ff76c111bdeb7eb382fd82da438f3ba
Reviewed-on: https://go-review.googlesource.com/41135
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
- PkgMap was only needed to test import/export in a "cleanroom"
environment, with debugFormat set. Provided helper function
instead.
- PkgList was only used to identify directly imported packages.
Instead, compute that list explicitly from the package map.
It happens only once, the list is small, and it's more robust
than keeping two data structures in sync.
Change-Id: I82dce3c0b5cb816faae58708e877799359c20fcb
Reviewed-on: https://go-review.googlesource.com/41078
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
There's already special code to access it.
Change-Id: I28ca4f44a04262407ee9f1c826ada4e7eba44775
Reviewed-on: https://go-review.googlesource.com/41073
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
That's where it belongs. Also, moved pkgMap and pkgs globals.
Change-Id: I531727fe5ce162c403efefec82f4cc90afa326d7
Reviewed-on: https://go-review.googlesource.com/41071
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
goobj.importPathToPrefix is 3x faster than gc.pathToPrefix so rename and
move it to cmd/internal/objabi which is already imported by both goobj and
gc.
Change-Id: I10eda5bce95ef6d5d888818c5c47258c2833ea45
Reviewed-on: https://go-review.googlesource.com/40875
Run-TryBot: Todd Neal <todd@tneal.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The last use of condOps was removed in c644a76.
Change-Id: I5383d0e7a9078fc17ca12ed032ecf8e7f4aa95d7
Reviewed-on: https://go-review.googlesource.com/41030
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Replace derecursed postorder computation with one that
mimics DFS traversal.
Corrected outerinner function in loopfinder
Leave enhanced checks in place.
Change-Id: I657ba5e89c88941028d6d4c72e9f9056e30f1ce8
Reviewed-on: https://go-review.googlesource.com/40872
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Follow-up on https://go-review.googlesource.com/#/c/39998/
which dropped this information.
The reported blocks are the innermost blocks containing a
label jumped to from outside, not the outermost block as
reported originally by cmd/compile.
We could report the outermost block with a slighly more
involved algorithm (need to track containing blocks for
all unresolved forward gotos), but since gccgo also reports
the innermost blocks, the current approach seems good enough.
Change-Id: Ic0235b8fafe8d5f99dc9872b58e90e8d9e72c5db
Reviewed-on: https://go-review.googlesource.com/40980
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Marvin Stenger <marvin.stenger94@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Instead of a separate check control flow pass (checkcfg.go)
operating on nodes, perform this check at parse time on the
new syntax tree. Permits this check to be done concurrently,
and doesn't depend on the specifics of the symbol's dclstack
implementation anymore. The remaining dclstack uses will be
removed in a follow-up change.
- added CheckBranches Mode flag (so we can turn off the check
if we only care about syntactic correctness, e.g. for tests)
- adjusted test/goto.go error messages: the new branches
checker only reports if a goto jumps into a block, but not
which block (we may want to improve this again, eventually)
- also, the new branches checker reports one variable that
is being jumped over by a goto, but it may not be the first
one declared (this is fine either way)
- the new branches checker reports additional errors for
fixedbugs/issue14006.go (not crucial to avoid those errors)
- the new branches checker now correctly reports only
variable declarations being jumped over, rather than
all declarations (issue 8042). Added respective tests.
Fixes#8042.
Change-Id: I53b6e1bda189748e1e1fb5b765a8a64337c27d40
Reviewed-on: https://go-review.googlesource.com/39998
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Now only cmd/asm and cmd/compile depend on cmd/internal/obj. Changing
the assembler backends no longer requires reinstalling cmd/link or
cmd/addr2line.
There's also now one canonical definition of the object file format in
cmd/internal/objabi/doc.go, with a warning to update all three
implementations.
objabi is still something of a grab bag of unrelated code (e.g., flag
and environment variable handling probably belong in a separate "tool"
package), but this is still progress.
Fixes#15165.
Fixes#20026.
Change-Id: Ic4b92fac7d0d35438e0d20c9579aad4085c5534c
Reviewed-on: https://go-review.googlesource.com/40972
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Automated refactoring using github.com/mdempsky/unbed (to rewrite
s.Foo to s.FuncInfo.Foo) and then gorename (to rename the FuncInfo
field to just Func).
Passes toolstash-check -all.
Change-Id: I802c07a1239e0efea058a91a87c5efe12170083a
Reviewed-on: https://go-review.googlesource.com/40670
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
A prior CL eliminated the last reference to Ctxt.Hash
from the compiler.
Change-Id: Ic97ff84ed1a14e0c93fb0e8ec0b2617c3397c0e8
Reviewed-on: https://go-review.googlesource.com/40699
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It was a bit weird to have it at the top of pgen.go.
This does half of the TODO at the top of the comment.
Change-Id: I65140fa05673b2dbb6feddb8c1877f6d624a7844
Reviewed-on: https://go-review.googlesource.com/40698
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The compiler handled gcargs and gclocals LSyms unusually.
It generated placeholder symbols (makefuncdatasym),
filled them in, and then renamed them for content-addressability.
This is an important binary size optimization;
the same locals information occurs over and over.
This CL continues to treat these LSyms unusually,
but in a slightly more explicit way,
and importantly for concurrent compilation,
in a way that does not require concurrent
modification of Ctxt.Hash.
Instead of creating gcargs and gclocals in the usual way,
by creating a types.Sym and then an obj.LSym,
we add them directly to obj.FuncInfo,
initialize them in obj.InitTextSym,
and deduplicate and add them to ctxt.Data at the end.
Then the backend's job is simply to fill them in
and rename them appropriately.
Updates #15756
name old alloc/op new alloc/op delta
Template 38.8MB ± 0% 38.7MB ± 0% -0.22% (p=0.016 n=5+5)
Unicode 29.8MB ± 0% 29.8MB ± 0% ~ (p=0.690 n=5+5)
GoTypes 113MB ± 0% 113MB ± 0% -0.24% (p=0.008 n=5+5)
SSA 1.25GB ± 0% 1.24GB ± 0% -0.39% (p=0.008 n=5+5)
Flate 25.3MB ± 0% 25.2MB ± 0% -0.43% (p=0.008 n=5+5)
GoParser 31.7MB ± 0% 31.7MB ± 0% -0.22% (p=0.008 n=5+5)
Reflect 78.2MB ± 0% 77.6MB ± 0% -0.80% (p=0.008 n=5+5)
Tar 26.6MB ± 0% 26.3MB ± 0% -0.85% (p=0.008 n=5+5)
XML 42.4MB ± 0% 41.9MB ± 0% -1.04% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
Template 378k ± 0% 377k ± 1% ~ (p=0.151 n=5+5)
Unicode 321k ± 1% 321k ± 0% ~ (p=0.841 n=5+5)
GoTypes 1.14M ± 0% 1.14M ± 0% -0.47% (p=0.016 n=5+5)
SSA 9.71M ± 0% 9.67M ± 0% -0.33% (p=0.008 n=5+5)
Flate 233k ± 1% 232k ± 1% ~ (p=0.151 n=5+5)
GoParser 316k ± 0% 315k ± 0% -0.49% (p=0.016 n=5+5)
Reflect 979k ± 0% 972k ± 0% -0.75% (p=0.008 n=5+5)
Tar 250k ± 0% 247k ± 1% -0.92% (p=0.008 n=5+5)
XML 392k ± 1% 389k ± 0% -0.67% (p=0.008 n=5+5)
Change-Id: Idc36186ca9d2f8214b5f7720bbc27b6bb22fdc48
Reviewed-on: https://go-review.googlesource.com/40697
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
They do basically the same work.
Setuintxx was only used in a single place,
so eliminate it in favor of WriteInt.
duintxxLSym's alignment rounding was not used in practice;
change it into alignment assertion.
Passes toolstash-check. No compiler performance changes.
Change-Id: I0f7410cf2ccffbdc02ad796eaf973ee6a83074f8
Reviewed-on: https://go-review.googlesource.com/40863
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It was added in 2013 in CL 7064048.
All uses of it in the compiler disappeared with
(or possibly before) the SSA backend.
Several releases have gone by without it,
from which I conclude that it is now not needed.
Change-Id: I2095f4ac05d4d7ab998168993a7fd5d954aeee88
Reviewed-on: https://go-review.googlesource.com/40856
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This comment was out of date since the bump to 80 done as the same time
as inlining transitive functions in:
commit 77ccb16eb1
Author: Russ Cox <rsc@golang.org>
Date: Tue Feb 24 12:19:01 2015 -0500
cmd/internal/gc: transitive inlining
Adjust the comment at the top of the file accordingly.
Change-Id: Ia6d7397c874e3b85396e82dc9678e56aab9ad728
Reviewed-on: https://go-review.googlesource.com/40910
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
On s390x unsigned integer comparisons with immediates require the immediate
to be an unsigned 32-bit integer. The rule was checking that the immediate
was a signed 32-bit integer.
This CL also adds a test for comparisons that could be turned into compare
with immediate or equivalent instructions (depending on architecture and
optimizations applied).
Fixes#19940.
Change-Id: Ifd6aa989fd3d50e282f7d30fec9db462c28422b1
Reviewed-on: https://go-review.googlesource.com/40433
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This avoids needing a mutex to protect stringsym,
and preserves a consistent ctxt.Data ordering
in the face of a concurrent backend.
Updates #15756
Change-Id: I775daae11db5db1269533a00f5249e3a03086ffc
Reviewed-on: https://go-review.googlesource.com/40509
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
To preserve reproducible builds, the text entries
during compilation will be sorted before being printed.
TestAssembly currently assumes that function init
comes after all user-defined functions.
Remove that assumption.
Instead of looking for "TEXT" to tell you where
a function ends--which may now yield lots of
non-function-code junk--look for a line beginning
with non-whitespace.
Updates #15756
Change-Id: Ibc82dba6143d769ef4c391afc360e523b1a51348
Reviewed-on: https://go-review.googlesource.com/39853
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Prior to this CL, flags such as NOSPLIT
on ATEXT Progs were stored in From3.Offset.
Some but not all of those flags were also
duplicated into From.Sym.Attribute.
This CL migrates all of those flags into
From.Sym.Attribute and stops creating a From3.
A side-effect of this is that printing an
ATEXT Prog can no longer simply dump From3.Offset.
That's kind of good, since the raw flag value
wasn't very informative anyway, but it did
necessitate a bunch of updates to the cmd/asm tests.
The reason I'm doing this work now is that
avoiding storing flags in both From.Sym and From3.Offset
simplifies some other changes to fix the data
race first described in CL 40254.
This CL almost passes toolstash-check -all.
The only changes are in cases where the assembler
has decided that a function's flags may be altered,
e.g. to make a function with no calls in it NOSPLIT.
Prior to this CL, that information was not printed.
Sample before:
"".Ctz64 t=1 size=63 args=0x10 locals=0x0
0x0000 00000 (/Users/josh/go/tip/src/runtime/internal/sys/intrinsics.go:35) TEXT "".Ctz64(SB), $0-16
0x0000 00000 (/Users/josh/go/tip/src/runtime/internal/sys/intrinsics.go:35) FUNCDATA $0, gclocals·f207267fbf96a0178e8758c6e3e0ce28(SB)
Sample after:
"".Ctz64 t=1 nosplit size=63 args=0x10 locals=0x0
0x0000 00000 (/Users/josh/go/tip/src/runtime/internal/sys/intrinsics.go:35) TEXT "".Ctz64(SB), NOSPLIT, $0-16
0x0000 00000 (/Users/josh/go/tip/src/runtime/internal/sys/intrinsics.go:35) FUNCDATA $0, gclocals·f207267fbf96a0178e8758c6e3e0ce28(SB)
Observe the additional "nosplit" in the first line
and the additional "NOSPLIT" in the second line.
Updates #15756
Change-Id: I5c59bd8f3bdc7c780361f801d94a261f0aef3d13
Reviewed-on: https://go-review.googlesource.com/40495
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
These patterns are the only uses of isArg and isAuto, and they all
follow a common pattern too. Extract out so that we can more easily
tweak the interface for isArg/isAuto.
Passes toolstash -cmp for linux/arm64.
Change-Id: I9c509dabdc123c93cb1ad2f34fe8c12a9f313f6d
Reviewed-on: https://go-review.googlesource.com/40490
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Also adjust truncfltlit to make it more similar to trunccmplxlit, and
make it report an error for bad Etypes.
Fixes#19947
Change-Id: I6684523e989c2293b8a8e85bd2bfb9c399c5ea36
Reviewed-on: https://go-review.googlesource.com/40453
Reviewed-by: Robert Griesemer <gri@golang.org>
When casting an ideal to complex{64,128}, for example during the
evaluation of
var a = complex64(0) / 1e-50
we want the compiler to report a division-by-zero error if a divisor
would be zero after the cast.
We already do this for floats; for example
var b = float32(0) / 1e-50
generates a 'division by zero' error at compile time (because
float32(1e-50) is zero, and the cast is done before performing the
division).
There's no such check in the path for complex{64,128} expressions, and
no cast is performed before the division in the evaluation of
var a = complex64(0) / 1e-50
which compiles just fine.
This patch changes the convlit1 function so that complex ideals
components (real and imag) are correctly truncated to float{32,64}
when doing an ideal -> complex{64, 128} cast.
Fixes#11674
Change-Id: Ic5f8ee3c8cfe4c3bb0621481792c96511723d151
Reviewed-on: https://go-review.googlesource.com/37891
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
A recent performance improvement for PPC64.rules introduced a
regression for the case where the size of a move is <= 8 bytes
and the value used in the offset field of the instruction is not
aligned correctly for the instruction. In the cases where this happened,
the assembler was not detecting the incorrect offset and still generated
the instruction even though it was invalid.
This fix changes the PPC64.rules for the moves that are now failing
to include the correct alignment checks, along some additional testcases
for gc/ssa for the failing alignments.
I will add a fix to the assembler to detect incorrect offsets in
another CL.
This fixes#19907
Change-Id: I3d327ce0ea6afed884725b1824f9217cef2fe6bf
Reviewed-on: https://go-review.googlesource.com/40290
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
Reviewed-by: David Chase <drchase@google.com>
Continues outside of a loop are not allowed. Most of these possibilities
were tested in label1.go, but one was missing - a plain continue in a
switch/select but no enclosing loop.
This used to error with a "continue not in loop" in 1.8, but recently
was broken by c03e75e5. In particular, innerloop does not only account
for loops, but also for switches and selects. Swap it by bools that
track whether breaks and continues should be allowed.
While at it, improve the wording of errors for breaks that are not where
they should be. Change "loop" by "loop, switch, or select" since they
can be used in any of those.
And add tests to make sure this isn't broken again. Use a separate func
since I couldn't get the compiler to crash on f() itself, possibly due
to the recursive call on itself.
Fixes#19934.
Change-Id: I8f09c6c2107fd95cac50efc2a8cb03cbc128c35e
Reviewed-on: https://go-review.googlesource.com/40357
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
sort.Slice was added in Go 1.8.
It's nice to use, and faster than sort.Sort,
so it'd be nice to be able to use it in the toolchain.
This CL adds obj.SortSlice, which is sort.Slice,
but with a slower fallback version for bootstrapping.
This CL also includes a single demo+test use.
Change-Id: I2accc60b61f8e48c8ab4f1a63473e3b87af9b691
Reviewed-on: https://go-review.googlesource.com/40114
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
These are never accessed.
Change-Id: I45975972d19d1f263f6545c9ed648511501094c6
Reviewed-on: https://go-review.googlesource.com/40315
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This makes the core Flushplist loop clearer.
We may also want to move the Sym initialization
much earlier in the compiler (see discussion on
CL 40254), for which this paves the way.
While we're here, eliminate package log in favor of ctxt.Diag.
Passes toolstash-check -all.
Updates #15756
Change-Id: Ieaf848d196764a5aa82578b689af7bc6638c385a
Reviewed-on: https://go-review.googlesource.com/40313
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
This avoids false positives
like those found in #19880.
Fixes#19880
Change-Id: I583c16cc3c71e7462a72500db9ea2547c468f8c1
Reviewed-on: https://go-review.googlesource.com/40255
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reduces the number of cases that need to be tested and
reduces size of the evconst function by 101 bytes.
Change-Id: Ie56055a89d0dadd311fb940b51c488fc003694b9
Reviewed-on: https://go-review.googlesource.com/39950
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
pkgByPath was added in d78c84c4 to eliminate the differences between the
export formats around the time of Go 1.7.
The last remnants of the textual export format was removed by Josh in
39850 making the pkgByPath sorting type unused.
Change-Id: I168816d6401f45119475a4fe5ada00d9ce571a9e
Reviewed-on: https://go-review.googlesource.com/40050
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This is a re-roll of CL 39710,
which broke deterministic builds.
typenamesym is called from three places:
typename, ngotype, and Type.Symbol.
Only in typename do we actually need a Node.
ngotype and Type.Symbol require only a Sym.
And writing the newly created Node to
Sym.Def is unsafe in a concurrent backend.
Rather than use a mutex protect to Sym.Def,
make typenamesym not touch Sym.Def.
The assignment to Sym.Def was serving a second purpose,
namely to prevent duplicate entries on signatlist.
Preserve that functionality by switching signatlist to a map.
This in turn requires that we sort signatlist
when exporting it, to preserve reproducibility.
We sort using exactly the same mechanism
that the export code (dtypesym) uses.
Failure to do that led to non-deterministic builds (#19872).
Since we've already calculated the Type's export name,
we could pass it to dtypesym, sparing it a bit of work.
That can be done as a future optimization.
Updates #15756
name old alloc/op new alloc/op delta
Template 39.2MB ± 0% 39.3MB ± 0% ~ (p=0.075 n=10+10)
Unicode 29.8MB ± 0% 29.8MB ± 0% ~ (p=0.393 n=10+10)
GoTypes 113MB ± 0% 113MB ± 0% +0.06% (p=0.027 n=10+8)
SSA 1.25GB ± 0% 1.25GB ± 0% +0.05% (p=0.000 n=8+10)
Flate 25.3MB ± 0% 25.3MB ± 0% ~ (p=0.105 n=10+10)
GoParser 31.7MB ± 0% 31.8MB ± 0% ~ (p=0.165 n=10+10)
Reflect 78.2MB ± 0% 78.2MB ± 0% ~ (p=0.190 n=10+10)
Tar 26.6MB ± 0% 26.6MB ± 0% ~ (p=0.481 n=10+10)
XML 42.2MB ± 0% 42.2MB ± 0% ~ (p=0.968 n=10+9)
name old allocs/op new allocs/op delta
Template 384k ± 1% 386k ± 1% +0.43% (p=0.019 n=10+10)
Unicode 320k ± 0% 321k ± 0% +0.36% (p=0.015 n=10+10)
GoTypes 1.14M ± 0% 1.14M ± 0% +0.33% (p=0.000 n=10+8)
SSA 9.69M ± 0% 9.71M ± 0% +0.18% (p=0.000 n=10+9)
Flate 233k ± 1% 233k ± 1% ~ (p=0.481 n=10+10)
GoParser 315k ± 1% 316k ± 1% ~ (p=0.113 n=9+10)
Reflect 979k ± 0% 979k ± 0% ~ (p=0.971 n=10+10)
Tar 250k ± 1% 250k ± 1% ~ (p=0.481 n=10+10)
XML 391k ± 1% 392k ± 0% ~ (p=1.000 n=10+9)
Change-Id: Ia9f21cc29c047021fa8a18c2a3d861a5146aefac
Reviewed-on: https://go-review.googlesource.com/39915
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Given code such as
type T struct {
_ string
}
func f() {
var x = T{"space"}
// ...
}
the compiler rewrote the 'var x' line as
var x T
x._ = "space"
The compiler then rejected the assignment to
a blank field, thus rejecting valid code.
It also failed to catch a number of invalid assignments.
And there were insufficient checks for validity
when emitting static data, leading to ICEs.
To fix, check earlier for explicit blanks field names,
explicitly handle legit blanks in sinit,
and don't try to emit static data for nodes
for which typechecking has failed.
Fixes#19482
Change-Id: I594476171d15e6e8ecc6a1749e3859157fe2c929
Reviewed-on: https://go-review.googlesource.com/38006
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This dowidth currently happens during AST to SSA conversion.
As such, it is a concurrency pinch point.
It's a bit silly, but do it here in walk instead.
This appears (fingers crossed) to be the last
unresolved dowidth concurrency problem.
Updates #15756
Change-Id: I87cbf718a14ad21aca74586003d79320cca75953
Reviewed-on: https://go-review.googlesource.com/39994
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
queuemethod was unused. As queuemethod is unused, nothing appends to the
methodqueue global. As methodqueue is always nil or empty, there are no
live callers of domethod, so it can be removed.
Change-Id: Ic7427ac4621bbf403947815e3988c3a1113487f2
Reviewed-on: https://go-review.googlesource.com/39931
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
I had too many failed attempts trying to remove iterFields that I
decided to overhaul this function. Much simpler and easier to
understand now (at least IMO).
Passes toolstash-check -all.
Change-Id: I41d00642a969698df3f4689e41a386346b966638
Reviewed-on: https://go-review.googlesource.com/39856
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This reverts commit c8b889cc48.
Reason for revert: broke noopt build, compiler performance regression, new Curfn uses
Let's fix those and then try this again.
Change-Id: Icc3cad1365d04cac8fd09da9dbb0bbf55c13ef44
Reviewed-on: https://go-review.googlesource.com/39991
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Change compiler and linker to emit DWARF lexical blocks in debug_info.
Version of debug_info is updated from DWARF v.2 to DWARF v.3 since version 2
does not allow lexical blocks with discontinuous ranges.
Second attempt at https://go-review.googlesource.com/#/c/29591/
Remaining open problems:
- scope information is removed from inlined functions
- variables in debug_info do not have DW_AT_start_scope attributes so a
variable will shadow other variables with the same name as soon as its
containing scope begins, before its declaration.
Updates golang/go#12899, golang/go#6913
Change-Id: I0e260a45b564d14a87b88974eb16c5387cb410a5
Reviewed-on: https://go-review.googlesource.com/36879
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
CL 38662 changed the x86 assembler to be eagerly
initialized, for a concurrent backend.
This CL puts in place a proper mechanism for doing so,
and switches all architectures to use it.
Passes toolstash-check -all.
Updates #15756
Change-Id: Id2aa527d3a8259c95797d63a2f0d1123e3ca2a1c
Reviewed-on: https://go-review.googlesource.com/39917
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL 38776 was not updated to use the new types package.
Fixes build.
Change-Id: Ie80ff4837cac95bd628e0405a937045171d56e0c
Reviewed-on: https://go-review.googlesource.com/39918
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently we expand comparison with small constant strings into len check
and a sequence of byte comparisons. Generate 16/32/64-bit comparisons,
instead of bytewise on 386 and amd64. Also increase limits on what is
considered small constant string.
Shaves ~30kb (0.5%) from go executable.
This also updates test/prove.go to keep test case valid.
Change-Id: I99ae8871a1d00c96363c6d03d0b890782fa7e1d9
Reviewed-on: https://go-review.googlesource.com/38776
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
They are in the types package, no need to mention the Type suffix.
Change-Id: Ie4fe1e3c1793514145e33f9df373d715f63e1aad
Reviewed-on: https://go-review.googlesource.com/39911
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
- created new package cmd/compile/internal/types
- moved Pkg, Sym, Type to new package
- to break cycles, for now we need the (ugly) types/utils.go
file which contains a handful of functions that must be installed
early by the gc frontend
- to break cycles, for now we need two functions to convert between
*gc.Node and *types.Node (the latter is a dummy type)
- adjusted the gc's code to use the new package and the conversion
functions as needed
- made several Pkg, Sym, and Type methods functions as needed
- renamed constructors typ, typPtr, typArray, etc. to types.New,
types.NewPtr, types.NewArray, etc.
Passes toolstash-check -all.
Change-Id: I8adfa5e85c731645d0a7fd2030375ed6ebf54b72
Reviewed-on: https://go-review.googlesource.com/39855
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Followup to previous typenod CL. Changes export data format, but only
the compiler-specific section, so no version bump.
Change-Id: I0c21737141f3d257366b29b2a9211bc7217c39ee
Reviewed-on: https://go-review.googlesource.com/39797
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The textual import/export format is ancient history.
Change-Id: Iebe90bfd9bd3074eb191186d86e5f4286ce3b1f3
Reviewed-on: https://go-review.googlesource.com/39850
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
typenamesym is called from three places:
typename, ngotype, and Type.Symbol.
Only in typename do we actually need a Node.
ngotype and Type.Symbol require only a Sym.
And writing the newly created Node to
Sym.Def is unsafe in a concurrent backend.
Rather than use a mutex protect to Sym.Def,
make typenamesym not touch Sym.Def.
The assignment to Sym.Def was serving a second purpose,
namely to prevent duplicate entries on signatlist.
Preserve that functionality by switching signatlist to a map.
This in turn requires that we sort signatlist
when exporting it, to preserve reproducibility.
We'd like to use Type.cmp for sorting,
but that causes infinite recursion at the moment;
see #19869.
For now, use Type.LongString as the sort key,
which is a complete description of the type.
Type.LongString is relatively expensive,
but we calculate it only once per type,
and signatlist is generally fairly small,
so the performance impact is minimal.
Updates #15756
name old alloc/op new alloc/op delta
Template 39.4MB ± 0% 39.4MB ± 0% ~ (p=0.222 n=5+5)
Unicode 29.8MB ± 0% 29.8MB ± 0% ~ (p=0.151 n=5+5)
GoTypes 113MB ± 0% 113MB ± 0% ~ (p=0.095 n=5+5)
SSA 1.25GB ± 0% 1.25GB ± 0% +0.04% (p=0.008 n=5+5)
Flate 25.3MB ± 0% 25.4MB ± 0% ~ (p=0.056 n=5+5)
GoParser 31.8MB ± 0% 31.8MB ± 0% ~ (p=0.310 n=5+5)
Reflect 78.3MB ± 0% 78.3MB ± 0% ~ (p=0.690 n=5+5)
Tar 26.7MB ± 0% 26.7MB ± 0% ~ (p=0.548 n=5+5)
XML 42.2MB ± 0% 42.2MB ± 0% ~ (p=0.222 n=5+5)
name old allocs/op new allocs/op delta
Template 387k ± 0% 388k ± 0% ~ (p=0.056 n=5+5)
Unicode 320k ± 0% 321k ± 0% +0.32% (p=0.032 n=5+5)
GoTypes 1.14M ± 0% 1.15M ± 0% ~ (p=0.095 n=5+5)
SSA 9.70M ± 0% 9.72M ± 0% +0.18% (p=0.008 n=5+5)
Flate 234k ± 0% 235k ± 0% +0.60% (p=0.008 n=5+5)
GoParser 317k ± 0% 317k ± 0% ~ (p=1.000 n=5+5)
Reflect 982k ± 0% 983k ± 0% ~ (p=0.841 n=5+5)
Tar 252k ± 1% 252k ± 0% ~ (p=0.310 n=5+5)
XML 393k ± 0% 392k ± 0% ~ (p=0.548 n=5+5)
Change-Id: I53a3b95d19cf1a7b7511a94fba896706addf84fb
Reviewed-on: https://go-review.googlesource.com/39710
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is a straightforward refactoring,
to reduce the scope of upcoming changes.
The symbol size and AttrLocal=true was not
set universally, but it appears not to matter,
since toolstash -cmp is happy.
Passes toolstash-check -all.
Change-Id: I7f8392f939592d3a1bc6f61dec992f5661f42fca
Reviewed-on: https://go-review.googlesource.com/39791
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It was simply a wrapper around Link.Lookup.
Unwrap everything.
CL prepared using eg with template:
package p
import "cmd/internal/obj"
func before(ctxt *obj.Link, name string, version int) *obj.LSym {
return obj.Linklookup(ctxt, name, version)
}
func after(ctxt *obj.Link, name string, version int) *obj.LSym {
return ctxt.Lookup(name, version)
}
Then one comment in cmd/asm/internal/asm/parse.go
was manually updated (and gofmt'ed!),
and func Linklookup deleted.
Passes toolstash-check (as a sanity measure).
Change-Id: Icc4d56b0b2b5c8888d3184c1898c48359ea1e638
Reviewed-on: https://go-review.googlesource.com/39715
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
For an AND that masks out leading or trailing bits, generic rules
rewrite it to a pair of shifts. On ARM64, the mask actually can
fit into an AND instruction. So we rewrite it back to AND.
Fixes#19857.
Change-Id: I479d7320ae4f29bb3f0056d5979bde4478063a8f
Reviewed-on: https://go-review.googlesource.com/39651
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This is a holdover from the days when we did not
have full SSA coverage and compiled things optimistically,
and catching the panic obscures useful information.
Change-Id: I196790cb6b97419d92b318a2dfa7f1e1097cefb7
Reviewed-on: https://go-review.googlesource.com/39534
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Popcount instructions on amd64 are not guaranteed to be
present, so we must guard their call. Rewrite rules can't
generate control flow at the moment, so the intrinsifier
needs to generate that code.
name old time/op new time/op delta
OnesCount-8 2.47ns ± 5% 1.04ns ± 2% -57.70% (p=0.000 n=10+10)
OnesCount16-8 1.05ns ± 1% 0.78ns ± 0% -25.56% (p=0.000 n=9+8)
OnesCount32-8 1.63ns ± 5% 1.04ns ± 2% -35.96% (p=0.000 n=10+10)
OnesCount64-8 2.45ns ± 0% 1.04ns ± 1% -57.55% (p=0.000 n=6+10)
Update #18616
Change-Id: I4aff2cc9aa93787898d7b22055fe272a7cf95673
Reviewed-on: https://go-review.googlesource.com/38320
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Note that this is a redo of an undo of the original buggy CL 38666.
We have lots of rewrite rules that vary only in the fact that
we have 2 versions for the 2 different orderings of various
commuting ops. For example:
(ADDL x (MOVLconst [c])) -> (ADDLconst [c] x)
(ADDL (MOVLconst [c]) x) -> (ADDLconst [c] x)
It can get unwieldly quickly, especially when there is more than
one commuting op in a rule.
Our existing "fix" for this problem is to have rules that
canonicalize the operations first. For example:
(Eq64 x (Const64 <t> [c])) && x.Op != OpConst64 -> (Eq64 (Const64 <t> [c]) x)
Subsequent rules can then assume if there is a constant arg to Eq64,
it will be the first one. This fix kinda works, but it is fragile and
only works when we remember to include the required extra rules.
The fundamental problem is that the rule matcher doesn't
know anything about commuting ops. This CL fixes that fact.
We already have information about which ops commute. (The register
allocator takes advantage of commutivity.) The rule generator now
automatically generates multiple rules for a single source rule when
there are commutative ops in the rule. We can now drop all of our
almost-duplicate source-level rules and the canonicalization rules.
I have some CLs in progress that will be a lot less verbose when
the rule generator handles commutivity for me.
I had to reorganize the load-combining rules a bit. The 8-way OR rules
generated 128 different reorderings, which was causing the generator
to put too much code in the rewrite*.go files (the big ones were going
from 25K lines to 132K lines). Instead I reorganized the rules to
combine pairs of loads at a time. The generated rule files are now
actually a bit (5%) smaller.
Make.bash times are ~unchanged.
Compiler benchmarks are not observably different. Probably because
we don't spend much compiler time in rule matching anyway.
I've also done a pass over all of our ops adding commutative markings
for ops which hadn't had them previously.
Fixes#18292
Change-Id: Ic1c0e43fbf579539f459971625f69690c9ab8805
Reviewed-on: https://go-review.googlesource.com/38801
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Instead of walking the list of nodes twice,
once to find static entries to add to an array
and once to find dynamic entries to generate code for,
do the split once up front, into two slices.
Then process each slice individually.
This makes the code easier to read
and more importantly, easier to modify.
While we're here, add a TODO to avoid
using temporaries for mapassign_fast calls.
It's not an important TODO;
the generated code would be basically identical.
It would just avoid a minor amount of
pointless SSA optimization work.
Passes toolstash-check.
No measureable compiler performance impact.
Updates #19751
Change-Id: I84a8f2c22f9025c718ef34639059d7bd02a3c406
Reviewed-on: https://go-review.googlesource.com/39351
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This triggers 119 times during make.bash.
This CL reduces the time it takes for the
compiler to panic while compiling the code in #19751
from 22 minutes to 15 minutes. Yay, I guess.
Updates #19751
Change-Id: I8ca7f1ae75f89d1eb2a361d67b3055a975221734
Reviewed-on: https://go-review.googlesource.com/39294
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Old buggy hardware incorrectly executes the shift-left-K
then shift-right-K idiom for clearing K leftmost bits.
Use a right rotate instead of shift to avoid triggering the
bug.
Fixes#19809.
Change-Id: I6dc646b183c29e9d01aef944729f34388dcc687d
Reviewed-on: https://go-review.googlesource.com/39310
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Minor cleanup.
This is the only such instance in the compiler.
Change-Id: I4e8ecde57d71867c7e1ac4d17e2154a91dd262b0
Reviewed-on: https://go-review.googlesource.com/39209
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
No particular need for this,
but it's nice to enforce invariants
when they are available.
Change-Id: Ia6fa88dc4116f65dac2879509746e123e2c1862a
Reviewed-on: https://go-review.googlesource.com/39201
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
nodfp is a global, so modifying it is unsafe in a concurrent backend.
It is also not necessary, since the Used marks
are only relevant for nodes in fn.Dcl.
For good measure, mark nodfp as always used.
Passes toolstash-check.
Updates #15756
Change-Id: I5320459f5eced2898615a17b395a10c1064bcaf5
Reviewed-on: https://go-review.googlesource.com/39200
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Follow-up to review comments on CL 39193.
Change-Id: I7649af9d70ad73e039061a7a66fea416a7476192
Reviewed-on: https://go-review.googlesource.com/39199
Reviewed-by: Robert Griesemer <gri@golang.org>
A few gc.Node ops may be shared across functions.
The compiler is (mostly) already careful to avoid mutating them.
However, from a concurrency perspective, replacing (say)
an empty list with an empty list still counts as a mutation.
One place this occurs is orderinit. Avoid it.
This requires fixing one spot where shared nodes were mutated.
It doesn't result in any functional or performance changes.
Passes toolstash-check.
Updates #15756
Change-Id: I63c93b31baeeac62d7574804acb6b7f2bc9d14a9
Reviewed-on: https://go-review.googlesource.com/39196
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
The node in typenamesym requires neither
a position nor a curfn.
Passes toolstash-check.
Updates #15756
Change-Id: I6d39a8961e5578fe5924aaceb29045b6de2699df
Reviewed-on: https://go-review.googlesource.com/39194
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
newnamel is newname but with no dependency on lineno or Curfn.
This makes it suitable for use in a concurrent back end.
Use it now to make tempAt global-free.
The decision to push the assignment to n.Name.Curfn
to the caller of newnamel is based on mdempsky's
comments in #19683 that he'd like to do that
for callers of newname as well.
Passes toolstash-check. No compiler performance impact.
Updates #19683
Updates #15756
Change-Id: Idc461a1716916d268c9ff323129830d9a6e4a4d9
Reviewed-on: https://go-review.googlesource.com/39191
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Convert yyerrors into Fatals.
Remove the goto.
Move variable declaration closer to use.
Unify printing strings a bit.
Convert an int param into a bool.
Passes toolstash-check. No compiler performance impact.
Change-Id: I9017681417b785cf8693d18b124ac4f1ff37f2b5
Reviewed-on: https://go-review.googlesource.com/39170
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The names never occur more than once,
so interning the results is counterproductive.
The impact is not very big, but neither is the fix.
name old time/op new time/op delta
Unicode 90.2ms ± 3% 88.3ms ± 5% -2.10% (p=0.000 n=94+98)
Change-Id: I1e3a24433db4ae0c9a6e98166568941824ff0779
Reviewed-on: https://go-review.googlesource.com/39193
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Also: Fix (testdata/gen/) copyGen.go, zeroGen.go, and arithConstGen.go
to actually match (testdata/) copy.go, zero.go, and arithConst.go, all
of which were manually edited in https://go-review.googlesource.com/20823
and https://go-review.googlesource.com/22748 despite the 'do not edit'
(or perhaps because it was missing in the case of arithConst.go).
For #13560.
Change-Id: I366e1b521e51885e0d318ae848760e5e14ccd488
Reviewed-on: https://go-review.googlesource.com/39172
Reviewed-by: Rob Pike <r@golang.org>
Prior to this CL, the SSA backend reported violations
of the //go:nowritebarrier annotation immediately.
This necessitated emitting errors during SSA compilation,
which is not compatible with a concurrent backend.
Instead, check for such violations later.
We already save the data required to do a late check
for violations of the //go:nowritebarrierrec annotation.
Use the same data, and check //go:nowritebarrier at the same time.
One downside to doing this is that now only a single
violation will be reported per function.
Given that this is for the runtime only,
and violations are rare, this seems an acceptable cost.
While we are here, remove several 'nerrors != 0' checks
that are rendered pointless.
Updates #15756Fixes#19250 (as much as it ever can be)
Change-Id: Ia44c4ad5b6fd6f804d9f88d9571cec8d23665cb3
Reviewed-on: https://go-review.googlesource.com/38973
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
We don't support stack frames over 2GB.
Rather than detect this during backend compilation,
check for it at the end of compilation.
This is arguably a more accurate check anyway,
since it takes into account the full frame,
including local stack, arguments, and arch-specific
rounding, although it's unlikely anyone would ever notice.
Also, rather than reporting the error right away,
take note of it and report it later, at the top level.
This is not relevant now, but it will help with making
the backend concurrent, as the append to the list of
oversized functions can be cheaply protected by a plain mutex.
Updates #15756
Updates #19250
Change-Id: Id3fa21906616d62e9dc66e27a17fd5f83304e96e
Reviewed-on: https://go-review.googlesource.com/38972
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
By checking GOARM in ssa/gen/ARM.rules, each intermediate operator
can be implemented via different instruction serials.
It is up to the user to choose between compitability and efficiency.
The Bswap32(x) is optimized to REV(x) when GOARM >= 6.
The CTZ(x) is optimized to CLZ(RBIT x) when GOARM == 7.
Change-Id: Ie9ee645fa39333fa79ad84ed4d1cefac30422814
Reviewed-on: https://go-review.googlesource.com/35610
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The symExport flag tells whether a symbol is in the export list
already or not (and it's also used to avoid being added to that
list). Exporting is based on that export list - no need to check
again.
Change-Id: I6056f97aa5c24a19376957da29199135c8da35f9
Reviewed-on: https://go-review.googlesource.com/39033
Reviewed-by: Dave Cheney <dave@cheney.net>
Remove one of the many lookup variants.
Change-Id: I4095aa030da4227540badd6724bbf50b728fbe93
Reviewed-on: https://go-review.googlesource.com/38990
Reviewed-by: Dave Cheney <dave@cheney.net>
Instead, add a scratchFpMem field to ssafn,
so that it may be passed on to genssa.
Updates #15756
Change-Id: Icdeae290d3098d14d31659fa07a9863964bb76ed
Reviewed-on: https://go-review.googlesource.com/38728
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
During the review of CL 38801 it was noted that it would be nice
to have a bit more clarity on how-and-why SB addressing is handled
strangely on s390x. This additional comment should hopefully help.
In general SB is handled differently because not all instructions
have variants that use relative addressing.
Change-Id: I3379012ae3f167478c191c435939c3b876c645ed
Reviewed-on: https://go-review.googlesource.com/38952
Reviewed-by: Keith Randall <khr@golang.org>
This is a better home for it.
Change-Id: I7ce96c16378d841613edaa53c07347b0ac99ea6e
Reviewed-on: https://go-review.googlesource.com/38970
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Even very large Types are not very big.
The haspointer cache looks like premature optimization.
Removing them has no detectable compiler performance impact,
and it removes mutable shared state used by the backend.
Updates #15756
Change-Id: I2d2cf03f470f5eef5bcd50ff693ef6a01d481700
Reviewed-on: https://go-review.googlesource.com/38912
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL 38147 eliminated package gc globals in formatting routines.
However, tconv still used the Type field Trecur
to avoid infinite recursion when formatting recursive
interfaces types such as (test/fixedbugs398.go):
type i1 interface {
F() interface {
i1
}
}
type i2 interface {
F() interface {
i2
}
}
This CL changes the recursion prevention to use a parameter,
and threads it through the formatting routines.
Because this fundamentally limits the embedding depth
of all types, it sets the depth limit to be much higher.
In practice, it is unlikely to impact any code at all,
one way or the other.
The remaining uses of Type.Trecur are boolean in nature.
A future CL will change Type.Trecur to be a boolean flag.
The removal of a couple of mode.Sprintf calls
makes this a very minor net performance improvement:
name old alloc/op new alloc/op delta
Template 40.0MB ± 0% 40.0MB ± 0% -0.13% (p=0.032 n=5+5)
Unicode 30.0MB ± 0% 29.9MB ± 0% ~ (p=0.310 n=5+5)
GoTypes 114MB ± 0% 113MB ± 0% -0.25% (p=0.008 n=5+5)
SSA 856MB ± 0% 855MB ± 0% -0.04% (p=0.008 n=5+5)
Flate 25.5MB ± 0% 25.4MB ± 0% -0.27% (p=0.008 n=5+5)
GoParser 31.9MB ± 0% 31.9MB ± 0% ~ (p=0.222 n=5+5)
Reflect 79.0MB ± 0% 78.6MB ± 0% -0.45% (p=0.008 n=5+5)
Tar 26.8MB ± 0% 26.7MB ± 0% -0.25% (p=0.032 n=5+5)
XML 42.4MB ± 0% 42.4MB ± 0% ~ (p=0.151 n=5+5)
name old allocs/op new allocs/op delta
Template 395k ± 0% 391k ± 0% -1.00% (p=0.008 n=5+5)
Unicode 321k ± 1% 319k ± 0% -0.56% (p=0.008 n=5+5)
GoTypes 1.16M ± 0% 1.14M ± 0% -1.61% (p=0.008 n=5+5)
SSA 7.63M ± 0% 7.60M ± 0% -0.30% (p=0.008 n=5+5)
Flate 239k ± 0% 234k ± 0% -1.94% (p=0.008 n=5+5)
GoParser 320k ± 0% 317k ± 1% -0.86% (p=0.008 n=5+5)
Reflect 1.00M ± 0% 0.98M ± 0% -2.17% (p=0.016 n=4+5)
Tar 255k ± 1% 251k ± 0% -1.35% (p=0.008 n=5+5)
XML 398k ± 0% 395k ± 0% -0.89% (p=0.008 n=5+5)
Updates #15756
Change-Id: Id23e647d347aa841f9a69d51f7d2d7d27b259239
Reviewed-on: https://go-review.googlesource.com/38797
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Now that we no longer generate dead code,
it is possible to follow block predecessors
into infinite loops with no variable definitions,
causing an infinite loop during phi insertion.
To fix that, check explicitly whether the predecessor
is dead in lookupVarOutgoing, and if so, bail.
The loop in lookupVarOutgoing is very hot code,
so I am wary of adding anything to it.
However, a long, CPU-only benchmarking run shows no
performance impact at all.
Fixes#19783
Change-Id: I8ef8d267e0b20a29b5cb0fecd7084f76c6f98e47
Reviewed-on: https://go-review.googlesource.com/38913
Reviewed-by: David Chase <drchase@google.com>
We use an "autogenerated" position in several places.
Rather than recreate it each time, make one early on and reuse it.
This removes the creation of new positions during the backend,
which was not concurrency-safe.
Updates #15756
Change-Id: Ic116b2e60f0e99de1a2ea87fe763831b50b645f8
Reviewed-on: https://go-review.googlesource.com/38915
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This reverts commit 041ecb697f.
Reason for revert: Not working on S390x and some 386 archs.
I have a guess why the S390x is failing. No clue on the 386 yet.
Revert until I can figure it out.
Change-Id: I64f1ce78fa6d1037ebe7ee2a8a8107cb4c1db70c
Reviewed-on: https://go-review.googlesource.com/38790
Reviewed-by: Keith Randall <khr@golang.org>
The uintptr-typed Data field in reflect.SliceHeader and
reflect.StringHeader needs special treatment because it is
really a pointer. Add the special treatment in walk for
bug #19168 to escape analysis.
Includes extra debugging that was helpful.
Fixes#19743.
Change-Id: I6dab5002f0d436c3b2a7cdc0156e4fc48a43d6fe
Reviewed-on: https://go-review.googlesource.com/38738
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Previously, an inlined call to wg.Done() in package main would have the
following incorrect symbol name:
main.(*sync.WaitGroup).Done
This change modifies methodname to return the correct symbol name:
sync.(*WaitGroup).Done
This fix was suggested by @mdempsky.
Fixes#19467.
Change-Id: I0117838679ac5353789299c618ff8c326712d94d
Reviewed-on: https://go-review.googlesource.com/37866
Reviewed-by: Austin Clements <austin@google.com>
We have lots of rewrite rules that vary only in the fact that
we have 2 versions for the 2 different orderings of various
commuting ops. For example:
(ADDL x (MOVLconst [c])) -> (ADDLconst [c] x)
(ADDL (MOVLconst [c]) x) -> (ADDLconst [c] x)
It can get unwieldly quickly, especially when there is more than
one commuting op in a rule.
Our existing "fix" for this problem is to have rules that
canonicalize the operations first. For example:
(Eq64 x (Const64 <t> [c])) && x.Op != OpConst64 -> (Eq64 (Const64 <t> [c]) x)
Subsequent rules can then assume if there is a constant arg to Eq64,
it will be the first one. This fix kinda works, but it is fragile and
only works when we remember to include the required extra rules.
The fundamental problem is that the rule matcher doesn't
know anything about commuting ops. This CL fixes that fact.
We already have information about which ops commute. (The register
allocator takes advantage of commutivity.) The rule generator now
automatically generates multiple rules for a single source rule when
there are commutative ops in the rule. We can now drop all of our
almost-duplicate source-level rules and the canonicalization rules.
I have some CLs in progress that will be a lot less verbose when
the rule generator handles commutivity for me.
I had to reorganize the load-combining rules a bit. The 8-way OR rules
generated 128 different reorderings, which was causing the generator
to put too much code in the rewrite*.go files (the big ones were going
from 25K lines to 132K lines). Instead I reorganized the rules to
combine pairs of loads at a time. The generated rule files are now
actually a bit (5%) smaller.
[Note to reviewers: check these carefully. Most of the other rule
changes are trivial.]
Make.bash times are ~unchanged.
Compiler benchmarks are not observably different. Probably because
we don't spend much compiler time in rule matching anyway.
I've also done a pass over all of our ops adding commutative markings
for ops which hadn't had them previously.
Fixes#18292
Change-Id: I999b1307272e91965b66754576019dedcbe7527a
Reviewed-on: https://go-review.googlesource.com/38666
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
It is currently possible in the compiler to create a struct type,
calculate the widths of types that depend on it,
and then alter the struct type.
transformclosure has local protection against this.
Protect against it at a deeper level.
This is preparation to call dowidth automatically,
rather than explicitly.
This is a re-roll of CL 38469.
Change-Id: Ic5b4baa250618504611fc57cbf51ab01d1eddf80
Reviewed-on: https://go-review.googlesource.com/38534
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Prior to this CL, Type.Width != 0 was the mark
of a Type whose Width had been calculated.
As a result, dowidth always recalculated
the width of struct{}.
This, combined with the prohibition on calculating
the width of a FuncArgsStruct and the use of
struct{} as a function argument,
meant that there were circumstances in which
it was forbidden to call dowidth on a type.
This inhibits refactoring to call dowidth automatically,
rather than explicitly.
Instead add a helper method, Type.WidthCalculated,
and implement as Type.Align > 0.
Type.Width is not a good candidate for tracking
whether the width has been calculated;
0 is a value type width, and Width is subject to
too much magic value game-playing.
For good measure, add a test for #11354.
Change-Id: Ie9a9fb5d924e7a2010c1904ae5e38ed4a38eaeb2
Reviewed-on: https://go-review.googlesource.com/38468
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
We don't need them any more since #15837 was fixed.
Fixes#19718
Change-Id: I13e46c62b321b2c9265f44c977b63bfb23163ca2
Reviewed-on: https://go-review.googlesource.com/38664
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Prior to this CL, instinit was called as needed.
This does not work well in a concurrent backend.
Initialization is very cheap; do it on startup instead.
Passes toolstash-check -all.
No compiler performance impact.
Updates #15756
Change-Id: Ifa5e82e8abf4504435e1b28766f5703a0555f42d
Reviewed-on: https://go-review.googlesource.com/38662
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Now that we have consistent use of xOrNil parse methods, we don't
need a special missing_statement singleton to distinguish between
missing actually statements and other errors (which have returned
nil before).
For #19663.
Change-Id: I8364f1441bdf8dd966bcd6d8219b2a42d6b88abd
Reviewed-on: https://go-review.googlesource.com/38656
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
- parser creates sensible nodes in case of syntax errors instead of nil
- a new BadExpr node is used in places where we can't do better
- fixed error message for incorrect type switch guard
- minor cleanups
Fixes#19663.
Change-Id: I028394c6db9cba7371f0e417ebf93f594659786a
Reviewed-on: https://go-review.googlesource.com/38653
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
ONAME, OLITERAL, and OTYPE nodes can be shared between functions.
In a concurrent backend, such nodes might be walked concurrently
with being read in other functions.
Arrange for them to be unmodified by walk.
This is a follow-up to CL 38609.
Passes toolstash-check.
Updates #15756
Change-Id: I03ff1d2c0ad81dafac3fd55caa218939cf7c0565
Reviewed-on: https://go-review.googlesource.com/38655
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Concurrent compilation requires providing an
explicit position and curfn to temp.
This implementation of tempAt temporarily
continues to use the globals lineno and Curfn,
so as not to collide with mdempsky's
work for #19683 eliminating the Curfn dependency
from func nod.
Updates #15756
Updates #19683
Change-Id: Ib3149ca4b0740e9f6eea44babc6f34cdd63028a9
Reviewed-on: https://go-review.googlesource.com/38592
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The type switch in walkexpr is giant.
Shrink it a little by coalescing identical cases
and removing some vertical whitespace.
No functional changes.
Passes toolstash-check.
Change-Id: I7f7efb4faae1f8657dfafac04585172f99d8b37d
Reviewed-on: https://go-review.googlesource.com/38652
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sym.Fsym is used only to avoid adding duplicate
entries to funcsyms, but that is easily
accomplished by detecting the first lookup
vs subsequent lookups of the func sym name.
This avoids creating an unnecessary ONAME node
during funcsym, which eliminates a dependency
in the backend on Curfn and lineno.
It also makes the code a lot simpler and clearer.
Updates #15756
Passes toolstash-check -all.
No compiler performance changes.
funcsymname does generate garbage via string
concatenation, but it is not called very much,
and this CL also eliminates allocation of several
Nodes and Names.
Change-Id: I7116c78fa39d975b7bd2c65a1d228749cf0dd46b
Reviewed-on: https://go-review.googlesource.com/38605
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Clean up code that does interface equality. Avoid doing checks
in efaceeq/ifaceeq that we already did before calling those routines.
No noticeable performance changes for existing benchmarks.
name old time/op new time/op delta
EfaceCmpDiff-8 604ns ± 1% 553ns ± 1% -8.41% (p=0.000 n=9+10)
Fixes#18618
Change-Id: I3bd46db82b96494873045bc3300c56400bc582eb
Reviewed-on: https://go-review.googlesource.com/38606
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
In the future, walk will probably run concurrently
with SSA construction. It is possible for walk
to be walking a function node that is referred
to by another function undergoing SSA construction.
In that case, this particular assignment to n.Type
is race-y.
This assignment is also not necessary;
evconst does not change the type of n.
Both arguments to evconst must have the same type,
and at the end of evconst, n is replaced with n.Left.
Remove the assignment, and add a check to ensure
that its removal remains correct.
Updates #15756
Change-Id: Id95faaff42d5abd76be56445d1d3e285775de8bf
Reviewed-on: https://go-review.googlesource.com/38609
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Avoid construction of incorrect syntax trees in presence of errors.
For #19663.
Change-Id: I43025a3cf0fe02cae9a57e8bb9489b5f628c3fd7
Reviewed-on: https://go-review.googlesource.com/38604
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Simple phi insertion already had a heuristic to check
for dead blocks, namely having no predecessors.
When we stopped generating code for dead blocks,
we eliminated some values contained in more subtle
dead blocks, which confused phi insertion.
Compensate by beefing up the reachability check.
Fixes#19678
Change-Id: I0081e4a46f7ce2f69b131a34a0553874a0cb373e
Reviewed-on: https://go-review.googlesource.com/38602
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
CL 37499 allows inlining more functions by ignoring dead code.
However, that dead code can contain non-exportable constructs.
Teach the exporter not to export dead code.
Fixes#19679
Change-Id: Idb1d3794053514544b6f1035d29262aa6683e1e7
Reviewed-on: https://go-review.googlesource.com/38601
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Almost never happens in practice.
The compiler will generate reasonable code anyway,
since assignments involving [0]T never do any work.
Fixes#19696Fixes#19671
Change-Id: I350d2e0c5bb326c4789c74a046ab0486b2cee49c
Reviewed-on: https://go-review.googlesource.com/38599
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We already handle n << (uint64(c)&63).
This change also handles n << (uint8(c)&63)
where the SSA compiler promotes the counter to 32 bits.
Fixes#19681
Change-Id: I9327d64a994286aa0dbf76eb995578880be6923a
Reviewed-on: https://go-review.googlesource.com/38550
Run-TryBot: Alexandru Moșoi <alexandru@mosoi.ro>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Pass around the imported package explicitly instead of relying
on a global variable.
Unfortunately we still need a global variable to communicate to
the typechecker that we're in an import, but the semantic load
is significantly reduced as it's just a bool, set/reset in a
couple of places only.
Change-Id: I4ebeae4064eb76ca0c4e2a15e4ca53813f005c29
Reviewed-on: https://go-review.googlesource.com/38595
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
First step towards removing global var importpkg.
Change-Id: Ifdda7c295e5720a7ff2da9baea17f03f190d48fa
Reviewed-on: https://go-review.googlesource.com/38594
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It is currently possible in the compiler to create a struct type,
calculate the widths of types that depend on it,
and then alter the struct type.
transformclosure has local protection against this.
Protect against it at a deeper level.
This is preparation to call dowidth automatically,
rather than explicitly.
Change-Id: Ic1578ca014610197cfe54a9f4d044d122a7217e8
Reviewed-on: https://go-review.googlesource.com/38469
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
In livenessepilogue, if we save liveness information for instructions
before updating liveout, we can avoid an extra bitvector temporary and
some extra copying around.
Passes toolstash-check -all.
Change-Id: I10d5803167ef3eba2e9e95094adc7e3d33929cc7
Reviewed-on: https://go-review.googlesource.com/38408
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
CL 38337 modified canMergeLoad to reject loads with multiple uses
so it is no longer necessary to check this in the SSA rules.
Change-Id: I03498390e778da1be8cb59ae0948e99289008315
Reviewed-on: https://go-review.googlesource.com/38473
Run-TryBot: Michael Munday <munday@ca.ibm.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Introduce a new type, gc.Progs, to manage
generation of Progs for a function.
Use it to replace globals pc and pcloc.
Passes toolstash-check -all.
Updates #15756
Change-Id: I2206998d7c58fe2a76b620904909f2e1cec8a57d
Reviewed-on: https://go-review.googlesource.com/38418
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Regalloc uses loop depths - make sure they are initialized!
Test to make sure we aren't pushing spills into loops.
This fixes a generated-code performance bug introduced with
the better spill placement change:
https://go-review.googlesource.com/c/34822/
Update #19595
Change-Id: Ib9f0da6fb588503518847d7aab51e569fd3fa61e
Reviewed-on: https://go-review.googlesource.com/38434
Reviewed-by: David Chase <drchase@google.com>
We want to merge a load and op into a single instruction
l = LOAD ptr mem
y = OP x l
into
y = OPload x ptr mem
However, all of our OPload instructions require that y uses
the same register as x. If x is needed past this instruction, then
we must copy x somewhere else, losing the whole benefit of merging
the instructions in the first place.
Disable this optimization if x is live past the OP.
Also disable this optimization if the OP is in a deeper loop than the load.
Update #19595
Change-Id: I87f596aad7e91c9127bfb4705cbae47106e1e77a
Reviewed-on: https://go-review.googlesource.com/38337
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
exporter.nesting was added in c7b9bd74 to mitigate #16369 which was
closed in ee272bbf. Remove the exporter.nesting field as it is now unused.
Change-Id: I07873d1a07d6a08b11994b817a1483ffc2f5e45f
Reviewed-on: https://go-review.googlesource.com/38490
Run-TryBot: Dave Cheney <dave@cheney.net>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This simplifies the code and removes a premature optimization.
It increases the amount of allocated syntax.Node space by ~0.4%
for parsing all of std lib, which is negligible.
Before the change (best of 5 runs):
$ go test -run StdLib -fast
parsed 1517022 lines (3394 files) in 793.487886ms (1911840 lines/s)
allocated 387.086Mb (267B/line, 487.828Mb/s)
After the change (best of 5 runs):
$ go test -run StdLib -fast
parsed 1516911 lines (3392 files) in 805.028655ms (1884294 lines/s)
allocated 388.466Mb (268B/line, 482.549Mb/s)
Change-Id: Id19d6210fdc62393862ba3b04913352d95c599be
Reviewed-on: https://go-review.googlesource.com/38439
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This change adds position information for { and } braces in the
source. There's a 1.9% increase in memory use for syntax.Nodes,
which is negligible relative to overall compiler memory consumption.
Parsing the std library (using syntax package only) and memory
consumption before this change (fastest of 5 runs):
$ go test -run StdLib -fast
parsed 1516827 lines (3392 files) in 780.612335ms (1943124 lines/s)
allocated 379.903Mb (486.673Mb/s)
After this change (fastest of 5 runs):
$ go test -run StdLib -fast
parsed 1517022 lines (3394 files) in 793.487886ms (1911840 lines/s)
allocated 387.086Mb (267B/line, 487.828Mb/s)
While not an exact apples-to-apples comparison (the syntax package
has changed and is also parsed), the overall impact is small.
Also: Small improvements to nodes_test.go.
Change-Id: Ib8a7f90bbe79de33d83684e33b1bf8dbc32e644a
Reviewed-on: https://go-review.googlesource.com/38435
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Starting in go1.9, the minimum processor requirement for ppc64 is POWER8.
Therefore, the checks for OldArch and the code enabled by it are not necessary
anymore.
Updates #19074
Change-Id: I33d6a78b2462c80d57c5dbcba2e13424630afab4
Reviewed-on: https://go-review.googlesource.com/38404
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
To enable this, inline the call to nod and simplify.
Eliminates a reference to lineno from the backend.
Passes toolstash-check -all.
Updates #15756
Change-Id: I9c4bd77d10d727aa8f5e6c6bb16b0e05de165631
Reviewed-on: https://go-review.googlesource.com/38441
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Must have been lost when rebasing the SSA liveness CLs.
Change-Id: Iaac33158cc7c92ea44a023c242eb914a7d6979c6
Reviewed-on: https://go-review.googlesource.com/38427
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
I think this got lost in a rebase somewhere.
Updates #15756
Change-Id: Ia3e7c60d1b9254f2877217073732b46c91059ade
Reviewed-on: https://go-review.googlesource.com/38425
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
During AllocFrame, we drop unused variables from Curfn.Func.Dcl, but
there might still be OpVarFoo instructions that reference those
variables. This wasn't a problem though because gvardefx used to emit
ANOP for unused variables instead of AVARFOO.
As an easy fix, if we see OpVarFoo (or OpKeepAlive) referencing an
unused variable, we can ignore it.
Fixes#19632.
Change-Id: I4e9ffabdb4058f7cdcc4663b540f5a5a692daf8b
Reviewed-on: https://go-review.googlesource.com/38400
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The chanrecv funcs don't use it at all. The chansend ones do, but the
element type is now part of the hchan struct, which is already a
parameter.
hchan can be nil in chansend when sending to a nil channel, so when
instrumenting we must copy to the stack to be able to read the channel
type.
name old time/op new time/op delta
ChanUncontended 6.42µs ± 1% 6.22µs ± 0% -3.06% (p=0.000 n=19+18)
Initially found by github.com/mvdan/unparam.
Fixes#19591.
Change-Id: I3a5e8a0082e8445cc3f0074695e3593fd9c88412
Reviewed-on: https://go-review.googlesource.com/38351
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Removing stray xori that came from big endian copy/paste.
Adding atomicand8 check to runtime.check() that would have revealed
this error.
Might fix#19396.
Change-Id: If8d6f25d3e205496163541eb112548aa66df9c2a
Reviewed-on: https://go-review.googlesource.com/38257
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Prior to this CL, the function's position was used.
The dottype Node's position is clearly better.
I'm not thrilled about introducing a reference to
lineno in the middle of SSA construction;
I will have to remove it later.
My immediate goal is stability and correctness of positions,
though, since that aids refactoring, so this is an improvement.
An example from package io:
func (t *multiWriter) WriteString(s string) (n int, err error) {
var p []byte // lazily initialized if/when needed
for _, w := range t.writers {
if sw, ok := w.(stringWriter); ok {
n, err = sw.WriteString(s)
The w.(stringWriter) type assertion includes loading
the address of static type data for stringWriter:
LEAQ type."".stringWriter(SB), R10
Prior to this CL, this instruction was given the line number
of the function declaration.
After this CL, this instruction is given the line number
of the type assertion itself.
Change-Id: Ifcca274b581a5a57d7e3102c4d7b7786bf307210
Reviewed-on: https://go-review.googlesource.com/38389
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Tested by fixedbugs/issue3705.go.
This removes a dependency on lineno
from near the backend.
Change-Id: I228bd0ad7295cf881b9bdeb0df9d18483fb96821
Reviewed-on: https://go-review.googlesource.com/38382
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This eliminates an old TODO,
and stabilizes the position information
for init functions.
Change-Id: Idf2d9a16a60e097ee08f42541b87e170da2f9d3a
Reviewed-on: https://go-review.googlesource.com/38388
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Previously, we handled recursive interfaces by deferring typechecking
of interface methods, while eagerly expanding interface embeddings.
This CL switches to eagerly evaluating interface methods, and
deferring expanding interface embeddings to dowidth. This allows us to
detect recursive interface embeddings with the same mechanism used for
detecting recursive struct embeddings.
Updates #16369.
Change-Id: If4c0320058047f8a2d9b52b9a79de47eb9887f95
Reviewed-on: https://go-review.googlesource.com/38391
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This CL changes the order that liveness analysis visits CFG blocks to
PC order, rather than RPO. This doesn't meaningfully change anything
except that the PCDATA_StackMapIndex values will be assigned in PC
order too.
However, this does have the benefit that the subsequent CL to port
liveness analysis to the SSA CFG (which has blocks in PC order) will
now pass toolstash-check.
Change-Id: I1de5a2eecb8027723a6e422d46186d0c63d48c8d
Reviewed-on: https://go-review.googlesource.com/38086
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Teach the backend to recognize that the address of a symbol
is equal with itself, and that the addresses of two different
symbols are different.
Some examples of where this rule hits in the standard library:
- inlined uses of (*time.Time).setLoc (e.g. time.UTC)
- inlined uses of bufio.NewReader (via type assertion)
Change-Id: I23dcb068c2ec333655c1292917bec13bbd908c24
Reviewed-on: https://go-review.googlesource.com/38338
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When optimizations are disabled, the compiler
cannot eliminate enough write barriers to satisfy
the runtime's nowritebarrier and nowritebarrierrec
annotations.
Enforce that requirement, and for convenience,
have cmd/go elide -N when compiling the runtime.
This came up in practice for me when running
toolstash -cmp. When toolstash -cmp detected
mismatches, it recompiled with -N, which caused
runtime compilation failures.
Change-Id: Ifcdef22c725baf2c59a09470f00124361508a8f3
Reviewed-on: https://go-review.googlesource.com/38380
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Instead we can use t.nod.Pos.
Change-Id: I643ee3226e402e38d4c77e8f328cbe83e55eac5c
Reviewed-on: https://go-review.googlesource.com/38309
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
CL 27254 changed a constant string to a byte array
in encoding/hex and got significant performance
improvements.
hex.Encode used the string twice in a single function.
The rewrite rules lower constant strings into components.
The pointer component requires an aux symbol.
The existing implementation created a new aux symbol every time.
As a result, constant string pointers were never CSE'd.
Tighten then moved the pointer calculation next to the uses, i.e.
into the loop.
The re-use of aux syms enabled by this CL
occurs 3691 times during make.bash.
This CL should not go in without CL 38338
or something like it.
Change-Id: Ibbf5b17283c0e31821d04c7e08d995c654de5663
Reviewed-on: https://go-review.googlesource.com/28219
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Report syntax error that was missed when moving to new parser.
Fixes#19610.
Change-Id: Ie5625f907a84089dc56fcccfd4f24df546042783
Reviewed-on: https://go-review.googlesource.com/38375
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This makes the overall naming and use of the functions
to create a Type more consistent.
Passes toolstash -cmp.
Change-Id: Ie0d40b42cc32b5ecf5f20502675a225038ea40e4
Reviewed-on: https://go-review.googlesource.com/38354
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I noted in CL 38327 that the SSA test API felt a bit
clunky after the ssa.Func/ssa.Cache/ssa.Config refactoring,
and promised to clean it up once the dust settled.
The dust has settled.
Along the way, this CL fixes a potential latent bug,
in which the amd64 test context was used for all dummy Syslook calls.
The lone SSA test using the s390x context did not depend on the
Syslook context being correct, so the bug did not arise in practice.
Change-Id: If964251d1807976073ad7f47da0b1f1f77c58413
Reviewed-on: https://go-review.googlesource.com/38346
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Mapping all empty interfaces onto the same Type
allows better reuse of the ptrTo and sliceOf
Type caches for *interface{} and []interface{}.
This has little compiler performance impact now,
but it will be helpful in the future,
when we will eagerly populate some of those caches.
Passes toolstash-check.
Change-Id: I17daee599a129b0b2f5f3025c1be43d569d6782c
Reviewed-on: https://go-review.googlesource.com/38344
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This reduces the number of calls back into the
gc Type routines, which will help performance
in a concurrent backend.
It also reduces the number of callsites
that must be considered in making the transition.
Passes toolstash-check -all. No compiler performance changes.
Updates #15756
Change-Id: Ic7a8f1daac7e01a21658ae61ac118b2a70804117
Reviewed-on: https://go-review.googlesource.com/38340
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Prior to this CL, the ssa.Frontend field was responsible
for providing types to the backend during compilation.
However, the types needed by the backend are few and static.
It makes more sense to use a struct for them
and to hang that struct off the ssa.Config,
which is the correct home for readonly data.
Now that Types is a struct, we can clean up the names a bit as well.
This has the added benefit of allowing early construction
of all types needed by the backend.
This will be useful for concurrent backend compilation.
Passes toolstash-check -all. No compiler performance change.
Updates #15756
Change-Id: I021658c8cf2836d6a22bbc20cc828ac38c7da08a
Reviewed-on: https://go-review.googlesource.com/38336
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
While we're here, also eliminate a few more Curfn uses.
Passes toolstash -cmp. No compiler performance impact.
Updates #15756
Change-Id: Ib8db9e23467bbaf16cc44bf62d604910f733d6b8
Reviewed-on: https://go-review.googlesource.com/38331
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is a first step towards eliminating the
Curfn global in the backend.
There's more to do.
Passes toolstash -cmp. No compiler performance impact.
Updates #15756
Change-Id: Ib09f550a001e279a5aeeed0f85698290f890939c
Reviewed-on: https://go-review.googlesource.com/38232
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Suggested by mdempsky in CL 38232.
This allows us to use the Frontend field
to associate frontend state and information
with a function.
See the following CL in the series for examples.
This is a giant CL, but it is almost entirely routine refactoring.
The ssa test API is starting to feel a bit unwieldy.
I will clean it up separately, once the dust has settled.
Passes toolstash -cmp.
Updates #15756
Change-Id: I71c573bd96ff7251935fce1391b06b1f133c3caf
Reviewed-on: https://go-review.googlesource.com/38327
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Prior to this CL, config was an explicit argument
to the SSA rewrite rules, and rules that needed
a Frontend got at it via config.
An upcoming CL moves Frontend from Config to Func,
so rules can no longer reach Frontend via Config.
Passing a Frontend as an argument to the rewrite rules
causes a 2-3% regression in compile times.
This CL takes a different approach:
It treats the variable names "config" and "fe"
as special and calculates them as needed.
The "as needed part" is also important to performance:
If they are calculated eagerly, the nilchecks themselves
cause a regression.
This introduces a little bit of magic into the rewrite
generator. However, from the perspective of the rules,
the config variable was already more or less magic.
And it makes the upcoming changes much clearer.
Passes toolstash -cmp.
Change-Id: I173f2bcc124cba43d53138bfa3775e21316a9107
Reviewed-on: https://go-review.googlesource.com/38326
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL changes the GOARCH.Init functions to take gc.Thearch as a
parameter, which gc.Main supplies.
Additionally, the x86 backend is refactored to decide within Init
whether to use the 387 or SSE2 instruction generators, rather than for
each individual SSA Value/Block.
Passes toolstash-check -all.
Change-Id: Ie6305a6cd6f6ab4e89ecbb3cbbaf5ffd57057a24
Reviewed-on: https://go-review.googlesource.com/38301
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
I don't know that it exists for any other architectures.
Update #18616
Change-Id: Idfe5dee251764d32787915889ec0be4bebc5be24
Reviewed-on: https://go-review.googlesource.com/38323
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This makes ssa.Func, ssa.Cache, and ssa.Config fulfill
the roles laid out for them in CL 38160.
The only non-trivial change in this CL is how cached
values and blocks get IDs. Prior to this CL, their IDs were
assigned as part of resetting the cache, and only modified
IDs were reset. This required knowing how many values and
blocks were modified, which required a tight coupling between
ssa.Func and ssa.Config. To eliminate that coupling,
we now zero values and blocks during reset,
and assign their IDs when they are used.
Since unused values and blocks have ID == 0,
we can efficiently find the last used value/block,
to avoid zeroing everything.
Bulk zeroing is efficient, but not efficient enough
to obviate the need to avoid zeroing everything every time.
As a happy side-effect, ssa.Func.Free is no longer necessary.
DebugHashMatch and friends now belong in func.go.
They have been left in place for clarity and review.
I will move them in a subsequent CL.
Passes toolstash -cmp. No compiler performance impact.
No change in 'go test cmd/compile/internal/ssa' execution time.
Change-Id: I2eb7af58da067ef6a36e815a6f386cfe8634d098
Reviewed-on: https://go-review.googlesource.com/38167
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
See https://go-review.googlesource.com/#/c/38313/ for background.
It turns out that only a few tests checked for this.
The new error message is shorter and very clear.
Change-Id: I8ab4ad59fb023c8b54806339adc23aefd7dc7b07
Reviewed-on: https://go-review.googlesource.com/38314
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is an evolution of https://go-review.googlesource.com/33616, as discussed
via email with Robert (gri):
$ cat foobar.go
package main
func main() {
a := "foo", "bar"
}
before:
./foobar.go:4:4: assignment count mismatch: want 1 values, got 2
after:
./foobar.go:4:4: assignment count mismatch: cannot assign 2 values to 1 variables
We could likely also eliminate the "assignment count mismatch" prefix now
without losing any information, but that string is matched by a number of
tests.
Change-Id: Ie6fc8a7bbd0ebe841d53e66e5c2f49868decf761
Reviewed-on: https://go-review.googlesource.com/38313
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
name old time/op new time/op delta
LeadingZeros-4 2.00ns ± 0% 1.34ns ± 1% -33.02% (p=0.000 n=8+10)
LeadingZeros16-4 1.62ns ± 0% 1.57ns ± 0% -3.09% (p=0.001 n=8+9)
LeadingZeros32-4 2.14ns ± 0% 1.48ns ± 0% -30.84% (p=0.002 n=8+10)
LeadingZeros64-4 2.06ns ± 1% 1.33ns ± 0% -35.08% (p=0.000 n=8+8)
8-bit args is a special case - the Go code is really fast because
it is just a single table lookup. So I've disabled that for now.
Intrinsics were actually slower:
LeadingZeros8-4 1.22ns ± 3% 1.58ns ± 1% +29.56% (p=0.000 n=10+10)
Update #18616
Change-Id: Ia9c289b9ba59c583ea64060470315fd637e814cf
Reviewed-on: https://go-review.googlesource.com/38311
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Remove size AuxInt in Store, and alignment in Move/Zero. We still
pass size AuxInt to Move/Zero, as it is used for partial Move/Zero
lowering (e.g. cmd/compile/internal/ssa/gen/386.rules:288).
SizeAndAlign is gone.
Passes "toolstash -cmp" on std.
Change-Id: I1ca34652b65dd30de886940e789fcf41d521475d
Reviewed-on: https://go-review.googlesource.com/38150
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The old writebarrier implementation fails to handle single-block
loop where a memory Phi value depends on the write barrier store
in the same block. The new implementation (CL 36834) doesn't have
this problem. Add a test to ensure it.
Fix#19067.
Change-Id: Iab13c6817edc12be8a048d18699b4450fa7ed712
Reviewed-on: https://go-review.googlesource.com/36940
Reviewed-by: David Chase <drchase@google.com>
Now that the write barrier insertion is moved to SSA, the SSA
building code can be simplified.
Updates #17583.
Change-Id: I5cacc034b11aa90b0abe6f8dd97e4e3994e2bc25
Reviewed-on: https://go-review.googlesource.com/36840
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
When the compiler insert write barriers, the frontend makes
conservative decisions at an early stage. This sometimes have
false positives because of the lack of information, for example,
writes on stack. SSA's writebarrier pass identifies writes on
stack and eliminates write barriers for them.
This CL moves write barrier insertion into SSA. The frontend no
longer makes decisions about write barriers, and simply does
normal assignments and emits normal Store ops when building SSA.
SSA writebarrier pass inserts write barrier for Stores when needed.
There, it has better information about the store because Phi and
Copy propagation are done at that time.
This CL only changes StoreWB to Store in gc/ssa.go. A followup CL
simplifies SSA building code.
Updates #17583.
Change-Id: I4592d9bc0067503befc169c50b4e6f4765673bec
Reviewed-on: https://go-review.googlesource.com/36839
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
For SSA Store/Move/Zero ops, attach the type of the value being
stored to the op as the Aux field. This type will be used for
write barrier insertion (in a followup CL). Since SSA passes
do not accurately propagate types of values (because of type
casting), we can't simply use type of the store's arguments
for write barrier insertion.
Passes "toolstash -cmp" on std.
Updates #17583.
Change-Id: I051d5e5c482931640d1d7d879b2a6bb91f2e0056
Reviewed-on: https://go-review.googlesource.com/36838
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Implement math/bits.TrailingZerosX using intrinsics.
Generally reorganize the intrinsic spec a bit.
The instrinsics data structure is now built at init time.
This will make doing the other functions in math/bits easier.
Update sys.CtzX to return int instead of uint{64,32} so it
matches math/bits.TrailingZerosX.
Improve the intrinsics a bit for amd64. We don't need the CMOV
for <64 bit versions.
Update #18616
Change-Id: Ic1c5339c943f961d830ae56f12674d7b29d4ff39
Reviewed-on: https://go-review.googlesource.com/38155
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
The line between ssa.Func and ssa.Config has blurred.
Concurrent compilation in the backend will require more precision.
This CL lays out an (aspirational) organization.
The implementation will come in follow-up CLs,
once the organization is settled.
ssa.Config holds basic compiler configuration,
mostly arch-specific information.
It is configured once, early on, and is readonly,
so it is safe for concurrent use.
ssa.Func is a single-shot object used for
compiling a single Func. It is not concurrency-safe
and not re-usable.
ssa.Cache is a multi-use object used to avoid
expensive allocations during compilation.
Each ssa.Func is given an ssa.Cache to use.
ssa.Cache is not concurrency-safe.
Change-Id: Id02809b6f3541541cac6c27bbb598834888ce1cc
Reviewed-on: https://go-review.googlesource.com/38160
Reviewed-by: Keith Randall <khr@golang.org>
Previously we always issued a spill right after the op
that was being spilled. This CL pushes spills father away
from the generator, hopefully pushing them into unlikely branches.
For example:
x = ...
if unlikely {
call ...
}
... use x ...
Used to compile to
x = ...
spill x
if unlikely {
call ...
restore x
}
It now compiles to
x = ...
if unlikely {
spill x
call ...
restore x
}
This is particularly useful for code which appends, as the only
call is an unlikely call to growslice. It also helps for the
spills needed around write barrier calls.
The basic algorithm is walk down the dominator tree following a
path where the block still dominates all of the restores. We're
looking for a block that:
1) dominates all restores
2) has the value being spilled in a register
3) has a loop depth no deeper than the value being spilled
The walking-down code is iterative. I was forced to limit it to
searching 100 blocks so it doesn't become O(n^2). Maybe one day
we'll find a better way.
I had to delete most of David's code which pushed spills out of loops.
I suspect this CL subsumes most of the cases that his code handled.
Generally positive performance improvements, but hard to tell for sure
with all the noise. (compilebench times are unchanged.)
name old time/op new time/op delta
BinaryTree17-12 2.91s ±15% 2.80s ±12% ~ (p=0.063 n=10+10)
Fannkuch11-12 3.47s ± 0% 3.30s ± 4% -4.91% (p=0.000 n=9+10)
FmtFprintfEmpty-12 48.0ns ± 1% 47.4ns ± 1% -1.32% (p=0.002 n=9+9)
FmtFprintfString-12 85.6ns ±11% 79.4ns ± 3% -7.27% (p=0.005 n=10+10)
FmtFprintfInt-12 91.8ns ±10% 85.9ns ± 4% ~ (p=0.203 n=10+9)
FmtFprintfIntInt-12 135ns ±13% 127ns ± 1% -5.72% (p=0.025 n=10+9)
FmtFprintfPrefixedInt-12 167ns ± 1% 168ns ± 2% ~ (p=0.580 n=9+10)
FmtFprintfFloat-12 249ns ±11% 230ns ± 1% -7.32% (p=0.000 n=10+10)
FmtManyArgs-12 504ns ± 7% 506ns ± 1% ~ (p=0.198 n=9+9)
GobDecode-12 6.95ms ± 1% 7.04ms ± 1% +1.37% (p=0.001 n=10+10)
GobEncode-12 6.32ms ±13% 6.04ms ± 1% ~ (p=0.063 n=10+10)
Gzip-12 233ms ± 1% 235ms ± 0% +1.01% (p=0.000 n=10+9)
Gunzip-12 40.1ms ± 1% 39.6ms ± 0% -1.12% (p=0.000 n=10+8)
HTTPClientServer-12 227µs ± 9% 221µs ± 5% ~ (p=0.114 n=9+8)
JSONEncode-12 16.1ms ± 2% 15.8ms ± 1% -2.09% (p=0.002 n=9+8)
JSONDecode-12 61.8ms ±11% 57.9ms ± 1% -6.30% (p=0.000 n=10+9)
Mandelbrot200-12 4.30ms ± 3% 4.28ms ± 1% ~ (p=0.203 n=10+8)
GoParse-12 3.18ms ± 2% 3.18ms ± 2% ~ (p=0.579 n=10+10)
RegexpMatchEasy0_32-12 76.7ns ± 1% 77.5ns ± 1% +0.92% (p=0.002 n=9+8)
RegexpMatchEasy0_1K-12 239ns ± 3% 239ns ± 1% ~ (p=0.204 n=10+10)
RegexpMatchEasy1_32-12 71.4ns ± 1% 70.6ns ± 0% -1.15% (p=0.000 n=10+9)
RegexpMatchEasy1_1K-12 383ns ± 2% 390ns ±10% ~ (p=0.181 n=8+9)
RegexpMatchMedium_32-12 114ns ± 0% 113ns ± 1% -0.88% (p=0.000 n=9+8)
RegexpMatchMedium_1K-12 36.3µs ± 1% 36.8µs ± 1% +1.59% (p=0.000 n=10+8)
RegexpMatchHard_32-12 1.90µs ± 1% 1.90µs ± 1% ~ (p=0.341 n=10+10)
RegexpMatchHard_1K-12 59.4µs ±11% 57.8µs ± 1% ~ (p=0.968 n=10+9)
Revcomp-12 461ms ± 1% 462ms ± 1% ~ (p=1.000 n=9+9)
Template-12 67.5ms ± 1% 66.3ms ± 1% -1.77% (p=0.000 n=10+8)
TimeParse-12 314ns ± 3% 309ns ± 0% -1.56% (p=0.000 n=9+8)
TimeFormat-12 340ns ± 2% 331ns ± 1% -2.79% (p=0.000 n=10+10)
The go binary is 0.2% larger. Not really sure why the size
would change.
Change-Id: Ia5116e53a3aeb025ef350ffc51c14ae5cc17871c
Reviewed-on: https://go-review.googlesource.com/34822
Reviewed-by: David Chase <drchase@google.com>
Interface wrapper functions now get compiled eagerly in some cases.
Consequently, they may be present in multiple translation units.
Mark them as DUPOK, just like closures.
Fixes#19548Fixes#19550
Change-Id: Ibe74adb5a62dbf6447db37fde22dcbb3479969ef
Reviewed-on: https://go-review.googlesource.com/38156
Reviewed-by: David Chase <drchase@google.com>
In the SSA CFG, TEXT, RET, and JMP instructions correspond to Blocks,
not Values. Rework liveness analysis so that progeffects only cares
about Progs that result from Values, and handle Blocks separately.
Passes toolstash-check -all.
Change-Id: Ic23719c75b0421fdb51382a08dac18c3ba042b32
Reviewed-on: https://go-review.googlesource.com/38085
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The fmtmode and fmtpkgpfx globals stand in the
way of making the compiler more concurrent (#15756).
This CL removes them.
The natural way to eliminate a global is to explicitly
thread it as a parameter through all function calls.
However, most of the functions in gc/fmt.go
get called indirectly, by way of fmt format strings,
so there's nowhere natural to add a parameter.
Since there are only a few fmtmode modes,
use named types to distinguish between modes.
For example, fmtNodeErr, fmtNodeDbg, and fmtNodeTypeId
are all gc.Node, but they print in different modes.
Varying the type allows us to thread mode through fmt.
Handle fmtpkgpfx by converting it to a printing mode,
FTypeIdName, and using the same type-based approach.
To avoid a loss of readability and danger of bugs
from introducing conversions at all call sites,
instead add a helper that systematically modifies the args.
The only remaining gc/fmt.go global is dumpdepth.
Since that is used for debugging only,
it that can be handled with a global mutex,
or some similarly basic, if inefficient, protection.
Passes toolstash -cmp. No compiler performance impact.
For future reference, other options for threading state
that were considered and rejected:
* Wrapping values in structs, such as:
type fmtNode struct {
n *Node
mode fmtMode
}
This reduces the proliferation of types, and supports
easily adding extra local parameters.
However, putting such a struct into an interface{} allocates.
This is unacceptable in this particular area of code.
* Passing state via precision, such as:
fmt.Fprintf("%*v", mode, n)
where mode is the state encoded as an integer.
This avoids extra allocations, but it is out of keeping
with the intended semantics of precision, and is less readable.
* Modify the fmt package to support setting/getting context
via fmt.State. Unavailable due to Go 1 compatibility,
and probably the wrong solution anyway.
* Give up on package fmt. This would be a huge readability
regression and cause high code churn.
* Attempt a de-novo rewrite that circumvents these problems.
Too high a risk of bugs, with insufficient reward for the effort,
particularly since long term plans call for elimination
of gc.Node.
Change-Id: Iea2440d5a34a938e64273707de27e3a897cb41d1
Reviewed-on: https://go-review.googlesource.com/38147
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
With this change, code like
h := sha1.New()
h.Write(buf)
sum := h.Sum()
gets compiled into static calls rather than
interface calls, because the compiler is able
to prove that 'h' is really a *sha1.digest.
The InterCall re-write rule hits a few dozen times
during make.bash, and hundreds of times during all.bash.
The most common pattern identified by the compiler
is a constructor like
func New() Interface { return &impl{...} }
where the constructor gets inlined into the caller,
and the result is used immediately. Examples include
{sha1,md5,crc32,crc64,...}.New, base64.NewEncoder,
base64.NewDecoder, errors.New, net.Pipe, and so on.
Some existing benchmarks that change on darwin/amd64:
Crc64/ISO4KB-8 2.67µs ± 1% 2.66µs ± 0% -0.36% (p=0.015 n=10+10)
Crc64/ISO1KB-8 694ns ± 0% 690ns ± 1% -0.59% (p=0.001 n=10+10)
Adler32KB-8 473ns ± 1% 471ns ± 0% -0.39% (p=0.010 n=10+9)
On architectures like amd64, the reduction in code size
appears to contribute more to benchmark improvements than just
removing the indirect call, since that branch gets predicted
accurately when called in a loop.
Updates #19361
Change-Id: I57d4dc21ef40a05ec0fbd55a9bb0eb74cdc67a3d
Reviewed-on: https://go-review.googlesource.com/38139
Run-TryBot: Philip Hofer <phofer@umich.edu>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Changes to ${GOARCH}Ops.go files were mechanically produced using
github.com/mdempsky/ssa-symops, a one-off tool that inserts
"SymEffect: X" elements by pattern matching against the Op names.
Change-Id: Ibf3e481ffd588647f2a31662d72114b740ccbfcf
Reviewed-on: https://go-review.googlesource.com/38084
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
To replace the progeffects tables for liveness analysis.
Change-Id: Idc4b990665cb0a9aa300d62cdf8ad12e51c5b991
Reviewed-on: https://go-review.googlesource.com/38083
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Bad pragmas should never make it to the backend.
I've confirmed manually that the error position is unchanged.
Updates #15756
Updates #19250
Change-Id: If14f7ce868334f809e337edc270a49680b26f48e
Reviewed-on: https://go-review.googlesource.com/38152
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
There were a surprising number of places
in the tree that used yyerror for failed internal
consistency checks. Switch them to Fatalf.
Updates #15756
Updates #19250
Change-Id: Ie4278148185795a28ff3c27dacffc211cda5bbdd
Reviewed-on: https://go-review.googlesource.com/38153
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This reverts commit 4e0c7c3f61.
Reason for revert: The presence-of-optimization test program is fragile, breaks under noopt, and might break if the Go libraries are tweaked. It needs to be (re)written without reference to other packages.
Change-Id: I3aaf1ab006a1a255f961a978e9c984341740e3c7
Reviewed-on: https://go-review.googlesource.com/38097
Reviewed-by: Keith Randall <khr@golang.org>
This abstracts creation of ACALL Progs into package gc. The main
benefit of this today is we can refactor away a lot of common
boilerplate code.
Later, once liveness analysis happens on the SSA graph, this will also
provide an easy insertion point for emitting the PCDATA Progs
immediately before call instructions.
Passes toolstash-check -all.
Change-Id: Ia15108ace97201cd84314f1ca916dfeb4f09d61c
Reviewed-on: https://go-review.googlesource.com/38081
Reviewed-by: Keith Randall <khr@golang.org>
Move the zeroing of results earlier. In particular, they need to
come before any move-to-heap operations, as those require allocation.
Those allocations are points at which the GC can see the uninitialized
result slots.
For the function:
func f() (x, y, z *int) {
defer(){}()
escape(&y)
return
}
We used to generate code like this:
x = nil
y = nil
&y = new(int)
z = nil
Now we will generate:
x = nil
y = nil
z = nil
&y = new(int)
Since the fix for #18860, the return slots are always live if there
is a defer, so the former ordering allowed the GC to see junk
in the z slot.
Fixes#19078
Change-Id: I71554ae437549725bb79e13b2c100b2911d47ed4
Reviewed-on: https://go-review.googlesource.com/38133
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The existing implementation started by eliminating
nil checks for OpAddr, OpAddPtr, and OpPhis with
all non-nil args.
However, some OpPhis had all non-nil args,
but their args had not been processed yet.
Pull the OpPhi checks into their own loop,
and repeat until stabilization.
Eliminates a dozen additional nilchecks during make.bash.
Negligible compiler performance impact.
Change-Id: If7b803c3ad7582af7d9867d05ca13e03e109d864
Reviewed-on: https://go-review.googlesource.com/37999
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
With this change, code like
h := sha1.New()
h.Write(buf)
sum := h.Sum()
gets compiled into static calls rather than
interface calls, because the compiler is able
to prove that 'h' is really a *sha1.digest.
The InterCall re-write rule hits a few dozen times
during make.bash, and hundreds of times during all.bash.
The most common pattern identified by the compiler
is a constructor like
func New() Interface { return &impl{...} }
where the constructor gets inlined into the caller,
and the result is used immediately. Examples include
{sha1,md5,crc32,crc64,...}.New, base64.NewEncoder,
base64.NewDecoder, errors.New, net.Pipe, and so on.
Some existing benchmarks that change on darwin/amd64:
Crc64/ISO4KB-8 2.67µs ± 1% 2.66µs ± 0% -0.36% (p=0.015 n=10+10)
Crc64/ISO1KB-8 694ns ± 0% 690ns ± 1% -0.59% (p=0.001 n=10+10)
Adler32KB-8 473ns ± 1% 471ns ± 0% -0.39% (p=0.010 n=10+9)
On architectures like amd64, the reduction in code size
appears to contribute more to benchmark improvements than just
removing the indirect call, since that branch gets predicted
accurately when called in a loop.
Updates #19361
Change-Id: Ia9d30afdd5f6b4d38d38b14b88f308acae8ce7ed
Reviewed-on: https://go-review.googlesource.com/37751
Run-TryBot: Philip Hofer <phofer@umich.edu>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
In Go 1.7 and earlier, gc.exportsize tracked the number of bytes
written through exportf. With the removal of the old exporter in Go 1.8
exportf is only used for printing the build id, and the header and
trailer of the binary export format. The size of the export data is
now returned directly from the exporter and exportsize is never
referenced. Remove it.
Change-Id: Id301144b3c26c9004c722d0c55c45b0e0801a88c
Reviewed-on: https://go-review.googlesource.com/38116
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Minor fix, because it's the right thing to do.
No significant impact.
Change-Id: I2138285d397494daa9a88c414149c2a7860edd7e
Reviewed-on: https://go-review.googlesource.com/38001
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Values have an Aux and an AuxInt.
We're setting AuxInt, not Aux.
Say so.
Change-Id: I41aa783273bb7e1ba47c941aa4233f818e37dadd
Reviewed-on: https://go-review.googlesource.com/37997
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Compiler errors now show the exact line and line byte offset (sometimes
called "column") of where an error occured. For `go tool compile x.go`:
package p
const c int = false
//line foo.go:123
type t intg
reports
x.go:2:7: cannot convert false to type int
foo.go:123[x.go:4:8]: undefined: intg
(Some errors use the "wrong" position for the error message; arguably
the byte offset for the first error should be 15, the position of 'false',
rathen than 7, the position of 'c'. But that is an indepedent issue.)
The byte offset (column) values are measured in bytes; they start at 1,
matching the convention used by editors and IDEs.
Positions modified by //line directives show the line offset only for the
actual source location (in square brackets), not for the "virtual" file and
line number because that code is likely generated and the //line directive
only provides line information.
Because the new format might break existing tools or scripts, printing
of line offsets can be disabled with the new compiler flag -C. We plan
to remove this flag eventually.
Fixes#10324.
Change-Id: I493f5ee6e78457cf7b00025aba6b6e28e50bb740
Reviewed-on: https://go-review.googlesource.com/37970
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
We could leave it alone and fix line offset (column) numbers when
reporting errors, but that is likely to cause confusion (internal
numbers don't match reported numbers). Instead, switch to default
numbering starting at 1.
For package syntax-internal use only, introduced constants defining
the line and column bases, and use them throughout the code and its
tests. It is possible to change these constants and package syntax
will continue to work. But changing them is going to break any client
that makes explicit assumptions about line and column numbers (which
is "all of them").
Change-Id: Ia3d136a8ec8d9372ed9c05ca47d3dff222cf030e
Reviewed-on: https://go-review.googlesource.com/37996
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This change adds a method to replace expressions
of the form
v.Args[len(v.Args)-1]
so that the code's intention to walk memory arguments
is explicit.
Passes toolstash-check.
Change-Id: I0c80d73bc00989dd3cdf72b4f2c8e1075a2515e0
Reviewed-on: https://go-review.googlesource.com/37757
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The type of the OffPtr for the first field was incorrect. It should
have been a pointer to the field type, rather than the field
type itself.
Fixes#19475.
Change-Id: I3960b404da0f4bee759331126cce6140d2ce1df7
Reviewed-on: https://go-review.googlesource.com/37869
Run-TryBot: Michael Munday <munday@ca.ibm.com>
Reviewed-by: Keith Randall <khr@golang.org>
Previously the base register was unset, which lead to the disassembler
using "FP" instead of "SP" as the base register. That lead to some
confusion as to what the difference betweeen the two was.
Be consistent and always use SP.
Fixes#19458
Change-Id: Ie8f8ee54653bd202c0cf6fbf1d350e3c8c8b67a0
Reviewed-on: https://go-review.googlesource.com/37971
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
After benchmarking with a compiler modified to have better
spill location, it became clear that this method of checking
was actually faster on (at least) two different architectures
(ppc64 and amd64) and it also provides more timely interruption
of loops.
This change adds a modified FOR loop node "FORUNTIL" that
checks after executing the loop body instead of before (i.e.,
always at least once). This ensures that a pointer past the
end of a slice or array is not made visible to the garbage
collector.
Without the rescheduling checks inserted, the restructured
loop from this change apparently provides a 1% geomean
improvement on PPC64 running the go1 benchmarks; the
improvement on AMD64 is only 0.12%.
Inserting the rescheduling check exposed some peculiar bug
with the ssa test code for s390x; this was updated based on
initial code actually generated for GOARCH=s390x to use
appropriate OpArg, OpAddr, and OpVarDef.
NaCl is disabled in testing.
Change-Id: Ieafaa9a61d2a583ad00968110ef3e7a441abca50
Reviewed-on: https://go-review.googlesource.com/36206
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The position information recorded now consists of the line-
directive relative filename and line number. It would be
relatively easy to also encode absolute position information
as necessary (by serializing src.PosBase data).
For example, given $GOROOT/src/tmp/x.go:
package p
const C0 = 0
//line c.go:10
const C1 = 1
//line t.go:20
type T int
//line v.go:30
var V T
//line f.go:40
func F() {}
The recorded positions for the exported entities are:
C0 $GOROOT/src/tmp/x.go 3
C1 c.go 10
T t.go 20
V v.go 30
F f.go 40
Fix verified by manual inspection. There's currently no easy way
to test this, but it will eventually be tested when we fix#7311.
Fixes#19391.
Change-Id: I6269067ea58358250fe6dd1f73bdf9e5d2adfe3d
Reviewed-on: https://go-review.googlesource.com/37936
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Several SSA ops will always behave identically regardless of target
architecture, so handle those within gc/ssa.go instead.
Passes toolstash-check -all.
Change-Id: I54d514e80ab86723e44332a5a38e3054cbca8c5d
Reviewed-on: https://go-review.googlesource.com/37931
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This commit reworks multiway select statements to use normal control
flow primitives instead of the previous setjmp/longjmp-like behavior.
This simplifies liveness analysis and should prevent issues around
"returns twice" function calls within SSA passes.
test/live.go is updated because liveness analysis's CFG is more
representative of actual control flow. The case bodies are the only
real successors of the selectgo call, but previously the selectsend,
selectrecv, etc. calls were included in the successors list too.
Updates #19331.
Change-Id: I7f879b103a4b85e62fc36a270d812f54c0aa3e83
Reviewed-on: https://go-review.googlesource.com/37661
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
When defining an int const, the compiler tries to cast the RHS
expression to int. The cast may fail for three reasons:
1. expr is an integer constant that overflows int
2. expr is a floating point constant
3. expr is a complex constant, or not a number
In the second case, in order to print a sensible error message, we
must distinguish between a floating point constant that should be
included in the error message and a floating point constant that
cannot be reasonably formatted for inclusion in an error message.
For example, in:
const a int = 1.1
const b int = 1 + 1e-100
a is in the former group, while b is in the latter, since the floating
point value resulting from the evaluation of the rhs of the assignment
(1.00...01) is too long to be fully printed in an error message, and
cannot be shortened without making the error message misleading
(rounding or truncating it would result in a "1", which looks like an
integer constant, and it makes little sense in an error message about
an invalid floating point expression).
To fix this problem, we try to format the float value using fconv
(which is used by the error reporting mechanism to format float
arguments), and then parse the resulting string back to a
big.Float. If the result is an integer, we assume that expr is a float
value that cannot be reasonably be formatted as a string, and we emit
an error message that does not include its string representation.
Also, change the error message for overflows to a more conservative
"integer too large", which does not mention overflows that are only
caused by an internal implementation restriction.
Also, change (*Mpint) SetFloat so that it returns a bool (instead of
0/-1 for success/failure).
Fixes#11371
Change-Id: Ibbc73e2ed2eaf41f07827b0649d0eb637150ecaa
Reviewed-on: https://go-review.googlesource.com/35411
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Instead of skipping them based on string matching much later in the
compilation process, skip them up front using the proper API.
Passes toolstash-check.
Change-Id: Ibd4c0448a0701ba0de3235d4689ef300235fa1d9
Reviewed-on: https://go-review.googlesource.com/37930
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
No need to keep as Nodes when they're all Types anyway.
Change-Id: I8157914ba5b09cadf2263247844680a60233a0f2
Reviewed-on: https://go-review.googlesource.com/37886
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This reverts commit cb6e0639fb.
The fix is incorrect as it's perfectly fine to refer to an
identifier 'init' inside a function, and 'init' may even be
a variable of function value. Misspelling 'init' in that
context would lead to an incorrect error message.
Reopened#8481.
Change-Id: I49787fdf7738213370ae6f0cab54013e9e3394a8
Reviewed-on: https://go-review.googlesource.com/37876
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Given
(Store [c] (OffPtr <T1> [0] (Addr <T> _)) _
(Store [c] (Addr <T> _) _ _))
dead store elimination doesn't eliminate the inner
Store, because it addresses a type of a different width
than the first store.
When decomposing StructMake operations, always generate
an OffPtr to address struct fields so that dead stores to
the first field of the struct can be optimized away.
benchmarks affected on darwin/amd64:
HTTPClientServer-8 73.2µs ± 1% 72.7µs ± 1% -0.69% (p=0.022 n=9+10)
TimeParse-8 304ns ± 1% 300ns ± 0% -1.61% (p=0.000 n=9+9)
RegexpMatchEasy1_32-8 80.1ns ± 0% 79.5ns ± 1% -0.84% (p=0.000 n=8+9)
GobDecode-8 6.78ms ± 0% 6.81ms ± 1% +0.46% (p=0.000 n=9+10)
Gunzip-8 36.1ms ± 1% 36.2ms ± 0% +0.37% (p=0.019 n=10+10)
JSONEncode-8 15.6ms ± 0% 15.7ms ± 0% +0.69% (p=0.000 n=9+10)
Change-Id: Ia80d73fd047f9400c616ca64fdee4f438a0e7f21
Reviewed-on: https://go-review.googlesource.com/37769
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Shrinks LSym somewhat for non-STEXT LSyms, which are much more common.
While here, switch to tracking Automs in a slice instead of a linked
list. (Previously, this would have made LSyms larger.)
Passes toolstash-check.
Change-Id: I082e50e1d1f1b544c9e06b6e412a186be6a4a2b5
Reviewed-on: https://go-review.googlesource.com/37872
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
By clearing out t.nod in copytype, we effectively lose the reference
from a Type back to its declaring OTYPE Node. This means later in
typenamesym when we add typenod(t) to signatlist, we end up creating a
new OTYPE Node. Moreover, this Node's position information will depend
on whatever context it happens be needed, and will be used for the
Type's position in the export data.
Updates #19391.
Change-Id: Ied93126449f75d7c5e3275cbdcc6fa657a8aa21d
Reviewed-on: https://go-review.googlesource.com/37870
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This makes Sym flags consistent with the rest of the code after
the CL 37445.
No functional changes.
Passes toolstash -cmp.
Change-Id: Ica919f2ab98581371c717fff9a70aeb11058ca17
Reviewed-on: https://go-review.googlesource.com/37847
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Change-Id: I6343c162e27e2e492547c96f1fc504909b1c03c0
Reviewed-on: https://go-review.googlesource.com/37793
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The assembler back end uses F15 as a temporary register in these
instructions.
Checked the assembler back end and made sure that this is the
only case clobbering F15.
Fixes#19403.
Change-Id: I02b9e00fdd9229db899f501c8e9b306e02912d83
Reviewed-on: https://go-review.googlesource.com/37792
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Match more patterns generated by the compiler where the index for
a bound check is bounded through a AND operation, with different
register sizes.
These rules trigger a dozen of times in a bootstrap.
Change-Id: Ic9fff16f21d08580f19a366c3ee1a372e58357d1
Reviewed-on: https://go-review.googlesource.com/37442
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
It was used with Node.Ullman, which is now gone.
Change-Id: I83b167645659ae7ef70043b7915d642e42ca524f
Reviewed-on: https://go-review.googlesource.com/37761
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Since switching to SSA, the only remaining use for the Ullman field
was in tracking whether or not an expression contained a function
call. Give it a new name and encode it in our fancy new bitset field.
Passes toolstash-check.
Change-Id: I95b7f9cb053856320c0d66efe14996667e6011c2
Reviewed-on: https://go-review.googlesource.com/37721
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
For example, `-d pctab=pctoinline` prints the PC-inline table and
inlining tree for every function.
Change-Id: Ia6b9ce4d83eed0b494318d40ffe06481ec5d58ab
Reviewed-on: https://go-review.googlesource.com/37235
Run-TryBot: David Lazar <lazard@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Without this, literals keep their original source positions through
inlining, which results in strange jumps in line numbers of inlined
function bodies. By copying literals, inlining can update their source
position like other nodes.
Fixes#15453.
Change-Id: Iad5d9bbfe183883794213266dc30e31bab89ee69
Reviewed-on: https://go-review.googlesource.com/37232
Run-TryBot: David Lazar <lazard@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
In order to generate accurate tracebacks, the runtime needs to know the
inlined call stack for a given PC. This creates two tables per function
for this purpose. The first table is the inlining tree (stored in the
function's funcdata), which has a node containing the file, line, and
function name for every inlined call. The second table is a PC-value
table that maps each PC to a node in the inlining tree (or -1 if the PC
is not the result of inlining).
To give the appearance that inlining hasn't happened, the runtime also
needs the original source position information of inlined AST nodes.
Previously the compiler plastered over the line numbers of inlined AST
nodes with the line number of the call. This meant that the PC-line
table mapped each PC to line number of the outermost call in its inlined
call stack, with no way to access the innermost line number.
Now the compiler retains line numbers of inlined AST nodes and writes
the innermost source position information to the PC-line and PC-file
tables. Some tools and tests expect to see outermost line numbers, so we
provide the OutermostLine function for displaying line info.
To keep track of the inlined call stack for an AST node, we extend the
src.PosBase type with an index into a global inlining tree. Every time
the compiler inlines a call, it creates a node in the global inlining
tree for the call, and writes its index to the PosBase of every inlined
AST node. The parent of this node is the inlining tree index of the
call. -1 signifies no parent.
For each function, the compiler creates a local inlining tree and a
PC-value table mapping each PC to an index in the local tree. These are
written to an object file, which is read by the linker. The linker
re-encodes these tables compactly by deduplicating function names and
file names.
This change increases the size of binaries by 4-5%. For example, this is
how the go1 benchmark binary is impacted by this change:
section old bytes new bytes delta
.text 3.49M ± 0% 3.49M ± 0% +0.06%
.rodata 1.12M ± 0% 1.21M ± 0% +8.21%
.gopclntab 1.50M ± 0% 1.68M ± 0% +11.89%
.debug_line 338k ± 0% 435k ± 0% +28.78%
Total 9.21M ± 0% 9.58M ± 0% +4.01%
Updates #19348.
Change-Id: Ic4f180c3b516018138236b0c35e0218270d957d3
Reviewed-on: https://go-review.googlesource.com/37231
Run-TryBot: David Lazar <lazard@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The Zero op right after newobject has been removed. But this rule
does not cover Store of constant zero (for SSA-able types). Add
rules to cover Store op as well.
Updates #19027.
Change-Id: I5d2b62eeca0aa9ce8dc7205b264b779de01c660b
Reviewed-on: https://go-review.googlesource.com/36836
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
On amd64p32, PtrSize and RegSize don't agree, and function return
value is aligned with RegSize. Fix this rule. Other architectures
are not affected, where PtrSize and RegSize are the same.
Change-Id: If187d3dfde3dc3b931b8e97db5eeff49a781551b
Reviewed-on: https://go-review.googlesource.com/37720
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Previously the compiler rewrote constant division into OHMUL
operations, but that rewriting was moved to SSA in CL 37015. Now OHMUL
is unused, so we can get rid of it.
Change-Id: Ib6fc7c2b6435510bafb5735b3b4f42cfd8ed8cdb
Reviewed-on: https://go-review.googlesource.com/37750
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
The compiler's -d flag accepts string-valued flags, but currently only
for SSA debug flags. Extend it to support string values for other
flags. This also makes the syntax somewhat more sane so flag=value and
flag:value now both accept integers and strings.
Change-Id: Idd144d8479a430970cc1688f824bffe0a56ed2df
Reviewed-on: https://go-review.googlesource.com/37345
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
A value is "volatile" if it is a pointer to the argument region
on stack which will be clobbered by function call. This is used
to make sure the value is safe when inserting write barrier calls.
The writebarrier pass can tell whether a value is such a pointer.
Therefore no need to mark it when building SSA and thread this
information through.
Passes "toolstash -cmp" on std.
Updates #17583.
Change-Id: Idc5fc0d710152b94b3c504ce8db55ea9ff5b5195
Reviewed-on: https://go-review.googlesource.com/36835
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This is now handled by os/exec.
Updates #12868
Change-Id: Ic21a6ff76a9b9517437ff1acf3a9195f9604bb45
Reviewed-on: https://go-review.googlesource.com/37698
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This adds the necessary changes so that atomics are treated as
intrinsics on ppc64x.
The implementations of And8 and Or8 require power8 for
both ppc64 and ppc64le. This is a new requirement
for ppc64.
Fixes#8739
Change-Id: Icb85e2755a49166ee3652668279f6ed5ebbca901
Reviewed-on: https://go-review.googlesource.com/36832
Reviewed-by: Keith Randall <khr@golang.org>
This makes a change in the SSA code generated for OpPPC64Xf2i64
and OpPPC64Xi2f64 to use register based instructions to convert
between float and integer. This will require at least power8.
Currently the conversion is done by storing to and loading
from memory, which is more expensive.
This improves some of the math functions:
BenchmarkExp-128 74.1 66.8 -9.85%
BenchmarkExpGo-128 87.4 66.3 -24.14%
BenchmarkExp2-128 72.2 64.3 -10.94%
BenchmarkExp2Go-128 74.3 65.9 -11.31%
BenchmarkLgamma-128 51.0 39.7 -22.16%
BenchmarkLog-128 42.9 40.6 -5.36%
BenchmarkLogb-128 11.5 9.16 -20.35%
BenchmarkLog1p-128 38.9 36.2 -6.94%
BenchmarkSin-128 29.5 23.7 -19.66%
BenchmarkTan-128 32.8 27.4 -16.46%
Fixes#18922
Change-Id: I8e1cf14d3880d7cd720dc5188dd174cba1f7fef7
Reviewed-on: https://go-review.googlesource.com/36725
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
Reviewed-by: Keith Randall <khr@golang.org>
The builtin runtime package definitions intentionally diverge from the
actual runtime package's, but this only works as long as they never
overlap.
To make it easier to expand the builtin runtime package, this CL now
loads their definitions into a logically separate "go.runtime"
package. By resetting the package's Prefix field to "runtime", any
references to builtin definitions will still resolve against the real
package runtime.
Fixes#14482.
Passes toolstash -cmp.
Change-Id: I539c0994deaed4506a331f38c5b4d6bc8c95433f
Reviewed-on: https://go-review.googlesource.com/37538
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Link.Plists never contained more than one Plist, and sometimes none.
Passing around the Plist being worked on is straightforward and makes
the data flow easier to follow.
Change-Id: I79cb30cb2bd3d319fdbb1dfa5d35b27fcb748e5c
Reviewed-on: https://go-review.googlesource.com/37169
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
There's no need to use @block rules, as canMergeLoad makes sure that
the load and op are already in the same block.
With no @block needed, we also don't need to set the type explicitly.
It can just be inherited from the op being rewritten.
Noticed while working on #19284.
Change-Id: Ied8bcc8058260118ff7e166093112e29107bcb7e
Reviewed-on: https://go-review.googlesource.com/37585
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
runtime.memclr* functions have signatures
func memclrNoHeapPointers(ptr unsafe.Pointer, n uintptr)
func memclrHasPointers(ptr unsafe.Pointer, n uintptr)
Update compiler's copy. Also teach gc/mkbuiltin.go to handle
unsafe.Pointer. The import statement and its support is not
really necessary, but just to make it look like real Go code.
Fixes#19185.
Change-Id: I251d02571fde2716d4727e31e04d56ec04b6f22a
Reviewed-on: https://go-review.googlesource.com/37257
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Explcitly block fused multiply-add pattern matching when a cast is used
after the multiplication, for example:
- (a * b) + c // can emit fused multiply-add
- float64(a * b) + c // cannot emit fused multiply-add
float{32,64} and complex{64,128} casts of matching types are now kept
as OCONV operations rather than being replaced with OCONVNOP operations
because they now imply a rounding operation (and therefore aren't a
no-op anymore).
Operations (for example, multiplication) on complex types may utilize
fused multiply-add and -subtract instructions internally. There is no
way to disable this behavior at the moment.
Improves the performance of the floating point implementation of
poly1305:
name old speed new speed delta
64 246MB/s ± 0% 275MB/s ± 0% +11.48% (p=0.000 n=10+8)
1K 312MB/s ± 0% 357MB/s ± 0% +14.41% (p=0.000 n=10+10)
64Unaligned 246MB/s ± 0% 274MB/s ± 0% +11.43% (p=0.000 n=10+10)
1KUnaligned 312MB/s ± 0% 357MB/s ± 0% +14.39% (p=0.000 n=10+8)
Updates #17895.
Change-Id: Ia771d275bb9150d1a598f8cc773444663de5ce16
Reviewed-on: https://go-review.googlesource.com/36963
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Fix up and enable a few rules.
They trigger a handful of times in std,
despite the frontend handling.
Change-Id: I83378c057cbbc95a4f2b58cd8c36aec0e9dc547f
Reviewed-on: https://go-review.googlesource.com/37227
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
A type conversion inserted between MOVD{LT,LE,GT,GE,EQ,NE} and CMPWconst
by CL 36256 broke the rewrite rule designed to merge the two.
This results in simple for loops (e.g. for i := 0; i < N; i++ {})
emitting two comparisons instead of one, plus a conditional move.
This CL explicitly types the input to CMPWconst so that the type conversion
can be omitted. It also adds a test to check that conditional moves aren't
emitted for loops with 'less than' conditions (i.e. i < N) on s390x.
Fixes#19227.
Change-Id: Ia39e806ed723791c3c755951aef23f957828ea3e
Reviewed-on: https://go-review.googlesource.com/37334
Reviewed-by: Keith Randall <khr@golang.org>
This is the escape analysis analog of CL 37499.
Fixes#12397Fixes#16871
The only "moved to heap" decisions eliminated by this
CL in std+cmd are:
cmd/compile/internal/gc/const.go:1514: moved to heap: ac
cmd/compile/internal/gc/const.go:1515: moved to heap: bd
cmd/compile/internal/gc/const.go:1516: moved to heap: bc
cmd/compile/internal/gc/const.go:1517: moved to heap: ad
cmd/compile/internal/gc/const.go:1546: moved to heap: ac
cmd/compile/internal/gc/const.go:1547: moved to heap: bd
cmd/compile/internal/gc/const.go:1548: moved to heap: bc
cmd/compile/internal/gc/const.go:1549: moved to heap: ad
cmd/compile/internal/gc/const.go:1550: moved to heap: cc_plus
cmd/compile/internal/gc/export.go:162: moved to heap: copy
cmd/compile/internal/gc/mpfloat.go:66: moved to heap: b
cmd/compile/internal/gc/mpfloat.go:97: moved to heap: b
Change-Id: I0d420b69c84a41ba9968c394e8957910bab5edea
Reviewed-on: https://go-review.googlesource.com/37508
Reviewed-by: David Chase <drchase@google.com>
Keep liveness bit vectors as simple live-variable vectors during
liveness analysis. We can defer expanding them into runtime heap
bitmaps until we're actually writing out the symbol data, and then we
only need temporary memory to expand one bitmap at a time.
This is logically cleaner (e.g., we no longer depend on stack frame
layout during analysis) and saves a little bit on allocations.
name old alloc/op new alloc/op delta
Template 41.4MB ± 0% 41.3MB ± 0% -0.28% (p=0.000 n=60+60)
Unicode 32.6MB ± 0% 32.6MB ± 0% -0.11% (p=0.000 n=59+60)
GoTypes 119MB ± 0% 119MB ± 0% -0.35% (p=0.000 n=60+59)
Compiler 483MB ± 0% 481MB ± 0% -0.47% (p=0.000 n=59+60)
name old allocs/op new allocs/op delta
Template 381k ± 1% 380k ± 1% -0.32% (p=0.000 n=60+60)
Unicode 325k ± 1% 325k ± 1% ~ (p=0.867 n=60+60)
GoTypes 1.16M ± 0% 1.15M ± 0% -0.40% (p=0.000 n=60+59)
Compiler 4.22M ± 0% 4.19M ± 0% -0.61% (p=0.000 n=59+60)
Passes toolstash -cmp.
Change-Id: I8175efe55201ffb5017f79ae6cb90df03f1b7e99
Reviewed-on: https://go-review.googlesource.com/37458
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Constant evaluation provides some rudimentary
knowledge of dead code at inlining decision time.
Use it.
This CL addresses only dead code inside if statements.
For statements are never inlined anyway,
and dead code inside for statements is rare.
Analyzing switch statements is worth doing,
but it is more complicated, since we would have
to evaluate each case; leave it for later.
Fixes#9274
After this CL, the following functions in std+cmd
can be newly inlined:
cmd/internal/obj/x86/asm6.go:3122: can inline subreg
cmd/vendor/golang.org/x/arch/x86/x86asm/decode.go:172: can inline instPrefix
cmd/vendor/golang.org/x/arch/x86/x86asm/decode.go:202: can inline truncated
go/constant/value.go:234: can inline makeFloat
go/types/labels.go:52: can inline (*block).insert
math/big/float.go:231: can inline (*Float).Sign
math/bits/bits.go:57: can inline OnesCount
net/http/server.go:597: can inline (*Server).newConn
runtime/hashmap.go:1165: can inline reflect_maplen
runtime/proc.go:207: can inline os_beforeExit
runtime/signal_unix.go:55: can inline init.5
runtime/stack.go:1081: can inline gostartcallfn
Change-Id: I4c92fb96aa0c3d33df7b3f2da548612e79b56b5b
Reviewed-on: https://go-review.googlesource.com/37499
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Follow-up to CL 37270.
This considerably reduces the time to run the test.
Before:
real 0m7.638s
user 0m14.341s
sys 0m2.244s
After:
real 0m4.867s
user 0m7.107s
sys 0m1.842s
Change-Id: I8837a5da0979a1c365e1ce5874d81708249a4129
Reviewed-on: https://go-review.googlesource.com/37461
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Munday <munday@ca.ibm.com>
New special case for booleans and byte-sized integer types
converted to interfaces needs to ensure that the operand is
not too complex, if it were to appear in a parameter list
for example.
Added test, also increased the recursive node dump depth to
a level that was actually useful for an actual bug.
Fixes#19275.
Change-Id: If36ac3115edf439e886703f32d149ee0a46eb2a5
Reviewed-on: https://go-review.googlesource.com/37470
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Josh Bleecher Snyder
Robert Griesemer
Austin Clements
Todd Neal
Dave Cheney
David Chase
Matthew Dempsky
Keith Randall
Michael Munday
Damien Lespiau
Alberto Donizetti
Lynn Boger
Daniel Martí
Martin Möhrmann
Alessandro Arzilli
Ilya Tocar
Cherry Zhang
Keith Randall
Eric Lagergren
Brad Fitzpatrick
Emmanuel Odeke
Ben Shi
David Lazar
Alexandru Moșoi
Carlos Eduardo Seo
Vladimir Stefanovic
Hugues Bruant
philhofer
Jeremy Jackins
Michael Stapelberg
khr
Aliaksandr Valialkin
Eitan Adler
Giovanni Bajo
Heschi Kreinick