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

3062 Commits

Author SHA1 Message Date
Ben Shi
de0e72610b test/codegen: add more combined store tests for arm64
Some combined store optimization was already implemented
in go-1.11, but there is no corresponding test cases.

Change-Id: Iebdad186e92047942e53a74f2c20b390922e1e9c
Reviewed-on: https://go-review.googlesource.com/122915
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-08-02 04:23:45 +00:00
Keith Randall
5fc70b6fac cmd/compile: set stricter inlining threshold in large functions
If we're compiling a large function, be more picky about how big
the function we're inlining is.  If the function is >5000 nodes,
we lower the inlining threshold from a cost of 80 to 20.

Turns out reflect.Value's cost is exactly 80.  That's the function
at issue in #26546.

20 was chosen as a proxy for "inlined body is smaller than the call would be".
Simple functions still get inlined, like this one at cost 7:

func ifaceIndir(t *rtype) bool {
	return t.kind&kindDirectIface == 0
}

5000 nodes was chosen as the big function size.  Here are all the
5000+ node (~~1000+ lines) functions in the stdlib:

5187 cmd/internal/obj/arm (*ctxt5).asmout
6879 cmd/internal/obj/s390x (*ctxtz).asmout
6567 cmd/internal/obj/ppc64 (*ctxt9).asmout
9643 cmd/internal/obj/arm64 (*ctxt7).asmout
5042 cmd/internal/obj/x86 (*AsmBuf).doasm
8768 cmd/compile/internal/ssa rewriteBlockAMD64
8878 cmd/compile/internal/ssa rewriteBlockARM
8344 cmd/compile/internal/ssa rewriteValueARM64_OpARM64OR_20
7916 cmd/compile/internal/ssa rewriteValueARM64_OpARM64OR_30
5427 cmd/compile/internal/ssa rewriteBlockARM64
5126 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_50
6152 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_60
6412 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_70
6486 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_80
6534 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_90
6534 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_100
6534 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_110
6675 cmd/compile/internal/gc typecheck1
5433 cmd/compile/internal/gc walkexpr
14070 cmd/vendor/golang.org/x/arch/arm64/arm64asm decodeArg

There are a lot more smaller (~1000 node) functions in the stdlib.
The function in #26546 has 12477 nodes.

At some point it might be nice to have a better heuristic for "inlined
body is smaller than the call", a non-cliff way to scale down the cost
as the function gets bigger, doing cheaper inlined calls first, etc.
All that can wait for another release. I'd like to do this CL for
1.11.

Fixes #26546
Update #17566

Change-Id: Idda13020e46ec2b28d79a17217f44b189f8139ac
Reviewed-on: https://go-review.googlesource.com/125516
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2018-07-24 16:11:08 +00:00
Cherry Zhang
48c79734ff test: add test for gccgo bug #26495
Gccgo produced incorrect order of evaluation for expressions
involving &&, || subexpressions. The fix is CL 125299.

Updates #26495.

Change-Id: I18d873281709f3160b3e09f0b2e46f5c120e1cab
Reviewed-on: https://go-review.googlesource.com/125301
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-07-20 20:08:15 +00:00
Keith Randall
59d0baeb33 cmd/compile: add test for OPmodify ops clobbering flags
Code fix was in CL 122556.  This is a corresponding test case.

Fixes #26426

Change-Id: Ib8769f367aed8bead029da0a8d2ddccee1d1dccb
Reviewed-on: https://go-review.googlesource.com/124535
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-07-19 16:24:53 +00:00
Daniel Martí
ba6974fdc3 cmd/compile: fix crash on invalid struct literal
If one tries to use promoted fields in a struct literal, the compiler
errors correctly. However, if the embedded fields are of struct pointer
type, the field.Type.Sym.Name expression below panics.

This is because field.Type.Sym is nil in that case. We can simply use
field.Sym.Name in this piece of code though, as it only concerns
embedded fields, in which case what we are after is the field name.

