1
0
mirror of https://github.com/golang/go synced 2024-11-07 06:46:12 -07:00
Commit Graph

4047 Commits

Author SHA1 Message Date
David Chase
90ec257735 cmd/compile: make the stack allocator more careful about register args.
Assignment between input parameters causes them to have more than
one "Name", and running this backwards from names to values can end
up confusing (conflating) parameter spill slots.

Around 105a6e9518, this cases a stack overflow running
go test -race encoding/pem
because two slice parameters spill (incorrectly) into the same
stack slots (in the AB?I-defined parameter spill area).

This also tickles a failure in cue, which turned out to be
easier to isolate.

Fixes #45851.
Updates #40724.

Change-Id: I39c56815bd6abb652f1ccbe83c47f4f373a125c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/313212
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2021-05-03 17:46:12 +00:00
Cuong Manh Le
844e1fc6f1 cmd/compile: make typecheckaste correctly report invalid use of "..."
Currently, when "..." argument is passed to non-variadic function, the
compiler may skip that check, but continue checking whether the number
of arguments matches the function signature.

That causes the sanity check which was added in CL 255241 trigger.

Instead, we should report an invalid use of "...", which matches the
behavior of new type checker and go/types.

Fixes #45913

Change-Id: Icbb254052cbcd756bbd41f966c2c8e316c44420f
Reviewed-on: https://go-review.googlesource.com/c/go/+/315796
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2021-05-03 15:03:57 +00:00
Matthew Dempsky
fadad851a3 cmd/compile: implement unsafe.Add and unsafe.Slice
Updates #19367.
Updates #40481.

Change-Id: Iabd2afdd0d520e5d68fd9e6dedd013335a4b3886
Reviewed-on: https://go-review.googlesource.com/c/go/+/312214
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Trust: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
2021-05-02 20:38:13 +00:00
Cherry Zhang
5b328c4a2f cmd/compile: use desired register only if it satisfies register mask
In the register allocator, if possible, we allocate a value to its
desired register (the ideal register for its next use). In some
cases the desired register does not satisfies the value's output
register mask. We should not use the register in this case.

In the following example, v33 is going to be returned as a
function result, so it is allocated to its desired register AX.
However, its Op cannot use AX as output, causing miscompilation.

v33 = CMOVQEQF <int> v24 v28 v29 : AX (~R0[int])
v35 = MakeResult <int,int,mem> v33 v26 v18
Ret v35

Change-Id: Id0f4f27c4b233ee297e83077e3c8494fe193e664
Reviewed-on: https://go-review.googlesource.com/c/go/+/314630
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
2021-04-28 16:13:40 +00:00
Cherry Zhang
becb9a278f test: do not run fuse test in noopt mode
Change-Id: Iad8ac2253ce28fd0a331bde36836d1b7f25797bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/314632
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-04-28 15:55:10 +00:00
eric fang
92d1afe989 cmd/compile/ssa: optimize the derivable known branch of If block
When the control value of a If block is known for a particular inbound edge
because its value can be inferred from the control value of its predecessor,
then this inbound edge can be redirected to the known successor directly,
This CL optimizes this kind of cases to eliminate unnecessary comparision.

For example, the following piece of code comes from runtime.atoi,
if !neg && un > uint(maxInt) {
	return 0, false
}
if neg && un > uint(maxInt)+1 {
	return 0, false
}

Before this optimization, if the first "if" statement does not return, both
conditions of the second "if" statement will be checked. But obviously the
value of neg is known through the first "if" statement, and there is no need
to check neg repeatedly.

After this optimization, this redundancy check is eliminated, and the execution
logic becomes as follows.
if !neg {
	if un > uint(maxInt) {
		return 0, false
	}
} else {
	if un > uint(maxInt)+1 {
		return 0, false
	}
}

This CL does not bring significant performance changes, but it makes the code
structure look more reasonable.

Statistical data from tool compilecmp on Linux/amd64:
name                      old time/op       new time/op       delta
Template                        380ms ± 4%        385ms ± 3%  +1.16%  (p=0.000 n=50+49)
Unicode                         168ms ± 9%        169ms ± 9%    ~     (p=0.421 n=49+46)
GoTypes                         1.99s ± 4%        2.02s ± 4%  +1.48%  (p=0.000 n=49+49)
Compiler                        188ms ± 8%        188ms ± 9%    ~     (p=0.997 n=49+50)
SSA                             11.8s ± 2%        12.0s ± 2%  +1.24%  (p=0.000 n=48+50)
Flate                           242ms ± 6%        244ms ± 9%    ~     (p=0.307 n=46+49)
GoParser                        361ms ± 3%        366ms ± 4%  +1.23%  (p=0.000 n=48+49)
Reflect                         836ms ± 3%        842ms ± 3%  +0.70%  (p=0.004 n=48+48)
Tar                             335ms ± 3%        340ms ± 4%  +1.47%  (p=0.000 n=49+46)
XML                             432ms ± 4%        437ms ± 4%  +1.11%  (p=0.002 n=49+49)
LinkCompiler                    701ms ± 4%        704ms ± 5%    ~     (p=0.278 n=49+50)
ExternalLinkCompiler            1.83s ± 3%        1.84s ± 3%  +0.51%  (p=0.034 n=48+49)
LinkWithoutDebugCompiler        436ms ± 6%        438ms ± 6%    ~     (p=0.419 n=48+49)
[Geo mean]                      612ms             617ms       +0.84%

