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

2887 Commits

Author SHA1 Message Date
Cherry Zhang
2baed3856d cmd/asm: fix assembling return jump
In RET instruction, the operand is the return jump's target,
which should be put in Prog.To.

Add an action "buildrundir" to the test driver, which builds
(compile+assemble+link) the code in a directory and runs the
resulting binary.

Fixes #23838.

Change-Id: I7ebe7eda49024b40a69a24857322c5ca9c67babb
Reviewed-on: https://go-review.googlesource.com/94175
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
2018-03-01 21:11:16 +00:00
Giovanni Bajo
f16cc298d3 test: implement negative rules in asmcheck
Change-Id: I2b507e35cc314100eaf2ec2d1e5107cc2fc9e7cf
Reviewed-on: https://go-review.googlesource.com/97818
Reviewed-by: Keith Randall <khr@golang.org>
2018-03-01 18:15:24 +00:00
Giovanni Bajo
0bcf8bcd99 test: in asmcheck, regexp must match from beginning of line
This avoid simple bugs like "ADD" matching "FADD". Obviously
"ADD" will still match "ADDQ" so some care is still required
in this regard, but at least a first class of possible errors
is taken care of.

Change-Id: I7deb04c31de30bedac9c026d9889ace4a1d2adcb
Reviewed-on: https://go-review.googlesource.com/97817
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Reviewed-by: Keith Randall <khr@golang.org>
2018-03-01 18:14:54 +00:00
Giovanni Bajo
879a1ff1e4 test: improve asmcheck syntax
asmcheck comments now support a compact form of specifying
multiple checks for each platform, using the following syntax:

   amd64:"SHL\t[$]4","SHR\t[$]4"

Negative checks are also parsed using the following syntax:

   amd64:-"ROR"

though they are still not working.

Moreover, out-of-line comments have been implemented. This
allows to specify asmchecks on comment-only lines, that will
be matched on the first subsequent non-comment non-empty line.

    // amd64:"XOR"
    // arm:"EOR"

    x ^= 1

Change-Id: I110c7462fc6a5c70fd4af0d42f516016ae7f2760
Reviewed-on: https://go-review.googlesource.com/97816
Reviewed-by: Keith Randall <khr@golang.org>
2018-03-01 18:10:48 +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
Giovanni Bajo
c9438cb198 test: add support for code generation tests (asmcheck)
The top-level test harness is modified to support a new kind
of test: "asmcheck". This is meant to replace asm_test.go
as an easier and more readable way to test code generation.

I've added a couple of codegen tests to get initial feedback
on the syntax. I've created them under a common "codegen"
subdirectory, so that it's easier to run them all with
"go run run.go -v codegen".

The asmcheck syntax allows to insert line comments that
can specify a regular expression to match in the assembly code,
for multiple architectures (the testsuite will automatically
build each testfile multiple times, one per mentioned architecture).

Negative matches are unsupported for now, so this cannot fully
replace asm_test yet.

Change-Id: Ifdbba389f01d55e63e73c99e5f5449e642101d55
Reviewed-on: https://go-review.googlesource.com/97355
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Alberto Donizetti <alb.donizetti@gmail.com>
2018-03-01 07:59:54 +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
Keith Randall
2413b54888 cmd/compile: mark the first word of an interface as a uintptr
The first word of an interface is a pointer, but for the purposes
of GC we don't need to treat it as such.
 1. If it is a non-empty interface, the pointer points to an itab
    which is always in persistentalloc space.
 2. If it is an empty interface, the pointer points to a _type.
   a. If it is a compile-time-allocated type, it points into
      the read-only data section.
   b. If it is a reflect-allocated type, it points into the Go heap.
      Reflect is responsible for keeping a reference to
      the underlying type so it won't be GCd.

If we ever have a moving GC, we need to change this for 2b (as
well as scan itabs to update their itab._type fields).

Write barriers on the first word of interfaces have already been removed.

Change-Id: I643e91d7ac4de980ac2717436eff94097c65d959
Reviewed-on: https://go-review.googlesource.com/97518
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2018-02-27 22:58:32 +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
Cherry Zhang
beeab6ac02 cmd/compile: fix go:uintptrescapes tag for unnamed parameters
The tag was overwritten by the code for special handling unnamed
parameters.

Fixes #23045.

Change-Id: Ie2e1db3e902a07a2bbbc2a3424cea300f0a42cc3
Reviewed-on: https://go-review.googlesource.com/82775
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-02-14 21:41:39 +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
Kunpei Sakai
e858a6b9f0 all: use Fatalf instead of Fatal if format is given
Change-Id: I30e9b938bb19ed4e674c3ea4a1cd389b9c4f0b88
Reviewed-on: https://go-review.googlesource.com/86875
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-01-10 01:35:45 +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
Russ Cox
c6ea277ac1 test: skip "# package/path" output from cmd/go in errchk
This allows errchk to be used with "go vet" output (as opposed to "go tool vet").

Change-Id: I0009a53c9cb74accd5bd3923c137d6dbf9e46326
Reviewed-on: https://go-review.googlesource.com/83836
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-12-13 21:32:01 +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
Matthew Dempsky
840fad13ec cmd/compile: fix unsafe.Pointer liveness for Syscall-like functions
The package unsafe docs say it's safe to convert an unsafe.Pointer to
uintptr in the argument list to an assembly function, but it was
erroneously only detecting normal pointers converted to unsafe.Pointer
and then to intptr.

Fixes #23051.

Change-Id: Id1be19f6d8f26f2d17ba815191717d2f4f899732
Reviewed-on: https://go-review.googlesource.com/82817
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-12-08 21:34:24 +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
Matthew Dempsky
2b9e7c1864 test: disable broken test for 1.10
This test was added recently as a regress test for the spec relaxation
in #9060, but doesn't work correctly yet. Disable for now to fix noopt
builders.

Updates #22444.

Change-Id: I45c521ae0da7ffb0c6859d6f7220c59828ac6149
Reviewed-on: https://go-review.googlesource.com/81775
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-12-04 19:29:14 +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
Robert Griesemer
088a9ad543 cmd/compile: permit indices of certain non-constant shifts
Per the decision for #14844, index expressions that are non-constant
shifts where the LHS operand is representable as an int are now valid.

Fixes #21693.

Change-Id: Ifafad2c0c65975e0200ce7e28d1db210e0eacd9d
Reviewed-on: https://go-review.googlesource.com/81277
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-12-01 20:39:50 +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
Austin Clements
e862f98d1e test: make inline_callers.go test not inline the runtime
CL 76551 modified inline_callers.go to build everything, including the
runtime, with -l=4. While that works in most places (and ideally
should work everywhere), it blows out the nosplit stack on
solaris/amd64.

Fix this by only building the test itself with -l=4.

This undoes some of the changes to this test from CL 73212, which
originally changed the go tool to rebuild all packages with the given
flags. This change modified the expected output of this test, so now
that we can go back to building only the test itself with inlining, we
revert these changes to the expected output. (That CL also changed
log.Fatalf to log.Printf, but didn't add "\n" to the end of the lines,
so this CL fixes that, too.)

Fixes #22797.

Change-Id: I6a91963a59ebe98edbe0921d8717af6b2c2191b0
Reviewed-on: https://go-review.googlesource.com/79197
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-11-22 00:52:54 +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