Added a test mirroring fixedbugs/issue23609.go, but with pointer types.

Fixes #26416.

Change-Id: Ia46ce62995c9e1653f315accb99d592aff2f285e
Reviewed-on: https://go-review.googlesource.com/124395
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-07-18 20:32:04 +00:00
Ben Shi
f6ce1e2aa5 cmd/compile: fix an arm64's comparison bug
The arm64 backend generates "TST" for "if uint32(a)&uint32(b) == 0",
which should be "TSTW".

fixes #26438

Change-Id: I7d64c30e3a840b43486bcd10eea2e3e75aaa4857
Reviewed-on: https://go-review.googlesource.com/124637
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2018-07-18 14:15:05 +00:00
Michael Munday
1546ab5a39 cmd/compile: keep autos if their address reaches a control value
Autos must be kept if their address reaches the control value of a
block. We didn't see this before because it is rare for an auto's
address to reach a control value without also reaching a phi or
being written to memory. We can probably optimize away the
comparisons that lead to this scenario since autos cannot alias
with pointers from elsewhere, however for now we take the
conservative approach and just ensure the auto is properly
initialised if its address reaches a control value.

Fixes #26407.

Change-Id: I02265793f010a9e001c3e1a5397c290c6769d4de
Reviewed-on: https://go-review.googlesource.com/124335
Reviewed-by: David Chase <drchase@google.com>
2018-07-17 14:58:54 +00:00
Keith Randall
85cfa4d55e cmd/compile: handle degenerate write barrier case
If both branches of a write barrier test go to the same block,
then there's no unsafe points.

This can only happen if the resulting memory state is somehow dead,
which can only occur in degenerate cases, like infinite loops. No
point in cleaning up the useless branch in these situations.

Fixes #26024.

Change-Id: I93a7df9fdf2fc94c6c4b1fe61180dc4fd4a0871f
Reviewed-on: https://go-review.googlesource.com/123655
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-07-12 19:06:09 +00:00
David Chase
0029cd479e cmd/compile: add LocalAddr that takes SP,mem operands
Lack of a well-defined order between VarDef and related
address operations sometimes causes problems with store order
and write barrier transformations; glitches in the order are
made irreparable (by later optimizations) if the two parts of
the glitch straddle a split in the original block caused by
insertion of a write barrier diamond.

Fix this by creating a LocalAddr for addresses of locals
(what VarDef matters for) that takes a memory input to
help make the order explicit.  Addr is modified to only
be legal for SB operand, so there is no overlap between
Addr and LocalAddr uses (there may be some downstream
cleanup from this).

Changes to generic.rules and rewrite.go ensure that codegen
tests continue to pass; CSE of LocalAddr is impaired, not
quite sure of the cost.

Fixes #26105.

Change-Id: Id4192b4440aa4e9d7ba54a465c456df9b530b515
Reviewed-on: https://go-review.googlesource.com/122483
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
2018-07-12 18:45:31 +00:00
Ian Lance Taylor
964c15f360 test: add test of valid code that gccgo failed to compile
Updates #26340

Change-Id: I3bc7cd544ea77df660bbda7de99a009b63d5be1b
Reviewed-on: https://go-review.googlesource.com/123477
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-07-12 05:29:12 +00:00
Matthew Dempsky
cc422e64d0 cmd/compile: fix ICE due to missing inline function body
For golang.org/cl/74110, I forgot that you can use range-based for
loops to extract key values from a map value.

This wasn't a problem for the binary format importer, because it was
more tolerant about missing inline function bodies. However, the
indexed importer is more particular about this.

We could potentially just make it more lenient like the binary
importer, but tweaking the logic here is easy enough and seems like
the preferable solution.

Fixes #26341.

Change-Id: I54564dcd0be60ea393f8a0f6954b7d3d61e96ee5
Reviewed-on: https://go-review.googlesource.com/123475
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2018-07-12 00:13:37 +00:00
Ian Lance Taylor
cda1947fd1 runtime: don't say "different packages" if they may not be different
Fix the panic message produced for an interface conversion error to
only say "types from different packages" if they are definitely from
different packges. If they may be from the same package, say "types
from different scopes."

