1
0
mirror of https://github.com/golang/go synced 2024-11-18 19:34:41 -07:00
Commit Graph

1348 Commits

Author SHA1 Message Date
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
Matthew Dempsky
13bf4ada80 cmd/compile: remove broken inlining accounting code
We can't currently inline functions that contain closures anyway, so
just delete this budgeting code for now. Re-enable once we can (if
ever) inline functions with nested closures.

Updates #15561.
Fixes #23093.

Change-Id: Idc5f8e042ccfcc8921022e58d3843719d4ab821e
Reviewed-on: https://go-review.googlesource.com/83538
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-12-12 20:26:33 +00:00
Keith Randall
4c800f03c9 cmd/compile: fix large load/store offsets on 386
Pointer arithemetic is done mod 2^32 on 386, so we can just
drop the high bits of any large constant offsets.

The bounds check will make sure wraparounds are never observed.

Fixes #21655

Change-Id: I68ae5bbea9f02c73968ea2b21ca017e5ecb89223
Reviewed-on: https://go-review.googlesource.com/82675
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2017-12-08 03:53:18 +00:00
Keith Randall
dd7cbf3a84 cmd/compile: fix map assignment with panicking right-hand side
Make sure that when we're assigning to a map, we evaluate the
right-hand side before we attempt to insert into the map.

We used to evaluate the left-hand side to a pointer-to-slot-in-bucket
(which as a side effect does len(m)++), then evaluate the right-hand side,
then do the assignment. That clearly isn't correct when the right-hand side
might panic.

Fixes #22881

Change-Id: I42a62870ff4bf480568c9bdbf0bb18958962bdf0
Reviewed-on: https://go-review.googlesource.com/81817
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-12-05 00:10:10 +00:00
Than McIntosh
88c2fb9d04 cmd/compile: fix bug in DWARF inl handling of unused autos
The DWARF inline info generation hooks weren't properly
handling unused auto vars in certain cases, triggering an assert (now
fixed). Also with this change, introduce a new autom "flavor" to
use for autom entries that are added to insure that a specific
auto type makes it into the linker (this is a follow-on to the fix
for 22941).

Fixes #22962.

Change-Id: I7a2d8caf47f6ca897b12acb6a6de0eb25f5cac8f
Reviewed-on: https://go-review.googlesource.com/81557
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>
2017-12-04 18:36:11 +00:00
Than McIntosh
9372166faa cmd/compile: fix DWARF type symbol buglet
The code that generates the list of DWARF variables for a function
(params and autos) will emit a "no-location" entry in the DWARF for a
user var that appears in the original pre-optimization version of the
function but is no longer around when optimization is complete. The
intent is that if a GDB user types "print foo" (where foo has been
optimized out), the response will be "<optimized out>" as opposed to
"there is no such variable 'foo'). This change fixes said code to
include vars on the autom list for the function, to insure that the
type symbol for the variable makes it to the linker.

Fixes #22941.

Change-Id: Id29f1f39d68fbb798602dfd6728603040624fc41
Reviewed-on: https://go-review.googlesource.com/81415
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>
2017-12-01 18:52:50 +00:00
Cherry Zhang
dbb1d198ab cmd/compile: fix loop depth of range expression in escape analysis
ORANGE node's Right node is the expression it is ranging over,
which is evaluated before the loop. In the escape analysis,
we should walk this node without loop depth incremented.

Fixes #21709.

Change-Id: Idc1e4c76e39afb5a344d85f6b497930a488ce5cf
Reviewed-on: https://go-review.googlesource.com/80740
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2017-11-30 13:45:18 +00:00
Matthew Dempsky
2f588ff08f cmd/compile: make -asmhdr work with type aliases
For "type T = U" we were accidentally emitting a #define for "U__size"
instead of "T__size".

Fixes #22877.

Change-Id: I5ed6757d697753ed6d944077c16150759f6e1285
Reviewed-on: https://go-review.googlesource.com/80759
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-11-29 20:40:41 +00:00
Matthew Dempsky
71a9c4430f cmd/compile: fix infinite recursion in isdirectiface
Fixes #22904.

Change-Id: Id504504eda7275c10d3c665add8b7ccd23f65820
Reviewed-on: https://go-review.googlesource.com/80301
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-11-28 21:58:16 +00:00
Matthew Dempsky
3c375f1b7e cmd/compile, go/types: error if main.main is not a function
Fixes #21256.

