1
0
mirror of https://github.com/golang/go synced 2024-09-25 15:10:11 -06:00
Commit Graph

1365 Commits

Author SHA1 Message Date
Martin Möhrmann
a8a60ac2a7 cmd/compile: optimize append(x, make([]T, y)...) slice extension
Changes the compiler to recognize the slice extension pattern

  append(x, make([]T, y)...)

and replace it with growslice and an optional memclr to avoid an allocation for make([]T, y).

Memclr is not called in case growslice already allocated a new cleared backing array
when T contains pointers.

amd64:
name                      old time/op    new time/op    delta
ExtendSlice/IntSlice         103ns ± 4%      57ns ± 4%   -44.55%  (p=0.000 n=18+18)
ExtendSlice/PointerSlice     155ns ± 3%      77ns ± 3%   -49.93%  (p=0.000 n=20+20)
ExtendSlice/NoGrow          50.2ns ± 3%     5.2ns ± 2%   -89.67%  (p=0.000 n=18+18)

name                      old alloc/op   new alloc/op   delta
ExtendSlice/IntSlice         64.0B ± 0%     32.0B ± 0%   -50.00%  (p=0.000 n=20+20)
ExtendSlice/PointerSlice     64.0B ± 0%     32.0B ± 0%   -50.00%  (p=0.000 n=20+20)
ExtendSlice/NoGrow           32.0B ± 0%      0.0B       -100.00%  (p=0.000 n=20+20)

name                      old allocs/op  new allocs/op  delta
ExtendSlice/IntSlice          2.00 ± 0%      1.00 ± 0%   -50.00%  (p=0.000 n=20+20)
ExtendSlice/PointerSlice      2.00 ± 0%      1.00 ± 0%   -50.00%  (p=0.000 n=20+20)
ExtendSlice/NoGrow            1.00 ± 0%      0.00       -100.00%  (p=0.000 n=20+20)

Fixes #21266

Change-Id: Idc3077665f63cbe89762b590c5967a864fd1c07f
Reviewed-on: https://go-review.googlesource.com/109517
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2018-05-06 04:28:23 +00:00
Richard Musiol
e3c684777a all: skip unsupported tests for js/wasm
The general policy for the current state of js/wasm is that it only
has to support tests that are also supported by nacl.

The test nilptr3.go makes assumptions about which nil checks can be
removed. Since WebAssembly does not signal on reading a null pointer,
all nil checks have to be explicit.

Updates #18892

Change-Id: I06a687860b8d22ae26b1c391499c0f5183e4c485
Reviewed-on: https://go-review.googlesource.com/110096
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-04-30 19:39:18 +00:00
Ilya Tocar
fb017c60bc cmd/compile/internal/ssa: fix endless compile loop on AMD64
We currently rewrite
(TESTQ (MOVQconst [c] x)) into (TESTQconst [c] x)
and (TESTQconst [-1] x) into (TESTQ x x)
if x is a (MOVQconst [-1]) we will be stuck in the endless rewrite loop.
Don't perform the rewrite in such cases.

Fixes #25006

Change-Id: I77f561ba2605fc104f1e5d5c57f32e9d67a2c000
Reviewed-on: https://go-review.googlesource.com/108879
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2018-04-24 16:20:41 +00:00
Josh Bleecher Snyder
566e3e074c cmd/compile: avoid runtime call during switch string(byteslice)
This triggers three times while building std,
once in image/png and twice in go/internal/gccgoimporter.

There are no instances in std in which a more aggressive
optimization would have triggered.

This doesn't necessarily avoid an allocation,
because escape analysis is already able in many cases
to use a temporary backing for the string,
but it does at a minimum avoid the runtime call and copy.

Fixes #24937

Change-Id: I7019e85638ba8cd7e2f03890e672558b858579bc
Reviewed-on: https://go-review.googlesource.com/108035
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-04-21 00:50:50 +00:00
Lynn Boger
30311e8860 cmd/compile: generate load without DS relocation for go.string on ppc64le
Due to some recent optimizations related to the compare
instruction, DS-form load instructions started to be used
to load 8-byte go.strings. This can cause link time errors
if the go.string is not aligned to 4 bytes.

For DS-form instructions, the value in the offset field must
be a multiple of 4. If the offset is known at the time the
rules are processed, a DS-form load will not be chosen. But for
go.strings, the offset is not known at that time, but a
relocation is generated indicating that the linker should fill
in the DS relocation. When the linker tries to fill in the
relocation, if the offset is not aligned properly, a link error
will occur.

To fix this, when loading a go.string using MOVDload, the full
address of the go.string is generated and loaded into the base
register. Then the go.string is loaded with a 0 offset field.

Added a testcase that reproduces this problem.

Fixes #24799

Change-Id: I6a154e8e1cba64eae290be0fbcb608b75884ecdd
Reviewed-on: https://go-review.googlesource.com/107855
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2018-04-20 16:16:47 +00:00
Josh Bleecher Snyder
c1ed1f3c80 cmd/compile: fix evaluation of "" < s
Fixes #24817