Updates #18911
Fixes #26094

Change-Id: I0cea50ba31007d88e70c067b4680009ede69bab9
Reviewed-on: https://go-review.googlesource.com/123395
Reviewed-by: Austin Clements <austin@google.com>
2018-07-11 21:34:05 +00:00
Ian Lance Taylor
787ff17dea test: add test case that failed with gccgo
Updates #26335

Change-Id: Ibfb1e232a0c66fa699842c8908ae5ff0f5d2177d
Reviewed-on: https://go-review.googlesource.com/123316
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-07-11 18:14:35 +00:00
Ian Lance Taylor
d49572eaf3 test: add order of evaluation test case that gccgo got wrong
Updates #23188

Change-Id: Idc5567546d1c4c592f997a4cebbbf483b85331e0
Reviewed-on: https://go-review.googlesource.com/123115
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-07-10 21:30:26 +00:00
Keith Randall
be9c994609 internal/bytealg: specify argmaps for exported functions
Functions exported on behalf of other packages need to have their
argument stack maps specified explicitly.  They don't get an implicit
map because they are not in the local package, and if they get defer'd
they need argument maps.

Fixes #24419

Change-Id: I35b7d8b4a03d4770ba88699e1007cb3fcb5397a9
Reviewed-on: https://go-review.googlesource.com/122676
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2018-07-10 17:13:53 +00:00
Ben Shi
f9800a9473 test/codegen: add more test cases for arm64
More test cases of combined load for arm64.

Change-Id: I7a9f4dcec6930f161cbded1f47dbf7fcef1db4f1
Reviewed-on: https://go-review.googlesource.com/122582
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2018-07-10 15:41:15 +00:00
Cherry Zhang
c801232525 cmd/compile: make sure alg functions are generated when we call them
When DWARF is disabled, some alg functions were not generated.
Make sure they are generated when we about to generate calls to
them.

Fixes #23546.

Change-Id: Iecfa0eea830e42ee92e55268167cefb1540980b2
Reviewed-on: https://go-review.googlesource.com/122403
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2018-07-10 01:20:45 +00:00
Cherry Zhang
5f256dc8e6 test: add test for gccgo bug #26248
The fix is CL 122756.

Updates #26248.

Change-Id: Ic4250ab5d01da9f65d0bc033e2306343d9c87a99
Reviewed-on: https://go-review.googlesource.com/122757
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-07-10 01:19:37 +00:00
Keith Randall
58d287e5e8 cmd/compile: ensure that loop condition is detected correctly
We need to make sure that the terminating comparison has the right
sense given the increment direction. If the increment is positive,
the terminating comparsion must be < or <=. If the increment is
negative, the terminating comparison must be > or >=.

Do a few cleanups,  like constant-folding entry==0, adding comments,
removing unused "exported" fields.

Fixes #26116

Change-Id: I14230ee8126054b750e2a1f2b18eb8f09873dbd5
Reviewed-on: https://go-review.googlesource.com/121940
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2018-07-09 18:23:39 +00:00
Matthew Dempsky
8d5fd871d7 cmd/compile: fix "width not calculated" ICE
Expanding interface method sets is handled during width calculation,
which can't be performed concurrently. Make sure that we eagerly
expand interfaces in the frontend when importing them, even if they're
not actually used by code, because we might need to generate a type
description of them.

Fixes #25055.

Change-Id: I6fd2756de2c7d5dbc33056f70b3028ca3aebab41
Reviewed-on: https://go-review.googlesource.com/122517
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-07-06 20:25:52 +00:00
Cherry Zhang
3f54e8537a cmd/compile: run generic deadcode in -N mode
Late opt pass may generate dead stores, which messes up store
chain calculation in later passes. Run generic deadcode even
in -N mode to remove them.

Fixes #26163.