Change-Id: I3af4c76e734c09d07f15525b793a544a7279b906
Reviewed-on: https://go-review.googlesource.com/79435
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-11-22 19:14:31 +00:00
Keith Randall
48e207d518 cmd/compile: fix mapassign_fast* routines for pointer keys
The signature of the mapassign_fast* routines need to distinguish
the pointerness of their key argument.  If the affected routines
suspend part way through, the object pointed to by the key might
get garbage collected because the key is typed as a uint{32,64}.

This is not a problem for mapaccess or mapdelete because the key
in those situations do not live beyond the call involved.  If the
object referenced by the key is garbage collected prematurely, the
code still works fine.  Even if that object is subsequently reallocated,
it can't be written to the map in time to affect the lookup/delete.

Fixes #22781

Change-Id: I0bbbc5e9883d5ce702faf4e655348be1191ee439
Reviewed-on: https://go-review.googlesource.com/79018
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
2017-11-22 04:30:27 +00:00
Emmanuel Odeke
5f29a7a705 cmd/compile: hint on wrong case-field names in composite literals
Improve the error message for wrong
case-field names in composite literals,
by mentioning the correct field name.

Given the program:
package main

type it struct {
        ID string
}

func main() {
        i1 := &it{id: "Bar"}
}

just like we do for usage of fields, we now
report wrongly cased fields as hints to give:

ts.go:8:14: unknown field 'id' in struct literal of type it (but does have ID)

instead of before:

ts.go:8:14: unknown field 'id' in struct literal of type it

Fixes #22794

Change-Id: I18cd70e75817025cb1df083503cae306e8d659fd
Reviewed-on: https://go-review.googlesource.com/78545
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-11-20 20:29:43 +00:00
Keith Randall
3abf117704 cmd/compile: add test for array decomposition
This test fails on 1.9.2, but is ok on tip.
CL 77331 has both the 1.9.2 fix and this test, and is on the 1.9 release branch.
This CL is just the test, and is on HEAD.  The buggy code doesn't exist on tip.

Update #22683

Change-Id: I04a24bd6c2d3068e18ca81da3347e2c1366f4447
Reviewed-on: https://go-review.googlesource.com/77332
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-11-13 20:22:22 +00:00
griesemer
ca2a886cba cmd/compile: record original and absolute file names for line directives
Also, with this change, error locations don't print absolute positions
in [] brackets following positions relative to line directives. To get
the absolute positions as well, specify the -L flag.

Fixes #22660.

Change-Id: I9ecfa254f053defba9c802222874155fa12fee2c
Reviewed-on: https://go-review.googlesource.com/77090
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2017-11-13 16:47:41 +00:00
David Chase
a042221cdb cmd/compile: adjust Pos setting for "empty" blocks
Plain blocks that contain only uninteresting instructions
(that do not have reliable Pos information themselves)
need to have their Pos left unset so that they can
inherit it from their successors.  The "uninteresting"
test was not properly applied and not properly defined.
OpFwdRef does not appear in the ssa.html debugging output,
but at the time of the test these instructions did appear,
and it needs to be part of the test.

Fixes #22365.

Change-Id: I99e5b271acd8f6bcfe0f72395f905c7744ea9a02
Reviewed-on: https://go-review.googlesource.com/74252
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2017-11-08 22:39:49 +00:00
Ian Lance Taylor
a4c009f5ae cmd/compile: don't put Noalg types in typelinks
They could get picked up by reflect code, yielding the wrong type.

Fixes #22605

Change-Id: Ie11fb361ca7f3255e662037b3407565c8f0a2c4c
Reviewed-on: https://go-review.googlesource.com/76315
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-11-07 22:29:48 +00:00
griesemer
17ff23f7c8 cmd/compile/internal/syntax: better syntax errors for typos in if/switch/for headers
Be more pessimistic when parsing if/switch/for headers for better error
messages when things go wrong.

Fixes #22581.

Change-Id: Ibb99925291ff53f35021bc0a59a4c9a7f695a194
Reviewed-on: https://go-review.googlesource.com/76290
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-11-06 23:17:24 +00:00
Emmanuel Odeke
0c55495748 cmd/compile: lock in test for column numbers in unused error
Updates #21317

@mdempsky fixed issue #21317 with CL 66810,
so lock a test in to ensure we don't regress.

The test is manual for now before test/run.go
has support for matching column numbers so do
it old school and match expected output after
an exec.

Change-Id: I6c2a66ddf04248f79d17ed7033a3280d50e41562
Reviewed-on: https://go-review.googlesource.com/76150
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-11-06 21:32:06 +00:00