Change-Id: Ifa79ab3dfe69297eeef85f7193cd5f85e5982bc5
Reviewed-on: https://go-review.googlesource.com/106655
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-04-12 19:38:37 +00:00
Josh Bleecher Snyder
2dfb423e6e cmd/compile: loop to ensure all autogenerated functions are compiled
I was wrong. There was a need to loop here.

Fixes #24761

Change-Id: If13b3ab72febde930bdaebdddd1c05e0d0446020
Reviewed-on: https://go-review.googlesource.com/105615
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-04-11 23:46:30 +00:00
Robert Griesemer
3d501df441 cmd/compile: better error message when referring to ambiguous method/field
Fixes #14321.

Change-Id: I9c92c767b01cf7938c4808a8fef9f2936fc667bc
Reviewed-on: https://go-review.googlesource.com/106119
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-04-10 23:39:13 +00:00
Matthew Dempsky
535ad8efb8 cmd/compile: fix check that ensures main.main is a function
The check was previously disallowing package main from even importing
a non-function symbol named "main".

Fixes #24801.

Change-Id: I849b9713890429f0a16860ef16b5dc7e970d04a4
Reviewed-on: https://go-review.googlesource.com/106120
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2018-04-10 23:34:12 +00:00
Matthew Dempsky
49ed4cbe85 cmd/compile: sort method sets using package height
Also, when statically building itabs, compare *types.Sym instead of
name alone so that method sets with duplicate non-exported methods are
handled correctly.

Fixes #24693.

Change-Id: I2db8a3d6e80991a71fef5586a15134b6de116269
Reviewed-on: https://go-review.googlesource.com/105039
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2018-04-10 00:06:06 +00:00
Matthew Dempsky
fe77a5413e cmd/compile: fix constant pointer comparison failure
Previously, constant pointer-typed expressions could use either Mpint
or NilVal as their Val depending on their construction, but const.go
expects each type to have a single corresponding Val kind.

This CL changes pointer-typed expressions to exclusively use Mpint.

Fixes #21221.

Change-Id: I6ba36c9b11eb19a68306f0b296acb11a8c254c41
Reviewed-on: https://go-review.googlesource.com/105315
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2018-04-09 23:19:45 +00:00
Robert Griesemer
4637699e92 cmd/compile/internal/syntax: better error message for incorrect if/switch header
Fixes #23664.

Change-Id: Ic0637e9f896b2fc6502dfbab2d1c4de3c62c0bd2
Reviewed-on: https://go-review.googlesource.com/104616
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
2018-04-03 21:57:37 +00:00
isharipo
dcaf3fb134 cmd/compile: make DCE remove nodes after terminating if
This change makes compiler frontend dead code elimination of const expr if
statements introduced in https://golang.org/cl/38773 treat both
	if constCondTrue { ...; returnStmt } toBeRemoved...
	if constCondFalse { ...; } else { returnStmt } toBeRemoved...
identically to:
	if constCondTrue { ...; returnStmt } else { toBeRemoved... }

Where "constCondTrue" is a an expression that can be evaluated
to "true" during compile time.

The additional checks are only triggered for const expr
if conditions that evaluate to true.

name       old time/op       new time/op       delta
Template         431ms ± 2%        429ms ± 1%    ~     (p=0.491 n=8+6)
Unicode          198ms ± 4%        201ms ± 2%    ~     (p=0.234 n=7+6)
GoTypes          1.40s ± 1%        1.41s ± 2%    ~     (p=0.053 n=7+7)
Compiler         6.72s ± 2%        6.81s ± 1%  +1.35%  (p=0.011 n=7+7)
SSA              17.3s ± 1%        17.3s ± 2%    ~     (p=0.731 n=6+7)
Flate            275ms ± 2%        275ms ± 2%    ~     (p=0.902 n=7+7)
GoParser         340ms ± 2%        339ms ± 2%    ~     (p=0.902 n=7+7)
Reflect          910ms ± 2%        905ms ± 1%    ~     (p=0.310 n=6+6)
Tar              403ms ± 1%        403ms ± 2%    ~     (p=0.366 n=7+6)
XML              486ms ± 1%        490ms ± 1%    ~     (p=0.065 n=6+6)
StdCmd           56.2s ± 1%        56.6s ± 2%    ~     (p=0.620 n=7+7)

