1
0
mirror of https://github.com/golang/go synced 2024-11-23 19:40:08 -07:00
Commit Graph

1222 Commits

Author SHA1 Message Date
Josh Bleecher Snyder
92363d52c0 cmd/compile: check width of embedded interfaces in expandiface
The code in #20162 contains an embedded interface.

It didn't get dowidth'd by the frontend,
and during DWARF generation, ngotype asked
for a string description of it,
which triggered a request for the number of fields
in the interface, which triggered a dowidth,
which is disallowed in the backend.

The other changes in this CL are to support the test.

Fixes #20162

Change-Id: I4d0be5bd949c361d4cdc89a8ed28b10977e40cf9
Reviewed-on: https://go-review.googlesource.com/42131
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-04-28 20:07:38 +00:00
Josh Bleecher Snyder
0b6a10ef24 cmd/compile: dowidth more in the front end
dowidth is fundamentally unsafe to call from the back end;
it will cause data races.

Replace all calls to dowidth in the backend with
assertions that the width has been calculated.

Then fix all the cases in which that was not so,
including the cases from #20145.

Fixes #20145.

Change-Id: Idba3d19d75638851a30ec2ebcdb703c19da3e92b
Reviewed-on: https://go-review.googlesource.com/41970
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-04-27 22:10:32 +00:00
Keith Randall
38dee12dea cmd/compile: zero ambiguously live variables at VARKILLs
At VARKILLs, zero a variable if it is ambiguously live.
After the VARKILL anything this variable references
might be collected. If it were to become live again later,
the GC will see references to already-collected objects.

We don't know a variable is ambiguously live until very
late in compilation (after lowering, register allocation, ...),
so it is hard to generate the code in an arch-independent way.
We also have to be careful not to clobber any registers.
Fortunately, this almost never happens so performance is ~irrelevant.

There are only 2 instances where this triggers in the stdlib.

Fixes #20029

Change-Id: Ia9585a91d7b823fad4a9d141d954464cc7af31f4
Reviewed-on: https://go-review.googlesource.com/41076
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2017-04-20 23:47:43 +00:00
Alberto Donizetti
3218b1aa6f cmd/compile: only print one error for bad-type literal in assignment
Fixes #8438

Change-Id: Ib43cdcdc962a8d9e14faf984bc859a92ba1eb517
Reviewed-on: https://go-review.googlesource.com/40531
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-04-20 22:21:55 +00:00
Robert Griesemer
912a638b0c cmd/compile: check labels and branches during parse time
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>
2017-04-19 00:36:34 +00:00
Josh Bleecher Snyder
dffe5ac9f2 cmd/compile: eliminate dead code in if statements after typechecking
This is a more thorough and cleaner fix
than doing dead code elimination separately
during inlining, escape analysis, and export.

Unfortunately, it does add another full walk of the AST.
The performance impact is very small, but not non-zero.

If a label or goto is present in the dead code, it is not eliminated.
This restriction can be removed once label/goto checking occurs
much earlier in the compiler. In practice, it probably doesn't
matter much.

Updates #19699
Fixes #19705

name       old alloc/op      new alloc/op      delta
Template        39.2MB ± 0%       39.3MB ± 0%  +0.28%  (p=0.008 n=5+5)
Unicode         29.8MB ± 0%       29.8MB ± 0%    ~     (p=1.000 n=5+5)
GoTypes          113MB ± 0%        113MB ± 0%  -0.55%  (p=0.008 n=5+5)
SSA             1.25GB ± 0%       1.25GB ± 0%  +0.02%  (p=0.008 n=5+5)
Flate           25.3MB ± 0%       25.3MB ± 0%  -0.24%  (p=0.032 n=5+5)
GoParser        31.7MB ± 0%       31.8MB ± 0%  +0.31%  (p=0.008 n=5+5)
Reflect         78.2MB ± 0%       78.3MB ± 0%    ~     (p=0.421 n=5+5)
Tar             26.6MB ± 0%       26.7MB ± 0%  +0.21%  (p=0.008 n=5+5)
XML             42.2MB ± 0%       42.2MB ± 0%    ~     (p=0.056 n=5+5)