name                      old alloc/op      new alloc/op      delta
Template                       38.7MB ± 1%       39.1MB ± 1%  +1.19%  (p=0.000 n=50+50)
Unicode                        28.1MB ± 0%       28.1MB ± 0%  +0.20%  (p=0.000 n=49+45)
GoTypes                         168MB ± 1%        170MB ± 1%  +1.05%  (p=0.000 n=48+49)
Compiler                       23.0MB ± 1%       23.1MB ± 1%  +0.63%  (p=0.000 n=50+50)
SSA                            1.54GB ± 1%       1.55GB ± 1%  +0.85%  (p=0.000 n=50+50)
Flate                          23.6MB ± 1%       23.9MB ± 1%  +1.36%  (p=0.000 n=43+46)
GoParser                       35.0MB ± 1%       35.3MB ± 1%  +0.94%  (p=0.000 n=50+50)
Reflect                        84.7MB ± 1%       86.1MB ± 1%  +1.72%  (p=0.000 n=49+49)
Tar                            34.5MB ± 1%       34.9MB ± 1%  +1.07%  (p=0.000 n=47+48)
XML                            44.2MB ± 3%       44.6MB ± 3%  +0.70%  (p=0.003 n=50+49)
LinkCompiler                    128MB ± 0%        128MB ± 0%  +0.01%  (p=0.004 n=49+50)
ExternalLinkCompiler            120MB ± 0%        120MB ± 0%  +0.01%  (p=0.000 n=49+50)
LinkWithoutDebugCompiler       77.3MB ± 0%       77.3MB ± 0%  +0.02%  (p=0.000 n=50+50)
[Geo mean]                     69.1MB            69.6MB       +0.75%

file      before    after     Δ       %
addr2line 4049276   4051308   +2032   +0.050%
api       5248940   5248996   +56     +0.001%
asm       4868093   4868037   -56     -0.001%
buildid   2627666   2626026   -1640   -0.062%
cgo       4614432   4615040   +608    +0.013%
compile   23298888  23301267  +2379   +0.010%
cover     4591609   4591161   -448    -0.010%
dist      3449638   3450254   +616    +0.018%
doc       3925667   3926363   +696    +0.018%
fix       3322936   3323464   +528    +0.016%
link      6628632   6629560   +928    +0.014%
nm        3991753   3996497   +4744   +0.119%
objdump   4396119   4395615   -504    -0.011%
pack      2399719   2399535   -184    -0.008%
pprof     13616418  13622866  +6448   +0.047%
test2json 2646121   2646081   -40     -0.002%
trace     10233087  10226359  -6728   -0.066%
vet       7117994   7121066   +3072   +0.043%
total     111026988 111039495 +12507  +0.011%

On linux arm64:
name                      old time/op       new time/op       delta
Template                        284ms ± 1%        286ms ± 1%  +0.70%  (p=0.000 n=49+50)
Unicode                         125ms ± 3%        125ms ± 2%    ~     (p=0.548 n=50+50)
GoTypes                         1.69s ± 1%        1.71s ± 1%  +1.02%  (p=0.000 n=49+49)
Compiler                        125ms ± 1%        124ms ± 2%  -0.35%  (p=0.020 n=50+50)
SSA                             12.7s ± 1%        12.8s ± 1%  +1.21%  (p=0.000 n=49+49)
Flate                           172ms ± 1%        173ms ± 1%  +0.20%  (p=0.047 n=50+50)
GoParser                        265ms ± 1%        266ms ± 1%  +0.64%  (p=0.000 n=50+50)
Reflect                         651ms ± 1%        650ms ± 1%    ~     (p=0.079 n=48+48)
Tar                             246ms ± 1%        246ms ± 1%    ~     (p=0.202 n=50+46)
XML                             328ms ± 1%        332ms ± 1%  +1.28%  (p=0.000 n=50+49)
LinkCompiler                    600ms ± 1%        599ms ± 1%    ~     (p=0.264 n=50+50)
ExternalLinkCompiler            1.88s ± 1%        1.90s ± 0%  +1.36%  (p=0.000 n=50+50)
LinkWithoutDebugCompiler        365ms ± 1%        365ms ± 1%    ~     (p=0.602 n=50+46)
[Geo mean]                      490ms             492ms       +0.47%