Change-Id: I8276101717bb978d5980e6c7998f53fd8d0ae10f
Reviewed-on: https://go-review.googlesource.com/121856
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
2018-07-02 20:53:23 +00:00
Michael Munday
adfa8b8691 cmd/compile: keep autos whose address reaches a phi
If the address of an auto reaches a phi then any further stores to
the pointer represented by the phi probably need to be kept. This
is because stores to the other arguments to the phi may be visible
to the program.

Fixes #26153.

Change-Id: Ic506c6c543bf70d792e5b1a64bdde1e5fdf1126a
Reviewed-on: https://go-review.googlesource.com/121796
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2018-07-02 19:43:07 +00:00
David Chase
d4d8237a5b Revert "cmd/compile: make OpAddr depend on VarDef in storeOrder"
This reverts commit 1a27f048ad.

Reason for revert: Broke the ssacheck and -N-l builders, and the -N-l fix looks like it will take some time and take a different route entirely.

Change-Id: Ie0ac5e86ab7d72a303dfbbc48dfdf1e092d4f61a
Reviewed-on: https://go-review.googlesource.com/121715
Reviewed-by: David Chase <drchase@google.com>
2018-06-29 19:48:48 +00:00
David Chase
1a27f048ad cmd/compile: make OpAddr depend on VarDef in storeOrder
Given a carefully constructed input, writebarrier would
split a block with the OpAddr in the first half and the
VarDef in the second half which ultimately leads to a
compiler crash because the scheduler is no longer able
to put them in the proper order.

To fix, recognize the implicit dependence of OpAddr on
the VarDef of the same symbol if any exists.

This fix was chosen over making OpAddr take a memory
operand to make the dependence explicit, because this
change is less invasive at this late part of the 1.11
release cycle.

Fixes #26105.

Change-Id: I9b65460673af3af41740ef877d2fca91acd336bc
Reviewed-on: https://go-review.googlesource.com/121436
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2018-06-29 15:20:52 +00:00
Cherry Zhang
d21bdf125c cmd/compile: check SSAability in handling of INDEX of 1-element array
SSA can handle 1-element array, but only when the element type
is SSAable. When building SSA for INDEX of 1-element array, we
did not check the element type is SSAable. And when it's not,
it resulted in an unhandled SSA op.

Fixes #26120.

Change-Id: Id709996b5d9d90212f6c56d3f27eed320a4d8360
Reviewed-on: https://go-review.googlesource.com/121496
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2018-06-29 12:05:05 +00:00
Robert Griesemer
b410ce750e cmd/compile: don't crash in untyped expr to interface conversion
Fixes #24763.

Change-Id: Ibe534271d75b6961d00ebfd7d42c43a3ac650d79
Reviewed-on: https://go-review.googlesource.com/121335
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-06-28 23:01:22 +00:00
Ilya Tocar
b71ea0b7dd cmd/compile: mark CMOVLEQF, CMOVWEQF as cloberring AX
Code generation for OpAMD64CMOV[WLQ]EQF uses AX as a scratch register,
but only CMOVQEQF, correctly lets compiler know. Mark other 2 as
clobbering AX.

Fixes #26097

Change-Id: I2a65bd67bf18a540898b4a0ae6c8766e0b767b19
Reviewed-on: https://go-review.googlesource.com/121336
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
2018-06-28 18:39:56 +00:00
Cherry Zhang
1d303a0086 cmd/compile: fold offset into address on Wasm
On Wasm, the offset was not folded into LoweredAddr, so it was
not rematerializeable. This led to the address-taken operation
in some cases generated too early, before the local variable
becoming live. The liveness code thinks the variable live when
the address is taken, then backs it up to live at function
entry, then complains about it, because nothing other than
arguments should be live on entry.

This CL folds the offset into the address operation, so it is
rematerializeable and so generated right before use, after the
variable actually becomes live.