name       old user-time/op  new user-time/op  delta
Template         559ms ± 8%        557ms ± 7%    ~     (p=0.713 n=8+7)
Unicode          266ms ±13%        277ms ± 9%    ~     (p=0.157 n=8+7)
GoTypes          1.83s ± 2%        1.84s ± 1%    ~     (p=0.522 n=8+7)
Compiler         8.67s ± 4%        8.89s ± 4%    ~     (p=0.077 n=7+7)
SSA              23.9s ± 1%        24.2s ± 1%  +1.31%  (p=0.005 n=7+7)
Flate            351ms ± 4%        342ms ± 5%    ~     (p=0.105 n=7+7)
GoParser         437ms ± 2%        423ms ± 5%  -3.14%  (p=0.016 n=7+7)
Reflect          1.16s ± 3%        1.15s ± 2%    ~     (p=0.362 n=7+7)
Tar              517ms ± 4%        511ms ± 3%    ~     (p=0.538 n=7+7)
XML              619ms ± 3%        617ms ± 4%    ~     (p=0.483 n=7+7)

Fixes #23521

Change-Id: I165a7827d869aeb93ce6047d026ff873d039a4f3
Reviewed-on: https://go-review.googlesource.com/91056
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2018-04-03 15:19:41 +00:00
Robert Griesemer
c65a2781be cmd/compile: better handling of incorrect type switches
Don't report errors if we don't have a correct type switch
guard; instead ignore it and leave it to the type-checker
to report the error. This leads to better error messages
concentrating on the type switch guard rather than errors
around (confusing) syntactic details.

Also clean up some code setting up AssertExpr (they never
have a nil Type field) and remove some incorrect TODOs.

Fixes #24470.

Change-Id: I69512f36e0417e3b5ea9c8856768e04b19d654a8
Reviewed-on: https://go-review.googlesource.com/103615
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-04-03 05:34:20 +00:00
isharipo
b44e73eacb test/fixedbugs: fix bug248 and bug345
When test/run script was removed, these two tests
were changed to be executed by test/run.go.
Because errchk does not exit with non-zero status on
errors, they were silently failing for a while.

This change makes 2 things:

1. Compile tested packages in GOROOT/test to match older runner script
   behavior (strictly required only in bug345, optional in bug248)