name                      old alloc/op      new alloc/op      delta
Template                       38.8MB ± 1%       39.1MB ± 1%  +0.92%  (p=0.000 n=44+42)
Unicode                        28.4MB ± 0%       28.4MB ± 0%  +0.22%  (p=0.000 n=44+45)
GoTypes                         169MB ± 1%        171MB ± 1%  +1.12%  (p=0.000 n=50+50)
Compiler                       23.2MB ± 1%       23.3MB ± 1%  +0.56%  (p=0.000 n=42+43)
SSA                            1.55GB ± 0%       1.56GB ± 0%  +0.91%  (p=0.000 n=48+49)
Flate                          23.7MB ± 2%       24.0MB ± 1%  +1.20%  (p=0.000 n=50+50)
GoParser                       35.3MB ± 1%       35.6MB ± 1%  +0.88%  (p=0.000 n=50+50)
Reflect                        85.0MB ± 0%       86.5MB ± 0%  +1.70%  (p=0.000 n=49+48)
Tar                            34.5MB ± 1%       34.9MB ± 1%  +1.03%  (p=0.000 n=47+50)
XML                            43.8MB ± 2%       44.0MB ± 0%  +0.41%  (p=0.002 n=49+38)
LinkCompiler                    136MB ± 0%        136MB ± 0%  +0.01%  (p=0.006 n=50+49)
ExternalLinkCompiler            127MB ± 0%        127MB ± 0%  +0.02%  (p=0.000 n=49+50)
LinkWithoutDebugCompiler       84.1MB ± 0%       84.1MB ± 0%    ~     (p=0.534 n=50+50)
[Geo mean]                     70.4MB            70.9MB       +0.69%

file      before    after     Δ       %
addr2line 4006004   4004556   -1448   -0.036%
api       5029716   5028828   -888    -0.018%
asm       4936863   4934943   -1920   -0.039%
buildid   2594947   2594099   -848    -0.033%
cgo       4399702   4399502   -200    -0.005%
compile   22233139  22230486  -2653   -0.012%
cover     4443681   4443881   +200    +0.005%
dist      3365902   3365486   -416    -0.012%
doc       3776175   3776151   -24     -0.001%
fix       3218624   3218600   -24     -0.001%
link      6365001   6361409   -3592   -0.056%
nm        3923345   3923065   -280    -0.007%
objdump   4295473   4296673   +1200   +0.028%
pack      2390561   2389393   -1168   -0.049%
pprof     12866419  12865115  -1304   -0.010%
test2json 2587113   2585561   -1552   -0.060%
trace     9609814   9610846   +1032   +0.011%
vet       6790272   6789760   -512    -0.008%
total     106832751 106818354 -14397  -0.013%

Update: #37608

Change-Id: I2831238b12e3af5aef2261f64f804bf0a8b43f86
Reviewed-on: https://go-review.googlesource.com/c/go/+/244737
Reviewed-by: eric fang <eric.fang@arm.com>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: eric fang <eric.fang@arm.com>
Run-TryBot: eric fang <eric.fang@arm.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-04-28 02:39:09 +00:00
Cuong Manh Le
c9f43507c6 cmd/compile: fix typechecking logical operators panic with non-boolean operand
In CL 255899, we added code to make clearer error when non-bool used
as operand to logical operators. The code is safe, because node type
is guaranteed to be non-nil.

In CL 279442, we refactored typechecking arith, including moving
typechecking logical operators to separate case. Now we have to
explicitly check if operand type is not nil, because calling Expr can
set operand type nil for non-bool operands.

Fixes #45804

Change-Id: Ie2b6e18f65c0614a803b343f60e78ee1d660bbeb
Reviewed-on: https://go-review.googlesource.com/c/go/+/314209
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2021-04-28 00:49:58 +00:00
Keith Randall
f432d3fc41 cmd/compile: fix nongeneric closures in generic functions
Ensure that formal parameter Names are correctly copied and marked
with the correct Curfn. We need to ensure this even when the underlying
closure has no type parameters.

(Aside: it is strange that the types of things contain formal
parameter names that need to be copied. Maybe that's an underlying
larger problem that needs to be fixed.)

Fixes #45738

Change-Id: Ia13d69eea992ff7080bd44065115bc52eb624e73
Reviewed-on: https://go-review.googlesource.com/c/go/+/313652
Trust: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
2021-04-27 19:30:11 +00:00
Dan Scales
55c517a8b3 cmd/compile: fix handling of ir.CurFunc during stenciling
The transform functions (specifically transformArgs, which is used from
transformCall/transformReturn) require that ir.CurFunc is set correctly.
Since transformCall() is used on the call of an instantiated generic
function, we need to set ir.CurFunc correctly in stencil(). Also,
correctly save/restore ir.CurFunc in genericSubst().