name       old allocs/op     new allocs/op     delta
Template          385k ± 0%         387k ± 0%  +0.51%  (p=0.016 n=5+5)
Unicode           321k ± 0%         321k ± 0%    ~     (p=1.000 n=5+5)
GoTypes          1.14M ± 0%        1.14M ± 0%    ~     (p=1.000 n=5+5)
SSA              9.71M ± 0%        9.72M ± 0%  +0.10%  (p=0.008 n=5+5)
Flate             234k ± 1%         234k ± 1%    ~     (p=0.690 n=5+5)
GoParser          315k ± 0%         317k ± 0%  +0.71%  (p=0.008 n=5+5)
Reflect           980k ± 0%         983k ± 0%  +0.30%  (p=0.032 n=5+5)
Tar               251k ± 0%         252k ± 0%  +0.55%  (p=0.016 n=5+5)
XML               392k ± 0%         393k ± 0%  +0.30%  (p=0.008 n=5+5)

Change-Id: Ia10ff4bbf5c6eae782582cc9cbc9785494d4fb83
Reviewed-on: https://go-review.googlesource.com/38773
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-04-18 17:13:30 +00:00
Alberto Donizetti
6266b0f08f cmd/compile: add test for Issue 12536
The fixedbugs/issue12536.go file was erroneously deleted just before
committing the patch that fixed the issue (CL 14400).

That's an easy test and there's a small reproducer in the issue, add
it back.

Updates #12536

Change-Id: Ib7b0cd245588299e9a5469e1d75805fd0261ce1a
Reviewed-on: https://go-review.googlesource.com/40712
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-04-14 14:16:09 +00:00
Brad Fitzpatrick
dc09a7af2e test: disable flaky test/fixedbugs/issue10958.go
Updates #18589

Change-Id: I2c3bbc8257c68295051bd2e63e1e11794d0609c3
Reviewed-on: https://go-review.googlesource.com/40651
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-04-13 18:28:29 +00:00
Alberto Donizetti
2e60882fc7 cmd/compile: do not print duplicate error on ideal->float{32,64} overflow
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>
2017-04-12 21:00:15 +00:00
Alberto Donizetti
10a200e560 cmd/compile: diagnose constant division by complex zero
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>
2017-04-12 17:29:28 +00:00
Josh Bleecher Snyder
423e7e6037 cmd/compile: skip array bounds errors when type is broken
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>
2017-04-10 20:57:08 +00:00
Josh Bleecher Snyder
b83a916f71 cmd/compile: make iface == iface const evaluation respect !=
Fixes #19911

Change-Id: Ib2b2505fe31ce00c6ffc021a0fe5df510633b44b
Reviewed-on: https://go-review.googlesource.com/40251
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-04-10 20:05:56 +00:00
Todd Neal
0d33dc3105 runtime: improve output of panic(x) where x is numeric
Fixes #19658

Change-Id: I41e46073b75c7674e2ed9d6a90ece367ce92166b
Reviewed-on: https://go-review.googlesource.com/39650
Run-TryBot: Todd Neal <todd@tneal.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-04-09 22:40:33 +00:00
Josh Bleecher Snyder
c9446398e8 cmd/compile: allow composite literal structs with _ fields
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>
2017-04-07 22:01:18 +00:00
Cherry Zhang
a1cedf0842 cmd/link: canonicalize the "package" of dupok text symbols
Dupok symbols may be defined in multiple packages. Its associated
package is chosen sort of arbitrarily (the first containing package
that the linker loads). Canonicalize its package to the package
with which it will be laid down in text, which is the first package
in dependency order that defines the symbol. So later passes (for
example, trampoline insertion pass) know that the dupok symbol
is laid down along with the package.

Fixes #19764.

Change-Id: I7cbc7474ff3016d5069c8b7be04af934abab8bc3
Reviewed-on: https://go-review.googlesource.com/39150
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: David Chase <drchase@google.com>
2017-04-02 03:25:02 +00:00
Josh Bleecher Snyder
5272a2cdc5 cmd/compile: avoid infinite loops in dead blocks during phi insertion
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>
2017-03-30 17:06:08 +00:00
David Chase
24e94766c0 cmd/compile: added special case for reflect header fields to esc
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>
2017-03-29 17:39:59 +00:00
David Lazar
83843b1610 cmd/compile: fix names of inlined methods from other packages
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>
2017-03-29 17:27:49 +00:00
Ian Lance Taylor
835b17c85f test: add test for gccgo compiler crash
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>
2017-03-28 20:05:34 +00:00
Ilya Tocar
4b50c81356 test/fixedbugs: add a test for 19201
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>
2017-03-28 19:07:23 +00:00
Josh Bleecher Snyder
8dafdb1be1 cmd/compile: add Type.WidthCalculated
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>
2017-03-28 18:06:09 +00:00
Robert Griesemer
5e954047bc cmd/compile: be slightly more tolerant in case of certain syntax errors
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>
2017-03-24 20:07:15 +00:00
Josh Bleecher Snyder
e00e57d67c cmd/compile: ignore all unreachable values during simple phi insertion
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>
2017-03-24 18:00:15 +00:00
Josh Bleecher Snyder
ad8c17b703 cmd/compile: don't export dead code in inlineable fuctions
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>
2017-03-24 17:21:05 +00:00
Josh Bleecher Snyder
7202341de9 cmd/compile: only SSA [0]T when T is SSA-able
Almost never happens in practice.
The compiler will generate reasonable code anyway,
since assignments involving [0]T never do any work.

