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>
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>
The `skip` argument passed to runtime.Caller and runtime.Callers should
be interpreted as the number of logical calls to skip (rather than the
number of physical stack frames to skip). This changes runtime.Callers
to skip inlined calls in addition to physical stack frames.
The result value of runtime.Callers is a slice of program counters
([]uintptr) representing physical stack frames. If the `skip` parameter
to runtime.Callers skips part-way into a physical frame, there is no
convenient way to encode that in the resulting slice. To avoid changing
the API in an incompatible way, our solution is to store the number of
skipped logical calls of the first frame in the _second_ uintptr
returned by runtime.Callers. Since this number is a small integer, we
encode it as a valid PC value into a small symbol called:
runtime.skipPleaseUseCallersFrames
For example, if f() calls g(), g() calls `runtime.Callers(2, pcs)`, and
g() is inlined into f, then the frame for f will be partially skipped,
resulting in the following slice:
pcs = []uintptr{pc_in_f, runtime.skipPleaseUseCallersFrames+1, ...}
We store the skip PC in pcs[1] instead of pcs[0] so that `pcs[i:]` will
truncate the captured stack trace rather than grow it for all i.
Updates #19348.
Change-Id: I1c56f89ac48c29e6f52a5d085567c6d77d499cf1
Reviewed-on: https://go-review.googlesource.com/37854
Run-TryBot: David Lazar <lazard@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Previously, we could not run tests with -l=4 on NaCl since the buildrun
action is not supported on NaCl. This lets us run tests with build flags
on NaCl.
Change-Id: I103370c7b823b4ff46f47df97e802da0dc2bc7c3
Reviewed-on: https://go-review.googlesource.com/38170
Run-TryBot: David Lazar <lazard@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Gccgo crashed compiling a function that returned multiple zero-sized values.
Change-Id: I499112cc310e4a4f649962f4d2bc9fee95dee1b6
Reviewed-on: https://go-review.googlesource.com/38772
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This was cherry-picked to 1.8 as CL 38587, but on master issue was fixed
by CL 37661. Add still relevant part (test) and close issue, since test passes.
Fixes#19201
Change-Id: I6415792e2c465dc6d9bd6583ba1e54b107bcf5cc
Reviewed-on: https://go-review.googlesource.com/37376
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@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>
Ensure that we have a test for when the compiler
encounters a type switch on a non-interface value.
Change-Id: Icb222f986894d0190e1241ca65396b4950e7d14f
Reviewed-on: https://go-review.googlesource.com/38661
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
On overloaded machines once we get to big N, the machine slowness dominates.
But we only retry once we get to a big N.
Instead, retry for small N too, and die on the first big N that fails.
Change-Id: I3ab9cfb88832ad86e2ba1389a926045091268aeb
Reviewed-on: https://go-review.googlesource.com/37543
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@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>
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>
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>
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>