Without this fix, ir.CurFunc can be nil when we call TransformCall()
from stencil(), which leads to some temp variables being added
incorrectly to ir.TodoFunc (which leads to the fatal panic in the
issue).

Fixes #45722

Change-Id: Iddf4a67d28f2100dde8cde5dbc9ca1e00dad6089
Reviewed-on: https://go-review.googlesource.com/c/go/+/313869
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
2021-04-27 16:55:20 +00:00
Cuong Manh Le
40254ec0db cmd/compile: fix wrong package path for unsafe.Pointer
It's not a predeclared type, but a type defined in "unsafe" package.

Fixes #44830

Change-Id: If39815b1070059b608be8231dfac9b7f3307cb15
Reviewed-on: https://go-review.googlesource.com/c/go/+/313349
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2021-04-27 01:16:39 +00:00
Matthew Dempsky
9f601690da cmd/compile: workaround inlining of closures with type switches
Within clovar, n.Defn can also be *ir.TypeSwitchGuard. The proper fix
here would be to populate m.Defn and have it filled in too, but we
already leave it nil in inlvar. So for consistency, this CL does the
same in clovar too.

Eventually inl.go should be rewritten to fully respect IR invariants.

Fixes #45743.

Change-Id: I8b38e5d8b2329ad242de97670f2141f713954d28
Reviewed-on: https://go-review.googlesource.com/c/go/+/313289
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Trust: Dan Scales <danscales@google.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
2021-04-26 21:27:41 +00:00
Matthew Dempsky
691e1b84c1 cmd/compile: generalize fix for lvalue-init evaluation
The previous fix to ensure early evaluation of lvalue-init statements
(CL 312632) added it after we'd already peeled away any array-OINDEX
expressions. But those might have init statements too, so we need to
do this earlier actually and perhaps more than once.

Longer term, lvalue expressions shouldn't have init statements anyway.
But rsc and I both spent a while looking into this earlier in the dev
cycle and couldn't come up with anything reasonable.

Fixes #45706.

Change-Id: I2d19c5ba421b3f019c62eec45774c84cf04b30ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/313011
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2021-04-23 20:57:54 +00:00
Cuong Manh Le
8c66669764 cmd/compile: make sure ascompatee walk lhs init statements
CL 281152 improved ascompatee by removing the call to safeExpr on lhs.
But we forgot that lhs int statements, if any, must be walked prior
saving subexpressions, which cause the bug in #45706.

Fixes #45706

Change-Id: I0064315056ef4ca92ebf3c332c2e3a9bb2b26f68
Reviewed-on: https://go-review.googlesource.com/c/go/+/312632
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2021-04-23 17:36:11 +00:00
Cuong Manh Le
d310b2a6b8 cmd/compile: set correct Defn for inlined vars
Currently, when copying definition node of an inlined var, we do not
update var Defn field to point to new copied node. That causes all
inlined vars point to the same Defn, and ir.StaticValue can not find
inlined var in the lhs of its definition.

clovar creates new ONAME node for local variables or params of closure
inside inlined function, by copying most of the old node fields. So the
new Node.Defn is not modified, its lhs still refer to old node
instead of new one.

To fix this, we need to do two things:

 - In subst.clovar, set a dummy Defn node for inlvar
 - During subst.node, when seeing OAS/OAS2 nodes, after substituting, we
   check if any node in lhs has the dummy Defn, then set it to the current
   OAS/OAS2 node.

Fixes #45606

Change-Id: Ib517b753a7643756dcd61d36deae60f1a0fc53c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/312630
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2021-04-23 05:10:41 +00:00
Than McIntosh
cfac62a1cc cmd/compile: fix bug in defer wrapping
The defer wrapping feature added to the compiler's "order" phase
creates temporaries into which it copies defer arguments. If one of
these temps is large enough that we place it into the defer closure by
address (as opposed to by value), then the temp in question can't be
reused later on in the order phase, nor do we want a VARKILL
annotation for it at the end of the current block scope.

Test written by Cherry.

Updates #40724.

Change-Id: Iec7efd87ec5a3e3d7de41cdcc7f39c093ed1e815
Reviewed-on: https://go-review.googlesource.com/c/go/+/312869
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2021-04-23 01:16:19 +00:00
Matthew Dempsky
14056d0d00 cmd/compile/internal/types2: add unsafe.Add and unsafe.Slice
This is a port of CL 312212, CL 312591 (except check_test.go), and
CL 312790 to types2.

Updates #19367.
Updates #40481.