2. Check command output with "(?m)^BUG" regexp.
   It approximates older `grep -q '^BUG' that was used before.

See referenced issue for detailed explanation.

Fixes #24629

Change-Id: Ie888dcdb4e25cdbb19d434bbc5cb03eb633e9ee8
Reviewed-on: https://go-review.googlesource.com/104095
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-04-02 20:08:27 +00:00
Bryan Chan
625f2dccd4 cmd/compile/internal/ssa: handle symbol address comparisons consistently
CL 38338 introduced SSA rules to optimize two types of pointer equality
tests: a pointer compared with itself, and comparison of addresses taken
of two symbols which may have the same base. This patch adds rules to
apply the same optimization to pointer inequality tests, which also ensures
that two pointers to zero-width types cannot be both equal and unequal
at the same time.

Fixes #24503.

Change-Id: Ic828aeb86ae2e680caf66c35f4c247674768a9ba
Reviewed-on: https://go-review.googlesource.com/102275
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-03-31 21:37:13 +00:00
Matthew Dempsky
7b177b1a03 cmd/compile: fix method set computation for shadowed methods
In expandmeth, we call expand1/expand0 to build a list of all
candidate methods to promote, and then we use dotpath to prune down
which names actually resolve to a promoted method and how.

However, previously we still computed "followsptr" based on the
expand1/expand0 traversal (which is depth-first), rather than
dotpath (which is breadth-first). The result is that we could
sometimes end up miscomputing whether a particular promoted method
involves a pointer traversal, which could result in bad code
generation for method trampolines.

Fixes #24547.

Change-Id: I57dc014466d81c165b05d78b98610dc3765b7a90
Reviewed-on: https://go-review.googlesource.com/102618
Reviewed-by: Robert Griesemer <gri@golang.org>
2018-03-27 18:56:36 +00:00
Michael Munday
ae10914e67 cmd/compile: mark LAA and LAAG as clobbering flags on s390x
The atomic add instructions modify the condition code and so need to
be marked as clobbering flags.

Fixes #24449.

Change-Id: Ic69c8d775fbdbfb2a56c5e0cfca7a49c0d7f6897
Reviewed-on: https://go-review.googlesource.com/101455
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-03-20 09:44:50 +00:00
Vladimir Kuzmin
c12b185a6e cmd/compile: avoid mapaccess at m[k]=append(m[k]..
Currently rvalue m[k] is transformed during walk into:

        tmp1 := *mapaccess(m, k)
        tmp2 := append(tmp1, ...)
        *mapassign(m, k) = tmp2

However, this is suboptimal, as we could instead produce just:
        tmp := mapassign(m, k)
        *tmp := append(*tmp, ...)

Optimization is possible only if during Order it may tell that m[k] is
exactly the same at left and right part of assignment. It doesn't work:
1) m[f(k)] = append(m[f(k)], ...)
2) sink, m[k] = sink, append(m[k]...)
3) m[k] = append(..., m[k],...)

Benchmark:
name                           old time/op    new time/op    delta
MapAppendAssign/Int32/256-8      33.5ns ± 3%    22.4ns ±10%  -33.24%  (p=0.000 n=16+18)
MapAppendAssign/Int32/65536-8    68.2ns ± 6%    48.5ns ±29%  -28.90%  (p=0.000 n=20+20)
MapAppendAssign/Int64/256-8      34.3ns ± 4%    23.3ns ± 5%  -32.23%  (p=0.000 n=17+18)
MapAppendAssign/Int64/65536-8    65.9ns ± 7%    61.2ns ±19%   -7.06%  (p=0.002 n=18+20)
MapAppendAssign/Str/256-8         116ns ±12%      79ns ±16%  -31.70%  (p=0.000 n=20+19)
MapAppendAssign/Str/65536-8       134ns ±15%     111ns ±45%  -16.95%  (p=0.000 n=19+20)

name                           old alloc/op   new alloc/op   delta
MapAppendAssign/Int32/256-8       47.0B ± 0%     46.0B ± 0%   -2.13%  (p=0.000 n=19+18)
MapAppendAssign/Int32/65536-8     27.0B ± 0%     20.7B ±30%  -23.33%  (p=0.000 n=20+20)
MapAppendAssign/Int64/256-8       47.0B ± 0%     46.0B ± 0%   -2.13%  (p=0.000 n=20+17)
MapAppendAssign/Int64/65536-8     27.0B ± 0%     27.0B ± 0%     ~     (all equal)
MapAppendAssign/Str/256-8         94.0B ± 0%     78.0B ± 0%  -17.02%  (p=0.000 n=20+16)
MapAppendAssign/Str/65536-8       54.0B ± 0%     54.0B ± 0%     ~     (all equal)

Fixes #24364
Updates #5147

Change-Id: Id257d052b75b9a445b4885dc571bf06ce6f6b409
Reviewed-on: https://go-review.googlesource.com/100838
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-03-20 01:47:07 +00:00
Matthew Dempsky
e601c07908 cmd/compile: reject type switch with guarded declaration and no cases
Fixes #23116.

Change-Id: I5db5c5c39bbb50148ffa18c9393b045f255f80a3
Reviewed-on: https://go-review.googlesource.com/100459
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2018-03-13 22:02:46 +00:00
Robert Griesemer
363bcd7b4f cmd/compile: use key position for key:val elements in composite literals
Fixes #24339.

Change-Id: Ie47764fed27f76b480834b1fdbed0512c94831d9
Reviewed-on: https://go-review.googlesource.com/100457
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-03-13 21:44:12 +00:00
Vladimir Kuzmin
7395083136 cmd/compile: avoid extra mapaccess in "m[k] op= r"
Currently, order desugars map assignment operations like

    m[k] op= r

into

    m[k] = m[k] op r

which in turn is transformed during walk into:

    tmp := *mapaccess(m, k)
    tmp = tmp op r
    *mapassign(m, k) = tmp

However, this is suboptimal, as we could instead produce just:

    *mapassign(m, k) op= r

One complication though is if "r == 0", then "m[k] /= r" and "m[k] %=
r" will panic, and they need to do so *before* calling mapassign,
otherwise we may insert a new zero-value element into the map.

It would be spec compliant to just emit the "r != 0" check before
calling mapassign (see #23735), but currently these checks aren't
generated until SSA construction. For now, it's simpler to continue
desugaring /= and %= into two map indexing operations.

Fixes #23661.

Change-Id: I46e3739d9adef10e92b46fdd78b88d5aabe68952
Reviewed-on: https://go-review.googlesource.com/91557
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
2018-03-12 19:27:44 +00:00
Kunpei Sakai
b75e8a2a3b cmd/compile: prevent detection of wrong duplicates
by including *types.Type in typeVal.

Updates #21866
Fixes #24159

Change-Id: I2f8cac252d88d43e723124f2867b1410b7abab7b
Reviewed-on: https://go-review.googlesource.com/98476
Run-TryBot: Kunpei Sakai <namusyaka@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-03-07 01:26:00 +00:00
ChrisALiles
42ecf39e85 cmd/compile: improve compiler error on embedded structs
Fixes #23609

Change-Id: I751aae3d849de7fce1306324fcb1a4c3842d873e
Reviewed-on: https://go-review.googlesource.com/97076
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-03-06 21:06:46 +00:00
Josh Bleecher Snyder
aa9c1a8f80 runtime: fix amd64p32 indexbytes in presence of overflow
When the slice/string length is very large,
probably artifically large as in CL 97523,
adding BX (length) to R11 (pointer) overflows.
As a result, checking DI < R11 yields the wrong result.
Since they will be equal when the loop is done,
just check DI != R11 instead.
Yes, the pointer itself could overflow, but if that happens,
something else has gone pretty wrong; not our concern here.

Fixes #24187

Change-Id: I2f60fc6ccae739345d01bc80528560726ad4f8c6
Reviewed-on: https://go-review.googlesource.com/97802
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2018-03-01 16:53:33 +00:00
Matthew Dempsky
b3f00c6985 cmd/compile: fix unexpected type alias crash
OCOMPLIT stores the pre-typechecked type in n.Right, and then moves it
to n.Type. However, it wasn't clearing n.Right, so n.Right continued
to point to the OTYPE node. (Exception: slice literals reused n.Right
to store the array length.)

When exporting inline function bodies, we don't expect to need to save
any type aliases. Doing so wouldn't be wrong per se, but it's
completely unnecessary and would just bloat the export data.

However, reexportdep (whose role is to identify types needed by inline
function bodies) uses a generic tree traversal mechanism, which visits
n.Right even for O{ARRAY,MAP,STRUCT}LIT nodes. This means it finds the
OTYPE node, and mistakenly interpreted that the type alias needs to be
exported.

The straight forward fix is to just clear n.Right when typechecking
composite literals.

Fixes #24173.

Change-Id: Ia2d556bfdd806c83695b08e18b6cd71eff0772fc
Reviewed-on: https://go-review.googlesource.com/97719
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
2018-02-28 20:18:37 +00:00
Daniel Martí
1e308fbc1a cmd/compile: improved error message when calling a shadowed builtin
Otherwise, the error can be confusing if one forgets or doesn't know
that the builtin is being shadowed, which is not common practice.

Fixes #22822.

Change-Id: I735393b5ce28cb83815a1c3f7cd2e7bb5080a32d
Reviewed-on: https://go-review.googlesource.com/97455
Reviewed-by: Robert Griesemer <gri@golang.org>
2018-02-28 19:39:52 +00:00
Robert Griesemer
0c884d0810 cmd/compile, cmd/compile/internal/syntax: print relative column info
This change enables printing of relative column information if a
prior line directive specified a valid column. If there was no
line directive, or the line directive didn't specify a column
(or the -C flag is specified), no column information is shown in
file positions.

Implementation: Column values (and line values, for that matter)
that are zero are interpreted as "unknown". A line directive that
doesn't specify a column records that as a zero column in the
respective PosBase data structure. When computing relative columns,
a relative value is zero of the base's column value is zero.
When formatting a position, a zero column value is not printed.

To make this work without special cases, the PosBase for a file
is given a concrete (non-0:0) position 1:1 with the PosBase's
line and column also being 1:1. In other words, at the position
1:1 of a file, it's relative positions are starting with 1:1 as
one would expect.

In the package syntax, this requires self-recursive PosBases for
file bases, matching what cmd/internal/src.PosBase was already
doing. In src.PosBase, file and inlining bases also need to be
based at 1:1 to indicate "known" positions.

This change completes the cmd/compiler part of the issue below.

Fixes #22662.

Change-Id: I6c3d2dee26709581fba0d0261b1d12e93f1cba1a
Reviewed-on: https://go-review.googlesource.com/97375
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-02-28 03:51:23 +00:00
Matthew Dempsky
d7cd61ceaa cmd/compile: fix inlining of constant if statements
We accidentally overlooked needing to still visit Ninit for OIF
statements with constant conditions in golang.org/cl/96778.

Fixes #24120.

Change-Id: I5b341913065ff90e1163fb872b9e8d47e2a789d2
Reviewed-on: https://go-review.googlesource.com/97475
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-02-27 19:27:32 +00:00
Robert Griesemer
515fa58ac9 cmd/compile: track line directives w/ column information
Extend cmd/internal/src.PosBase to track column information,
and adjust the meaning of the PosBase position to mean the
position at which the PosBase's relative (line, col) position
starts (rather than indicating the position of the //line
directive). Because this semantic change is made in the
compiler's noder, it doesn't affect the logic of src.PosBase,
only its test setup (where PosBases are constructed with
corrected incomming positions). In short, src.PosBase now
matches syntax.PosBase with respect to the semantics of
src.PosBase.pos.

For #22662.

Change-Id: I5b1451cb88fff3f149920c2eec08b6167955ce27
Reviewed-on: https://go-review.googlesource.com/96535
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-02-26 18:32:03 +00:00
Ian Lance Taylor
9fba50545d test: add test case where gccgo incorrectly rejected aliases
Updates #23912

Change-Id: I50d06506a8ac91ed99a761a9ff3fd0b03d4c8121
Reviewed-on: https://go-review.googlesource.com/94995
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-02-20 01:57:14 +00:00
Keith Randall
4313d7767d cmd/compile: reset branch prediction when deleting a branch
When we go from a branch block to a plain block, reset the
branch prediction bit. Downstream passes asssume that if the
branch prediction is set, then the block has 2 successors.

Fixes #23504

Change-Id: I2898ec002228b2e34fe80ce420c6939201c0a5aa
Reviewed-on: https://go-review.googlesource.com/88955
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2018-02-20 00:56:13 +00:00
Austin Clements
2b415549b8 runtime: use sparse mappings for the heap
This replaces the contiguous heap arena mapping with a potentially
sparse mapping that can support heap mappings anywhere in the address
space.

This has several advantages over the current approach:

* There is no longer any limit on the size of the Go heap. (Currently
  it's limited to 512GB.) Hence, this fixes #10460.

* It eliminates many failures modes of heap initialization and
  growing. In particular it eliminates any possibility of panicking
  with an address space conflict. This can happen for many reasons and
  even causes a low but steady rate of TSAN test failures because of
  conflicts with the TSAN runtime. See #16936 and #11993.

* It eliminates the notion of "non-reserved" heap, which was added
  because creating huge address space reservations (particularly on
  64-bit) led to huge process VSIZE. This was at best confusing and at
  worst conflicted badly with ulimit -v. However, the non-reserved
  heap logic is complicated, can race with other mappings in non-pure
  Go binaries (e.g., #18976), and requires that the entire heap be
  either reserved or non-reserved. We currently maintain the latter
  property, but it's quite difficult to convince yourself of that, and
  hence difficult to keep correct. This logic is still present, but
  will be removed in the next CL.

* It fixes problems on 32-bit where skipping over parts of the address
  space leads to mapping huge (and never-to-be-used) metadata
  structures. See #19831.

This also completely rewrites and significantly simplifies
mheap.sysAlloc, which has been a source of many bugs. E.g., #21044,
 #20259, #18651, and #13143 (and maybe #23222).

This change also makes it possible to allocate individual objects
larger than 512GB. As a result, a few tests that expected huge
allocations to fail needed to be changed to make even larger
allocations. However, at the moment attempting to allocate a humongous
object may cause the program to freeze for several minutes on Linux as
we fall back to probing every page with addrspace_free. That logic
(and this failure mode) will be removed in the next CL.

Fixes #10460.
Fixes #22204 (since it rewrites the code involved).

This slightly slows down compilebench and the x/benchmarks garbage
benchmark.

name       old time/op     new time/op     delta
Template       184ms ± 1%      185ms ± 1%    ~     (p=0.065 n=10+9)
Unicode       86.9ms ± 3%     86.3ms ± 1%    ~     (p=0.631 n=10+10)
GoTypes        599ms ± 0%      602ms ± 0%  +0.56%  (p=0.000 n=10+9)
Compiler       2.87s ± 1%      2.89s ± 1%  +0.51%  (p=0.002 n=9+10)
SSA            7.29s ± 1%      7.25s ± 1%    ~     (p=0.182 n=10+9)
Flate          118ms ± 2%      118ms ± 1%    ~     (p=0.113 n=9+9)
GoParser       147ms ± 1%      148ms ± 1%  +1.07%  (p=0.003 n=9+10)
Reflect        401ms ± 1%      404ms ± 1%  +0.71%  (p=0.003 n=10+9)
Tar            175ms ± 1%      175ms ± 1%    ~     (p=0.604 n=9+10)
XML            209ms ± 1%      210ms ± 1%    ~     (p=0.052 n=10+10)

(https://perf.golang.org/search?q=upload:20171231.4)

name                       old time/op  new time/op  delta
Garbage/benchmem-MB=64-12  2.23ms ± 1%  2.25ms ± 1%  +0.84%  (p=0.000 n=19+19)

(https://perf.golang.org/search?q=upload:20171231.3)

Relative to the start of the sparse heap changes (starting at and
including "runtime: fix various contiguous bitmap assumptions"),
overall slowdown is roughly 1% on GC-intensive benchmarks:

name        old time/op     new time/op     delta
Template        183ms ± 1%      185ms ± 1%  +1.32%  (p=0.000 n=9+9)
Unicode        84.9ms ± 2%     86.3ms ± 1%  +1.65%  (p=0.000 n=9+10)
GoTypes         595ms ± 1%      602ms ± 0%  +1.19%  (p=0.000 n=9+9)
Compiler        2.86s ± 0%      2.89s ± 1%  +0.91%  (p=0.000 n=9+10)
SSA             7.19s ± 0%      7.25s ± 1%  +0.75%  (p=0.000 n=8+9)
Flate           117ms ± 1%      118ms ± 1%  +1.10%  (p=0.000 n=10+9)
GoParser        146ms ± 2%      148ms ± 1%  +1.48%  (p=0.002 n=10+10)
Reflect         398ms ± 1%      404ms ± 1%  +1.51%  (p=0.000 n=10+9)
Tar             173ms ± 1%      175ms ± 1%  +1.17%  (p=0.000 n=10+10)
XML             208ms ± 1%      210ms ± 1%  +0.62%  (p=0.011 n=10+10)
[Geo mean]      369ms           373ms       +1.17%

(https://perf.golang.org/search?q=upload:20180101.2)

name                       old time/op  new time/op  delta
Garbage/benchmem-MB=64-12  2.22ms ± 1%  2.25ms ± 1%  +1.51%  (p=0.000 n=20+19)

(https://perf.golang.org/search?q=upload:20180101.3)

Change-Id: I5daf4cfec24b252e5a57001f0a6c03f22479d0f0
Reviewed-on: https://go-review.googlesource.com/85887
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
2018-02-15 21:12:23 +00:00
Robert Griesemer
33eb0633e1 cmd/compile/internal/syntax: don't assume (operator) ~ means operator ^
The scanner assumed that ~ really meant ^, which may be helpful when
coming from C. But ~ is not a valid Go token, and pretending that it
should be ^ can lead to confusing error messages. Better to be upfront
about it and complain about the invalid character in the first place.

This was code "inherited" from the original yacc parser which was
derived from a C compiler. It's 10 years later and we can probably
assume that people are less confused about C and Go.

Fixes #23587.

Change-Id: I8d8f9b55b0dff009b75c1530d729bf9092c5aea6
Reviewed-on: https://go-review.googlesource.com/94160
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-02-15 16:41:24 +00:00
Ian Lance Taylor
0849dfd4a3 test: add new test that gccgo failed to compile
Updates #22305

Change-Id: I0e6bbd880599fc1b70d0378b746d162d4a846c65
Reviewed-on: https://go-review.googlesource.com/91556
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-02-15 03:37:20 +00:00
Robert Griesemer
1a22738749 cmd/compile/internal/syntax: more tolerant handling of missing function invocation in go/defer
Assume that an expression that is not a function call in a defer/go
statement is indeed a function that is just missing its invocation.
Report the error but continue with a sane syntax tree.

Fixes #23586.

Change-Id: Ib45ebac57c83b3e39ae4a1b137ffa291dec5b50d
Reviewed-on: https://go-review.googlesource.com/94156
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-02-15 01:48:16 +00:00
Matthew Dempsky
d3f6d11d84 cmd/compile: fix typechecking of untyped boolean expressions
Previously, if we typechecked a statement like

    var x bool = p1.f == p2.f && p1.g == p2.g

we would correctly update the '&&' node's type from 'untyped bool' to
'bool', but the '==' nodes would stay 'untyped bool'. This is
inconsistent, and caused consistency checks during walk to fail.

This CL doesn't pass toolstash because it seems to slightly affect the
register allocator's heuristics. (Presumably 'untyped bool's were
previously making it all the way through SSA?)

Fixes #23414.

Change-Id: Ia85f8cfc69b5ba35dfeb157f4edf57612ecc3285
Reviewed-on: https://go-review.googlesource.com/94022
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2018-02-15 01:00:06 +00:00
Kunpei Sakai
bcb563f4db cmd/compile: allow converting defined string types to []rune
Fixes #23298

Change-Id: I107c6f3a80db83f063c0daf262c6e7f7492e4d4c
Reviewed-on: https://go-review.googlesource.com/87695
Run-TryBot: Kunpei Sakai <namusyaka@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-02-15 00:25:21 +00:00
Ian Lance Taylor
d1f679a6af test: add test case for incorrect gccgo compilation error
Updates #23489

Change-Id: Ie846ccfe4c4d9295857f5da6863ac8f2ac0f2f6a
Reviewed-on: https://go-review.googlesource.com/89935
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-02-14 20:13:22 +00:00
Ian Lance Taylor
034aca1cbb test: add a test that gccgo miscompiled
Updates #20923

Change-Id: Ia1210ea3dec39e5db2521aeafca24d6e731f0c93
Reviewed-on: https://go-review.googlesource.com/91657
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-02-14 20:13:07 +00:00
Ian Lance Taylor
d9fc1929e9 test: add test for rounding to positive zero
Per the language spec clarification in https://golang.org/cl/14727.

Updates #12576
Updates #12621

Change-Id: I1e459c3c11a571bd29582761faacaa9ca3178ba6
Reviewed-on: https://go-review.googlesource.com/91895
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-02-14 20:13:05 +00:00
Keith Randall
755b36aa53 cmd/compile: fix constant folding of right shifts
The sub-word shifts need to sign-extend before shifting, to avoid
bringing in data from higher in the argument.

Fixes #23812

Change-Id: I0a95a0b49c48f3b40b85765bb4a9bb492be0cd73
Reviewed-on: https://go-review.googlesource.com/93716
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2018-02-14 00:03:36 +00:00
Emmanuel Odeke
25d22d9aba cmd/compile: report the struct type in invalid number of initializer values
Fixes #23732

Disambiguate "too few" or "too many" values in struct
initializer messages by reporting the name of the literal.

After:
issue23732.go:27:3: too few values in Foo literal
issue23732.go:34:12: too many values in Bar literal
issue23732.go:40:6: too few values in Foo literal
issue23732.go:40:12: too many values in Bar literal

Change-Id: Ieca37298441d907ac78ffe960c5ab55741a362ef
Reviewed-on: https://go-review.googlesource.com/93277
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-02-13 19:42:15 +00:00
Austin Clements
2010189407 runtime: remove legacy eager write barrier
Now that the buffered write barrier is implemented for all
architectures, we can remove the old eager write barrier
implementation. This CL removes the implementation from the runtime,
support in the compiler for calling it, and updates some compiler
tests that relied on the old eager barrier support. It also makes sure
that all of the useful comments from the old write barrier
implementation still have a place to live.

Fixes #22460.

Updates #21640 since this fixes the layering concerns of the write
barrier (but not the other things in that issue).

Change-Id: I580f93c152e89607e0a72fe43370237ba97bae74
Reviewed-on: https://go-review.googlesource.com/92705
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-02-13 16:34:46 +00:00
Keith Randall
23e8e197b0 cmd/compile: use unsigned loads for multi-element comparisons
When loading multiple elements of an array into a single register,
make sure we treat them as unsigned.  When treated as signed, the
upper bits might all be set, causing the shift-or combo to clobber
the values higher in the register.

Fixes #23719.

Change-Id: Ic87da03e9bd0fe2c60bb214b99f846e4e9446052
Reviewed-on: https://go-review.googlesource.com/92335
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
2018-02-06 18:24:33 +00:00
Cherry Zhang
43288467d2 test: add test for gccgo bug 23545
The fix is CL 91035.

Build only with gccgo at the moment, as it hits issue #23546.

Updates #23545.

Change-Id: I3a1367bb31b04773d31f71016f8fd7bd1855d7b5
Reviewed-on: https://go-review.googlesource.com/89735
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-01-31 20:03:31 +00:00
Keith Randall
7eaa8efbe4 cmd/compile: don't let spills clobber arguments
The compiler allows code to have multiple differently-typed views of a
single argument. For instance, if we have

func f(x float64) {
   y := *(*int64)(unsafe.Pointer(&x))
   ...
}

Then in SSA we get two OpArg ops, one with float64 type and one with
int64 type.

The compiler will try to reuse argument slots for spill slots. It
checks that the argument slot is dead by consulting an interference
graph.

When building the interference graph, we normally ignore cross-type
edges because the values on either end of that edge can't be allocated
to the same slot. (This is just a space-saving optimization.) This
rule breaks down when one of the values is an argument, because of the
multiple views described above. If we're spilling a float64, it is not
enough that the float64 version of x is dead; the int64 version of x
has to be dead also.

Remove the optimization of not recording interference edges if types
don't match. That optimization is incorrect if one of the values
connected by the edge is an argument.

Fixes #23522

Change-Id: I361f85d80fe3bc7249014ca2c3ec887c3dc30271
Reviewed-on: https://go-review.googlesource.com/89335
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-01-23 21:51:55 +00:00
Cherry Zhang
7f1c4b3afb cmd/compile: disable "redundant zeroextensions" optimization for Select on AMD64
A Select Op could produce a value with upper 32 bits NOT zeroed,
for example, Div32 is lowered to (Select0 (DIVL x y)).

In theory, we could look into the argument of a Select to decide
whether the upper bits are zeroed. As it is late in release cycle,
just disable this optimization for Select for now.

Fixes #23305.

Change-Id: Icf665a2af9ccb0a7ba0ae00c683c9e349638bf85
Reviewed-on: https://go-review.googlesource.com/85736
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
2018-01-02 21:08:35 +00:00
Than McIntosh
841d865a56 cmd/compile: second attempt at fix for issue 23179
My previous fix for issue 23179 was incomplete; it turns out that if
an unnamed parameter is below a specific size threshold, it gets
register-promoted away by the compiler (hence not encountered during
some parts of DWARF inline info processing), but if it is sufficiently
large, it is allocated to the stack as a named variable and treated as
a regular parameter by DWARF generation. Interestingly, something in
the ppc64le build of k8s causes an unnamed parameter to be retained
(where on amd64 it is deleted), meaning that this wasn't caught in my
amd64 testing.

The fix is to insure that "_" params are treated in the same way that
"~r%d" return temps are when matching up post-optimization inlined
routine params with pre-inlining declarations. I've also updated the
test case to include a "_" parameter with a very large size, which
also triggers the bug on amd64.

Fixes #23179.

Change-Id: I961c84cc7a873ad3f8f91db098a5e13896c4856e
Reviewed-on: https://go-review.googlesource.com/84975
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2017-12-20 20:39:16 +00:00
Than McIntosh
0447216316 cmd/compile: fix corner case in DWARF inline info generation
The helper routine for returning pre-inlining parameter declarations
wasn't properly handling the case where you have more than one
parameter named "_" in a function signature; this triggered a map
collision later on when the function was inlined and DWARF was
generated for the inlined routine instance.

Fixes #23179.

Change-Id: I12e5d6556ec5ce08e982a6b53666a4dcc1d22201
Reviewed-on: https://go-review.googlesource.com/84755
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-12-19 19:00:42 +00:00