Fixes #19696
Fixes #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>
2017-03-24 16:53:22 +00:00
Robert Griesemer
b5e1ae46ad cmd/compile: don't crash when reporting some syntax errors
Fixes #19667.

Change-Id: Iaa71e2020af123c1bd3ac25e0b760956688e8bdf
Reviewed-on: https://go-review.googlesource.com/38458
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-03-23 04:54:27 +00:00
Matthew Dempsky
051cbf3f37 cmd/compile: add regress test for issue 19632
Updates #19632.

Change-Id: I1411dd997c8c6a789d17d0dcc0bfbd2281447b16
Reviewed-on: https://go-review.googlesource.com/38401
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-03-21 19:52:45 +00:00
Matthew Dempsky
ee272bbf36 cmd/compile/internal/gc: export interface embedding information
Fixes #16369.

Change-Id: I23f8c36370d0da37ac5b5126d012d22f78782782
Reviewed-on: https://go-review.googlesource.com/38392
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-03-21 02:35:40 +00:00
Matthew Dempsky
07de3465be cmd/compile/internal/gc: handle recursive interfaces better
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>
2017-03-21 01:56:25 +00:00
Robert Griesemer
422c7fea70 cmd/compile: don't permit declarations in post statement of for loop
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>
2017-03-20 20:02:34 +00:00
Robert Griesemer
3c7a812485 cmd/compile: eliminate "assignment count mismatch" - not needed anymore
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>
2017-03-17 00:31:35 +00:00
Josh Bleecher Snyder
604455a46c cmd/compile: ensure TESTQconst AuxInt is in range
Fixes #19555

Change-Id: I7aa0551a90f6bb630c0ba721f3525a8a9cf793fd
Reviewed-on: https://go-review.googlesource.com/38164
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2017-03-15 21:44:08 +00:00
Philip Hofer
710f4d3e7e cmd/compile/internal/gc: mark generated wrappers as DUPOK
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 #19548
Fixes #19550

Change-Id: Ibe74adb5a62dbf6447db37fde22dcbb3479969ef
Reviewed-on: https://go-review.googlesource.com/38156
Reviewed-by: David Chase <drchase@google.com>
2017-03-15 00:27:13 +00:00
Cherry Zhang
8a44c8efae cmd/compile: don't spill rematerializeable value when resolving merge edges
Fixes #19515.

Change-Id: I4bcce152cef52d00fbb5ab4daf72a6e742bae27c
Reviewed-on: https://go-review.googlesource.com/38158
Reviewed-by: Keith Randall <khr@golang.org>
2017-03-14 22:55:52 +00:00
khr
a51e4cc9ce cmd/compile: zero return parameters earlier
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>
2017-03-13 19:39:15 +00:00
David Chase
d71f36b5aa cmd/compile: check loop rescheduling with stack bound, not counter
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>
2017-03-08 18:52:12 +00:00
Alberto Donizetti
e99dafc4a8 cmd/compile: fix misleading "truncated to int" messages
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>
2017-03-07 19:34:22 +00:00
Robert Griesemer
cf710949a9 Revert "cmd/compile: improve error message if init is directly invoked"
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>
2017-03-06 23:48:37 +00:00
Quentin Smith
4b261a1410 test/fixedbugs: add test for #19403
Change-Id: Ie52dac8eb4daed95e049ad74d5ae101e8a5cb854
Reviewed-on: https://go-review.googlesource.com/37725
Run-TryBot: Quentin Smith <quentin@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-03-06 21:39:00 +00:00
Josh Bleecher Snyder
04fc887761 runtime: delay marking maps as writing until after first alg call
Fixes #19359