Change-Id: I58ba0b0dad157baba3f82c909d5eb1268b931be4
Reviewed-on: https://go-review.googlesource.com/c/go/+/312511
Trust: Matthew Dempsky <mdempsky@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2021-04-23 00:41:01 +00:00
Cherry Zhang
e8666abd98 cmd/compile: keep call's args in elim dead auto pass
If the address of an auto is used in a Call, we need to keep it,
as we keep the Call itself.

Fixes #45693.

Change-Id: Ie548d6dffc95bf916868a8885d4ab4cf9e86355a
Reviewed-on: https://go-review.googlesource.com/c/go/+/312670
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2021-04-22 16:38:37 +00:00
Keith Randall
255056395e test: add a field tracking test
Now that we can set experiments at build time instead of make.bash time,
we can actually write a test for field tracking!

Update #20014

This CL contains a test for the functionality fixed in CL 312069.

Change-Id: I7569a7057bbc7c88ae25ae7bf974b0c8a4e35be8
Reviewed-on: https://go-review.googlesource.com/c/go/+/312217
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2021-04-21 20:24:34 +00:00
Cherry Zhang
e5bc4f2a77 cmd/compile: reenable name preservation on copies in expand_calls
This reverts CL 311829, and reenables CL 309330. The issue
should be fixed in the previous CL.

Change-Id: I69db0565c72470a1814f135d8f8ec62c781bfc5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/312094
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
2021-04-21 14:16:01 +00:00
Cuong Manh Le
5f1df260a9 cmd/compile: allow export/import OSLICE2ARRPTR
Updates #395
Fixes #45665

Change-Id: Iaf053c0439a573e9193d40942fbdb22ac3b4d3bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/312070
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-04-21 09:07:09 +00:00
Josh Bleecher Snyder
faa4fa1a6e cmd/compile: allow conversion from slice to array ptr
Panic if the slice is too short.

Updates #395

Change-Id: I90f4bff2da5d8f3148ba06d2482084f32b25c29a
Reviewed-on: https://go-review.googlesource.com/c/go/+/301650
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2021-04-21 00:53:48 +00:00
Cherry Zhang
109d7580a5 cmd/compile: disable name preservation on copies in expand_calls
Apparently CL 309330 caused the compiler OOMing on some large
input (giant generated switch statement). I don't quite understand
it for now. Disable it for now.

Change-Id: I19c84f3f5e158897bff0b32d6217fcff3c66874d
Reviewed-on: https://go-review.googlesource.com/c/go/+/311829
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-04-20 16:44:18 +00:00
Cherry Zhang
3ff6ff7f84 cmd/compile: preserve pointerness when creating map key temp
When creating the temporary for map functions, if the key
contains pointer, we need to create pointer-typed temporary. So
if the temporary is live across a function call, the pointer is
live.

Change-Id: Id6e14ec9def8bc7987f0f8ce8423caf1e3754fcb
Reviewed-on: https://go-review.googlesource.com/c/go/+/311379
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
2021-04-20 14:30:03 +00:00
Cherry Zhang
6b8e3e2d06 cmd/compile: reduce redundant register moves for regabi calls
Currently, if we have AX=a and BX=b, and we want to make a call
F(1, a, b), to move arguments into the desired registers it emits

	MOVQ AX, CX
	MOVL $1, AX // AX=1
	MOVQ BX, DX
	MOVQ CX, BX // BX=a
	MOVQ DX, CX // CX=b

This has a few redundant moves.

This is because we process inputs in order. First, allocate 1 to
AX, which kicks out a (in AX) to CX (a free register at the
moment). Then, allocate a to BX, which kicks out b (in BX) to DX.
Finally, put b to CX.

Notice that if we start with allocating CX=b, then BX=a, AX=1,
we will not have redundant moves. This CL reduces redundant moves
by allocating them in different order: First, for inpouts that are
already in place, keep them there. Then allocate free registers.
Then everything else.

                             before       after
cmd/compile binary size     23703888    23609680
            text size        8565899     8533291

(with regabiargs enabled.)

Change-Id: I69e1bdf745f2c90bb791f6d7c45b37384af1e874
Reviewed-on: https://go-review.googlesource.com/c/go/+/311371
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
2021-04-19 16:21:39 +00:00
Cherry Zhang
b21e739f87 test: add test for CL 310589
Change-Id: Iff0876bd17c2a93db72dc90678f3a46ef8effd74
Reviewed-on: https://go-review.googlesource.com/c/go/+/311370
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2021-04-19 14:51:24 +00:00
Cherry Zhang
a9c244a849 test: add liveness test for regabi
With defer/go wrapping and register arguments, some liveness info
changed and live.go test was disabled for regabi. This CL adds a
new one for regabi.