It might be possible to relax the liveness code not to think a
variable live when its address being taken, but until the address
actually being used. But it would be quite complicated. As we're
late in Go 1.11 freeze, it would be better not to do it. Also,
I think the address operation is rematerializeable now on all
architectures, so this is probably less necessary.

This may also be a slight optimization, as the address+offset is
now rematerializeable, which can be generated on the Wasm stack,
without using any "registers" which are emulated by local
variables on Wasm. I don't know how to do benchmarks on Wasm. At
least, cmd/go binary size shrinks 9K.

Fixes #25966.

Change-Id: I01e5869515d6a3942fccdcb857f924a866876e57
Reviewed-on: https://go-review.googlesource.com/120599
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Richard Musiol <neelance@gmail.com>
2018-06-27 14:30:00 +00:00
David Chase
7e9d55eeeb cmd/compile: avoid remainder in loopbce when increment=0
For non-unit increment, loopbce checks to see if the
increment evenly divides the difference between (constant)
loop start and end.  This test panics when the increment
is zero.

Fix: check for zero, if found, don't optimize the loop.

Also added missing copyright notice to loopbce.go.

Fixes #26043.

Change-Id: I5f460104879cacc94481949234c9ce8c519d6380
Reviewed-on: https://go-review.googlesource.com/120759
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-06-25 20:36:42 +00:00
Matthew Dempsky
f422bea498 cmd/compile: fix compile failure for lazily resolved shadowed types
If expanding an inline function body required lazily expanding a
package-scoped type whose identifier was shadowed within the function
body, the lazy expansion would instead overwrite the local symbol
definition instead of the package-scoped symbol. This was due to
importsym using s.Def instead of s.PkgDef.

Unfortunately, this is yet another consequence of the current awkward
scope handling code.

Passes toolstash-check.

Fixes #25984.

Change-Id: Ia7033e1749a883e6e979c854d4b12b0b28083dd8
Reviewed-on: https://go-review.googlesource.com/120456
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
2018-06-22 17:57:09 +00:00
Cherry Zhang
78a579316b cmd/compile: convert uint32 to int32 in ARM constant folding rules
MOVWconst's AuxInt is Int32. SSA check complains if the AuxInt
does not fit in int32. Convert uint32 to int32 to make it happy.

The generated code is unchanged. MOVW only cares low 32 bits.

Passes "toolstash -cmp" std cmd for ARM.

Fixes #25993.

Change-Id: I2b6532c9c285ea6d89652505fb7c553f85a98864
Reviewed-on: https://go-review.googlesource.com/120335
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2018-06-22 16:25:32 +00:00
David Chase
c6e455bb11 cmd/compile: conditional on -race, disable inline of go:norace
Adds the appropriate check to inl.go.
Includes tests of both -race+go:norace and plain go:norace.

Fixes #24651.

Change-Id: Id806342430c20baf4679a985d12eea3b677092e0
Reviewed-on: https://go-review.googlesource.com/119195
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-06-19 20:59:10 +00:00
Robert Griesemer
707ca18d97 cmd/compile: more accurate position for select case error message
Fixes #25958.

Change-Id: I1f4808a70c20334ecfc4eb1789f5389d94dcf00e
Reviewed-on: https://go-review.googlesource.com/119755
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-06-19 17:52:23 +00:00
David Chase
c359d759a7 cmd/compile: ensure that operand of ORETURN is not double-walked
Inlining of switch statements into a RETURNed expression
can sometimes lead to the switch being walked twice, which
results in a miscompiled switch statement. The bug depends
on:

1) multiple results
2) named results
3) a return statement whose expression includes a call to a
function containing a switch statement that is inlined.

It may also be significant that the default case of that
switch is a panic(), though that's not proven.

Rearranged the walk case for ORETURN so that double walks are
not possible.  Added a test, because this is so fiddly.
Added a check against double walks, verified that it fires
w/o other fix.

Fixes #25776.