Change-Id: I196b47cf0471915b6dc63785e8542aa1876ff695
Reviewed-on: https://go-review.googlesource.com/37665
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2017-03-02 17:38:30 +00:00
Matthew Dempsky
0ee9c46cb1 cmd/compile: add missing WBs for reflect.{Slice,String}Header.Data
Fixes #19168.

Change-Id: I3f4fcc0b189c53819ac29ef8de86fdad76a17488
Reviewed-on: https://go-review.googlesource.com/37663
Reviewed-by: Keith Randall <khr@golang.org>
2017-03-02 17:21:50 +00:00
Josh Bleecher Snyder
542a60fbde cmd/compile: don't crash when slicing non-slice
Fixes #19323

Change-Id: I92d1bdefb15de6178a577a4fa0f0dc004f791904
Reviewed-on: https://go-review.googlesource.com/37584
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-03-02 15:48:19 +00:00
David Chase
febafe60d4 cmd/compile: added cheapexpr call to simplify operand of CONVIFACE
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>
2017-02-25 04:53:23 +00:00
Josh Bleecher Snyder
d9270ecb3a cmd/compile: evaluate zero-sized values converted to interfaces
CL 35562 substituted zerobase for the pointer for
interfaces containing zero-sized values.
However, it failed to evaluate the zero-sized value
expression for side-effects. Fix that.

The other similar interface value optimizations
are not affected, because they all actually use the
value one way or another.

Fixes #19246

Change-Id: I1168a99561477c63c29751d5cd04cf81b5ea509d
Reviewed-on: https://go-review.googlesource.com/37395
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-02-24 19:09:41 +00:00
David Chase
abdb2c35b6 cmd/compile: repaired loop-finder to handle trickier nesting
The loop-A-encloses-loop-C code did not properly handle the
case where really C was already known to be enclosed by B,
and A was nearest-outer to B, not C.

Fixes #19217.

Change-Id: I755dd768e823cb707abdc5302fed39c11cdb34d4
Reviewed-on: https://go-review.googlesource.com/37340
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-02-23 22:28:44 +00:00
David R. Jenni
d55f528826 cmd/compile: silence superfluous assignment error message
Avoid printing a second error message when a field of an undefined
variable is accessed.

Fixes #8440.

Change-Id: I3fe0b11fa3423cec3871cb01b5951efa8ea7451a
Reviewed-on: https://go-review.googlesource.com/36751
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-02-23 21:06:11 +00:00
Emmanuel Odeke
19d2061d50 cmd/compile: suppress callsite signatures if any type is unknown
Fixes #19012.

Fallback to return signatures without detailed types.
These error message will be of the form of issue:
* https://golang.org/issues/4215
* https://golang.org/issues/6750

So:
func f(x int, y uint) {
    return x > y
}

f(10, "a" < 3)

will give errors:
too many errors to return
too many arguments in call to f

instead of:

too many errors to return
  have (<T>)
  want ()
too many arguments in call to f
  have (number, <T>)
  want (number, number)

Change-Id: I680abc7cdd8444400e234caddf3ff49c2d69f53d
Reviewed-on: https://go-review.googlesource.com/36806
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-02-22 17:55:45 +00:00
David Chase
11b283092a cmd/compile: add opcode flag hasSideEffects for do-not-remove
Added a flag to generic and various architectures' atomic
operations that are judged to have observable side effects
and thus cannot be dead-code-eliminated.

Test requires GOMAXPROCS > 1 without preemption in loop.

Fixes #19182.

Change-Id: Id2230031abd2cca0bbb32fd68fc8a58fb912070f
Reviewed-on: https://go-review.googlesource.com/37333
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-02-22 15:15:47 +00:00
Cherry Zhang
6464e5dc4b cmd/compile: do not fold offset into load/store for args on ARM64
Args may be not at 8-byte aligned offset to SP. When the stack
frame is large, folding the offset of args may cause large
unaligned offsets that does not fit in a machine instruction on
ARM64. Therefore disable folding offsets for args.

This has small performance impact (see below). A better fix would
be letting the assembler backend fix up the offset by loading it
into a register if it doesn't fit into an instruction. And the
compiler can simply generate large load/stores with offset. Since
in most of the cases the offset is aligned or the stack frame is
small, it can fit in an instruction and no fixup is needed. But
this is too complicated for Go 1.8.