Change-Id: I65f03a6ef156366d8b76c62a16251c3e818f4b02
Reviewed-on: https://go-review.googlesource.com/c/go/+/311369
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
2021-04-19 14:42:58 +00:00
Cuong Manh Le
a72622d028 cmd/compile: skip "_" function in reflectdata.MarkUsedIfaceMethod
CL 256798 added compiler ability to retain only used interface methods,
by generating a mark relocation whenever an interface method is used. To
do that, the compiler needs the current function linker object.

However, for unnamed function "func _()", its linker object is nil,
causes the compiler crashes for code in #45258.

CL 283313 fixed the code in #45258 unintentionally, since when the
compiler now does not walk unnamed functions anymore.

This CL fixes the root issue, by making reflectdata.MarkUsedIfaceMethod
skips unnamed functions, and also adding regression test.

Fixes #45258

Change-Id: I4cbefb0a89d9928f70c00dc8a271cb61cd20a49c
Reviewed-on: https://go-review.googlesource.com/c/go/+/311130
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2021-04-19 04:37:32 +00:00
Russ Cox
dba89283ad cmd/go, go/build: add ToolTags to build.Default
The build.Default context really needs to accurately describe
the default build context. The goexperiment tags being a special
case in the go command violates that rule and is the root cause
of the various try-bot failures blocking the enabling of regabi.

(The cleanups I made in golang.org/x/tools were long overdue
but are not strictly necessary for making regabi work; this CL is.)

Having moved the GOEXPERIMENT parsing into internal/buildcfg,
go/build can now use it to set up build.Default, in the new field
ToolTags, meant to hold toolchain-determined tags (for now,
just the experiments). And at the same time we can remove the
duplication of GOOS and GOARCH defaults.

And then once build.Default is set up accurately, the special case
code in cmd/go itself can be removed, and the special case code
in test/run.go is at least a bit less special.

Change-Id: Ib7394e10aa018e492cb9a83fb8fb9a5011a8c25b
Reviewed-on: https://go-review.googlesource.com/c/go/+/310732
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Austin Clements <austin@google.com>
2021-04-16 19:21:48 +00:00
David Chase
52df9291aa test/abi: reenable test on windows
Reverses CL 308889.
Fixes #45465.
Updates #40724.

Change-Id: I34b0d396dc34d0ec8c216e9b6a668de9dfce677c
Reviewed-on: https://go-review.googlesource.com/c/go/+/310649
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2021-04-16 13:38:02 +00:00
Austin Clements
d26fc68aa1 cmd/internal/objabi,test: use correct GOEXPERIMENT build tags in test/run.go
Currently, run.go sets GOEXPERIMENT build tags based on the
*difference* from the baseline experiment configuration, rather than
the absolute experiment configuration. This differs from cmd/go. As a
result, if we set a baseline configuration and don't override it with
a GOEXPERIMENT setting, run.go won't set any GOEXPERIMENT build tags,
instead of setting the tags corresponding to the baseline
configuration.

Fix this by making compile -V=goexperiment produce the full
GOEXPERIMENT configuration, which run.go can then use to set exactly
the right set of build tags.

For #40724.

Change-Id: Ieda6ea62f1a1fabbe8d749d6d09c198fd5ca8377
Reviewed-on: https://go-review.googlesource.com/c/go/+/310171
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2021-04-16 03:16:55 +00:00
Robert Griesemer
a63ff398d5 cmd/compile/internal/syntax: fix error message for ... without type
Only complain about missing type; leave it to type-checking
to decide whether "..." is permitted in the first place.

Fixes #43674.

Change-Id: Icbc8f084e364fe3ac16076406a134354219c08d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/310209
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-04-15 19:41:38 +00:00
Keith Randall
083a26c7d2 cmd/compile: propagate pragmas from generic function to stenciled implementation
Change-Id: I28a1910890659aaa449ffd2a847cd4ced5a8600d
Reviewed-on: https://go-review.googlesource.com/c/go/+/310211
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
2021-04-15 00:29:05 +00:00
Dan Scales
bf634c76b2 cmd/compile: look for function in instantiations in all global assignments
Add in some missing global assignment ops to the list of globals ops
that should be traversed to look for generic function instantiations.
The most common other one for global assigments (and the relevant one
for this bug) is OAS2FUNC, but also look at global assigments with
OAS2DOTTYPE, OAS2MAPR, OAS2RECV, and OASOP.

Bonus small fix: get rid of -G=3 case in ir.IsAddressable. Now that we
don't call the old typechecker from noder2, we don't need this -G-3
check anymore.

Fixes #45547.

Change-Id: I75fecec55ea0d6f62e1c2294d4d77447ed9be6ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/310210
Trust: Dan Scales <danscales@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2021-04-15 00:14:55 +00:00
David Chase
4df3d0e4df cmd/compile: rescue stmt boundaries from OpArgXXXReg and OpSelectN.
Fixes this failure:
go test cmd/compile/internal/ssa -run TestStmtLines -v
=== RUN   TestStmtLines
    stmtlines_test.go:115: Saw too many (amd64, > 1%) lines without
    statement marks, total=88263, nostmt=1930
    ('-run TestStmtLines -v' lists failing lines)