Change-Id: I2d594351fa082632512ef989af67eb887059729b
Reviewed-on: https://go-review.googlesource.com/118318
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2018-06-14 20:08:10 +00:00
Emmanuel T Odeke
7bc99ffa05 cmd/compile: make case insensitive suggestions aware of package
Ensure that compiler error suggestions after case insensitive
field lookups don't mistakenly reported unexported fields if
those fields aren't in the local package being processed.

Fixes #25727

Change-Id: Icae84388c2a82c8cb539f3d43ad348f50a644caa
Reviewed-on: https://go-review.googlesource.com/117755
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-06-13 23:12:54 +00:00
Brad Fitzpatrick
39ad208c13 test: add test to verify that string copies don't get optimized away
Fixes #25834

Change-Id: I33e58dabfd04b84dfee1a9a3796796b5d19862e7
Reviewed-on: https://go-review.googlesource.com/118295
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-06-12 19:10:34 +00:00
Robert Griesemer
48987baa09 cmd/compile: correct alias cycle detection
The original fix (https://go-review.googlesource.com/c/go/+/35831)
for this issue was incorrect as it reported cycles in cases where
it shouldn't.

Instead, use a different approach: A type cycle containing aliases
is only a cycle if there are no type definitions. As soon as there
is a type definition, alias expansion terminates and there is no
cycle.

Approach: Split sprint_depchain into two non-recursive and more
easily understandable functions (cycleFor and cycleTrace),
and use those instead for cycle reporting. Analyze the cycle
returned by cycleFor before issueing an alias cycle error.

Also: Removed original fix (main.go) which introduced a separate
crash (#23823).

Fixes #18640.
Fixes #23823.
Fixes #24939.

Change-Id: Ic3707a9dec40a71dc928a3e49b4868c5fac3d3b7
Reviewed-on: https://go-review.googlesource.com/118078
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-06-12 18:55:39 +00:00
David Chase
c08b01ecb4 cmd/compile: fix panic-okay-to-inline change; adjust tests
This line of the inlining tuning experiment
https://go-review.googlesource.com/c/go/+/109918/1/src/cmd/compile/internal/gc/inl.go#347
was incorrectly rewritten in a later patch to use the call
cost, not the panic cost, and thus the inlining of panic
didn't occur when it should.  I discovered this when I
realized that tests should have failed, but didn't.

Fix is to make the correct change, and also to modify the
tests that this causes to fail.  One test now asserts the
new normal, the other calls "ppanic" instead which is
designed to behave like panic but not be inlined.

Change-Id: I423bb7f08bd66a70d999826dd9b87027abf34cdf
Reviewed-on: https://go-review.googlesource.com/116656
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2018-06-06 20:35:23 +00:00
Yury Smolsky
7c1f361e25 test: add comments for all test actions
Every action has a short annotation.
The errorCheck function has a comment adapted from errchk script.

Removed redundant assigments to tmpDir.

Change-Id: Ifdd1284de046a0ce2aad26bd8da8a8e6a7707a8e
Reviewed-on: https://go-review.googlesource.com/115856
Run-TryBot: Yury Smolsky <yury@smolsky.by>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-06-06 15:12:32 +00:00
Richard Musiol
88dc4aee7c cmd/compile: fix OffPtr with negative offset on wasm
The wasm archtecture was missing a rule to handle OffPtr with a
negative offset. This commit makes it so OffPtr always gets lowered
to I64AddConst.

Fixes #25741

Change-Id: I1d48e2954e3ff31deb8cba9a9bf0cab7c4bab71a
Reviewed-on: https://go-review.googlesource.com/116595
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
2018-06-06 14:40:25 +00:00
Robert Griesemer
9be4f312bf cmd/compile: revert internal parameter rename (from ".anonX" to "") before export
In the old binary export format, parameter names for parameter lists
which contained only types where never written, so this problem didn't
come up.

Fixes #25101.

Change-Id: Ia8b817f7f467570b05f88d584e86b6ef4acdccc6
Reviewed-on: https://go-review.googlesource.com/116376
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2018-06-05 20:22:07 +00:00
Robert Griesemer
4a2bec9726 cmd/compile: fix printing of array types in error messages
Fixes #23094.

Change-Id: I9aa36046488baa5f55cf2099e10cfb39ecd17b06
Reviewed-on: https://go-review.googlesource.com/116256
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-06-05 00:43:22 +00:00
Robert Griesemer
75c1aed345 runtime: slightly better error message for assertion panics with identical looking types
Fixes #18911.

Change-Id: Ice10f37460a4f0a66cddeacfe26c28045f5e60fe
Reviewed-on: https://go-review.googlesource.com/116255
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2018-06-05 00:29:50 +00:00
Keith Randall
06b326054d cmd/compile: include callee args section when checking frame too large
The stack frame includes the callee args section. At the point where
we were checking the frame size, that part of the frame had not been
computed yet. Move the check later so we can include the callee args size.

Fixes #20780
Update #25507

Change-Id: Iab97cb89b3a24f8ca19b9123ef2a111d6850c3fe
Reviewed-on: https://go-review.googlesource.com/115195
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2018-06-02 18:00:44 +00:00
Yury Smolsky
7cb1810fe8 test: skip test/fixedbugs/bug345.go on windows
Before the CL 115277 we did not run the test on Windows,
so let's just go back to not running the test on Windows.
There is nothing OS-specific about this test,
so skipping it on Windows doesn't seem like a big deal.

Updates #25693
Fixes #25586

Change-Id: I1eb3e158b322d73e271ef388f8c6e2f2af0a0729
Reviewed-on: https://go-review.googlesource.com/115857
Run-TryBot: Yury Smolsky <yury@smolsky.by>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-06-01 21:46:07 +00:00
Yury Smolsky
57d40f1b27 test: remove rundircmpout and cmpout actions
This CL removes the rundircmpout action completely
because it is not used anywhere.

The run case already looks for output files. Rename the cmpout action
mentioned in tests to the run action and remove "cmpout" from run.go.

Change-Id: I835ceb70082927f8e9360e0ea0ba74f296363ab3
Reviewed-on: https://go-review.googlesource.com/115575
Run-TryBot: Yury Smolsky <yury@smolsky.by>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-05-31 17:36:45 +00:00
Yury Smolsky
caf968616a test: eliminate use of Perl in fixedbugs/bug345.go
To allow testing of fixedbugs/bug345.go in Go,
a new flag -n is introduced. This flag disables setting
of relative path for local imports and imports search path
to current dir, namely -D . -I . are not passed to the compiler.
Error regexps are fixed to allow running the test in temp directory.

This change eliminates the last place where Perl
script "errchk" was used.

Fixes #25586.

Change-Id: If085f466e6955312d77315f96d3ef1cb68495aef
Reviewed-on: https://go-review.googlesource.com/115277
Run-TryBot: Yury Smolsky <yury@smolsky.by>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-05-31 13:29:50 +00:00
Keith Randall
db9341a024 cmd/compile: update WBLoads during deadcode
When we deadcode-remove a block which is a write barrier test,
remove that block from the list of write barrier test blocks.

Fixes #25516

Change-Id: I1efe732d5476003eab4ad6bf67d0340d7874ff0c
Reviewed-on: https://go-review.googlesource.com/115037
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-05-29 17:45:36 +00:00
Keith Randall
d15d055054 cmd/compile: reject large argument areas
Extend stack frame limit of 1GB to include large argument/return areas.
Argument/return areas are part of the parent frame, not the frame itself,
so they need to be handled separately.

Fixes #25507.

Change-Id: I309298a58faee3e7c1dac80bd2f1166c82460087
Reviewed-on: https://go-review.googlesource.com/115036
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-05-29 16:58:05 +00:00
Josh Bleecher Snyder
002c764533 test: gofmt bounds.go
Change-Id: I8b462e20064658120afc8eb1cbac926254d1e24e
Reviewed-on: https://go-review.googlesource.com/114937
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-05-29 02:39:16 +00:00