name                     old time/op    new time/op    delta
BinaryTree17-8              8.30s ± 0%     8.31s ± 0%    ~     (p=0.579 n=10+10)
Fannkuch11-8                6.14s ± 0%     6.18s ± 0%  +0.53%  (p=0.000 n=9+10)
FmtFprintfEmpty-8           117ns ± 0%     117ns ± 0%    ~     (all equal)
FmtFprintfString-8          196ns ± 0%     197ns ± 0%  +0.72%  (p=0.000 n=10+10)
FmtFprintfInt-8             204ns ± 0%     205ns ± 0%  +0.49%  (p=0.000 n=9+10)
FmtFprintfIntInt-8          302ns ± 0%     307ns ± 1%  +1.46%  (p=0.000 n=10+10)
FmtFprintfPrefixedInt-8     329ns ± 2%     326ns ± 0%    ~     (p=0.083 n=10+10)
FmtFprintfFloat-8           540ns ± 0%     542ns ± 0%  +0.46%  (p=0.000 n=8+7)
FmtManyArgs-8              1.20µs ± 1%    1.19µs ± 1%  -1.02%  (p=0.000 n=10+10)
GobDecode-8                17.3ms ± 1%    17.8ms ± 0%  +2.75%  (p=0.000 n=10+7)
GobEncode-8                15.3ms ± 1%    15.4ms ± 0%  +0.57%  (p=0.004 n=9+10)
Gzip-8                      789ms ± 0%     803ms ± 0%  +1.78%  (p=0.000 n=9+10)
Gunzip-8                    128ms ± 0%     130ms ± 0%  +1.73%  (p=0.000 n=10+9)
HTTPClientServer-8          202µs ± 6%     201µs ±10%    ~     (p=0.739 n=10+10)
JSONEncode-8               42.0ms ± 0%    42.1ms ± 0%  +0.19%  (p=0.028 n=10+9)
JSONDecode-8                159ms ± 0%     161ms ± 0%  +1.05%  (p=0.000 n=9+10)
Mandelbrot200-8            10.1ms ± 0%    10.1ms ± 0%  -0.07%  (p=0.000 n=10+9)
GoParse-8                  8.46ms ± 1%    8.61ms ± 1%  +1.77%  (p=0.000 n=10+10)
RegexpMatchEasy0_32-8       227ns ± 1%     226ns ± 0%  -0.35%  (p=0.001 n=10+9)
RegexpMatchEasy0_1K-8      1.63µs ± 0%    1.63µs ± 0%  -0.13%  (p=0.000 n=10+9)
RegexpMatchEasy1_32-8       250ns ± 0%     249ns ± 0%  -0.40%  (p=0.001 n=8+9)
RegexpMatchEasy1_1K-8      2.07µs ± 0%    2.08µs ± 0%  +0.05%  (p=0.027 n=9+9)
RegexpMatchMedium_32-8      350ns ± 0%     350ns ± 0%    ~     (p=0.412 n=9+8)
RegexpMatchMedium_1K-8      104µs ± 0%     104µs ± 0%  +0.31%  (p=0.000 n=10+7)
RegexpMatchHard_32-8       5.82µs ± 0%    5.82µs ± 0%    ~     (p=0.937 n=9+9)
RegexpMatchHard_1K-8        176µs ± 0%     176µs ± 0%  +0.03%  (p=0.000 n=9+8)
Revcomp-8                   1.36s ± 1%     1.37s ± 1%    ~     (p=0.218 n=10+10)
Template-8                  151ms ± 1%     156ms ± 1%  +3.21%  (p=0.000 n=10+10)
TimeParse-8                 737ns ± 0%     758ns ± 2%  +2.74%  (p=0.000 n=10+10)
TimeFormat-8                801ns ± 2%     789ns ± 1%  -1.51%  (p=0.000 n=10+10)
[Geo mean]                  142µs          143µs       +0.50%

Fixes #19137.

Change-Id: Ib8a21ea98c0ffb2d282a586535b213cc163e1b67
Reviewed-on: https://go-review.googlesource.com/37251
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2017-02-21 19:39:08 +00:00
Cherry Zhang
3557d54609 cmd/compile: check both syms when folding address into load/store on ARM64
The rules for folding addresses into load/stores checks sym1 is
not on stack (because the stack offset is not known at that point).
But sym1 could be nil, which invalidates the check. Check merged
sym instead.

Fixes #19137.

Change-Id: I8574da22ced1216bb5850403d8f08ec60a8d1005
Reviewed-on: https://go-review.googlesource.com/37145
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2017-02-17 21:23:24 +00:00