The failure has two causes.

One is that the first-line adjuster in code generation was relocating
"first lines" to instructions that would either not have any code generated,
or would have the statment marker removed by a different believed-good heuristic.

The other was that statement boundaries were getting attached to register
values (that with the old ABI were loads from the stack, hence real instructions).
The register values disappear at code generation.

The fixes are to (1) note that certain instructions are not good choices for
"first value" and skip them, and (2) in an expandCalls post-pass, look for
register valued instructions and under appropriate conditions move their
statement marker to a compatible use.

Also updates TestStmtLines to always log the score, for easier comparison of
minor compiler changes.

Updates #40724.

Change-Id: I485573ce900e292d7c44574adb7629cdb4695c3f
Reviewed-on: https://go-review.googlesource.com/c/go/+/309649
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2021-04-14 18:46:08 +00:00
Keith Randall
6d8ba77896 cmd/compile: fix importing of method expressions
For OMETHEXPR, the Name in the Selection needs to be properly
linked up to the method declaration. Use the same code we
already have for ODOTMETH and OCALLPART to do that.

Fixes #45503

Change-Id: I7d6f886d606bae6faad8c104f50c177f871d41c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/309831
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Dan Scales <danscales@google.com>
2021-04-14 04:02:01 +00:00
Dan Scales
eb433ed5a2 cmd/compile: set types properly for imported funcs with closures
For the new export/import of node types, we were just missing setting
the types of the closure variables (which have the same types as the
captured variables) and the OCLOSURE node itself (which has the same
type as the Func node).

Re-enabled inlining of functions with closures.

Change-Id: I687149b061f3ffeec3244ff02dc6e946659077a9
Reviewed-on: https://go-review.googlesource.com/c/go/+/308974
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2021-04-14 01:28:16 +00:00
Cherry Zhang
444d28295b test: make codegen/memops.go work with both ABIs
Following CL 309335, this fixes memops.go.

Change-Id: Ia2458b5267deee9f906f76c50e70a021ea2fcb5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/309552
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
2021-04-13 14:07:17 +00:00
Cherry Zhang
263e13d1f7 test: make codegen tests work with both ABIs
Some codegen tests were written with the assumption that
arguments and results are in memory, and with a specific stack
layout. With the register ABI, the assumption is no longer true.
Adjust the tests to work with both cases.

- For tests expecting in memory arguments/results, change to use
  global variables or memory-assigned argument/results.

- Allow more registers. E.g. some tests expecting register names
  contain only letters (e.g. AX), but  it can also contain numbers
  (e.g. R10).

- Some instruction selection changes when operate on register vs.
  memory, e.g. ADDQ vs. LEAQ, MOVB vs. MOVL. Accept both.

TODO: mathbits.go and memops.go still need fix.
Change-Id: Ic5932b4b5dd3f5d30ed078d296476b641420c4c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/309335
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2021-04-12 21:59:59 +00:00
Cherry Zhang
9ed0e32059 test: consider default GOEXPERIMENT when matching build tags
If GOEXPERIMENT environment variable is unset, use the default
value that is baked into the toolchain (instead of no
experiments).

Change-Id: I41f863e6f7439f2d53e3ebd25a7d9cf4a176e32e
Reviewed-on: https://go-review.googlesource.com/c/go/+/309333
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
2021-04-12 16:25:59 +00:00
Keith Randall
1129a60f1c cmd/compile: include typecheck information in export/import
Include type information on exported function bodies, so that the
importer does not have to re-typecheck the body. This involves
including type information in the encoded output, as well as
avoiding some of the opcode rewriting and other changes that the
old exporter did assuming there would be a re-typechecking pass.

This CL could be considered a cleanup, but is more important than that
because it is an enabling change for generics. Without this CL, we'd
have to upgrade the current typechecker to understand generics. With
this CL, the current typechecker can mostly go away in favor of the
types2 typechecker.

For now, inlining of functions that contain closures is turned off.
We will hopefully resolve this before freeze.

Object files are only 0.07% bigger.

Change-Id: I85c9da09f66bfdc910dc3e26abb2613a1831634d
Reviewed-on: https://go-review.googlesource.com/c/go/+/301291
Trust: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Dan Scales <danscales@google.com>
2021-04-10 14:58:18 +00:00
Cherry Zhang
5305bdedb0 test: do not run (another) softfloat test with regabiargs
I missed one in CL 308710.

Change-Id: Ia277eaba6982f4944992d1bee1e11775934b789f
Reviewed-on: https://go-review.googlesource.com/c/go/+/309151
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
2021-04-09 23:18:47 +00:00
David Chase
d11968012c test/abi: disable test with old-style build tag known to run.go
A quick check of the source to run.go suggests that it does not
look for the new-style build tags.

Updates #45465.

Change-Id: Ib4be040935d71e732f81d52c4a22c2b514195f40
Reviewed-on: https://go-review.googlesource.com/c/go/+/308934
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Trust: David Chase <drchase@google.com>
2021-04-09 17:37:22 +00:00
David Chase
d138ee2cfb test/abi: disable test on windows for now
This tickles some other bug, do this to clear builders.

Updates #40724.
Updates #45465.

Change-Id: Id51efbcf474865da231fcbc6216e5d604f99c296
Reviewed-on: https://go-review.googlesource.com/c/go/+/308889
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-04-09 14:31:01 +00:00
Cherry Zhang
d25c4fbe05 test: do not run softfloat test with regabiargs
Softfloat mode with register ABI is not implemented yet. In
particular, we did not rewrite the float types in AuxCalls to
integer types, so arguments are still passed in floating point
registers, which do not exist in softfloat mode. To make it work
I think we may want to reorder softfloat pass with expand_calls
pass. We also need to rewrite the OpArgFloatRegs for the spilling
of non-SSA-able arguments, which may involve renumbering interger
arguments. Maybe in softfloat mode we want to just define the
ABI with 0 float registers. They are not fundamentally hard, but
may be not worth doing for the moment, as we don't use softfloat
mode on AMD64 anyway.

Run the test with noregabiargs. Also in the compiler reject
-d=softfloat if regabiargs is enabled.

Change-Id: I8cc0c2cfa88a138bc1338ed8710670245f1bd2cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/308710
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
2021-04-09 00:11:25 +00:00
Than McIntosh
bb76193a7f cmd/compile: fix buglet in walk convert phase relating to convF32/64
The helper function used by the compiler's walk phase to determine
whether a param can be passed in a single float register wasn't quite
correct (didn't allow for the possibility of struct with two fields,
first zero size and second float). Fix up the helper to take this
case into account.

Updates #40724.

Change-Id: I55b42a1b17ea86de1d696788f029ad3aae4a179c
Reviewed-on: https://go-review.googlesource.com/c/go/+/308689
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2021-04-08 19:58:50 +00:00
Than McIntosh
ca8540affd cmd/compile: fix buglet in walk convert phase relating to convT64
The function runtime.convT64 accepts a single uint64 argument, but the
compiler's rules in the walk phase for determining whether is it ok to
pass a value of type T to a call to runtime.convT64 were slightly off.
In particular the test was allowing a type T with size less than eight
bytes but with more than one internal element (e.g. a struct). This
patch tightens up the rules somewhat to prevent this from happening.

Updates #40724.

Change-Id: I3b909267534db59429b0aa73a3d73333e1bd6432
Reviewed-on: https://go-review.googlesource.com/c/go/+/308069
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2021-04-08 17:44:21 +00:00
David Chase
d474b6c824 test/abi: clean up test to fix builders
go.mod file was not tidy, made builders sad.

Updates #40724.

Change-Id: I28371a1093108f9ec473eb20bb4d185e35dee67d
Reviewed-on: https://go-review.googlesource.com/c/go/+/308590
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2021-04-08 17:43:33 +00:00
David Chase
283b02063b cmd/compile: sanitize before/after expansion OpSelectN references
In expand_calls, OpSelectN occurs both before and after the rewriting.
Attempting to rewrite a post-expansion OpSelectN is bad.
(The only ones rewritten in place are the ones returning mem;
others are synthesized to replace other selection chains with
register references.)

Updates #40724.
Updates #44816#issuecomment-815258897.

Change-Id: I7b6022cfb47f808d3ce6cc796c067245f36047f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/308309
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
2021-04-08 15:03:31 +00:00
Robert Griesemer
4bbe046aad cmd/compile/internal/syntax: add "~" operator
Change-Id: I7991103d97b97260d9615b7f5baf7ec75ad87d1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/307370
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-04-07 05:19:41 +00:00
David Chase
8462169b5a cmd/compile: pre-spill pointers in aggregate-typed register args
There's a problem in liveness, where liveness of any
part of an aggregate keeps the whole aggregate alive,
but the not-live parts don't get spilled.  The GC
can observe those live-but-not-spilled slots, which
can contain junk.

A better fix is to change liveness to work
pointer-by-pointer, but that is also a riskier,
trickier fix.

To avoid this, in the case of

(1) an aggregate input parameter
(2) containing pointers
(3) passed in registers

pre-spill the pointers.

Updates #40724.

Change-Id: I6beb8e0a353b1ae3c68c16072f56698061922c04
Reviewed-on: https://go-review.googlesource.com/c/go/+/307909
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2021-04-07 03:42:11 +00:00