Skip to content

Commit c0cfe96

Browse files
committed
cmd/compile: rewrite f(g()) for multi-value g() during typecheck
This is a re-attempt at CL 153841, which caused two regressions: 1. crypto/ecdsa failed to build with -gcflags=-l=4. This was because when "t1, t2, ... := g(); f(t1, t2, ...)" was exported, we were losing the first assignment from the call's Ninit field. 2. net/http/pprof failed to run with -gcflags=-N. This is due to a conflict with CL 159717: as of that CL, package-scope initialization statements are executed within the "init.ializer" function, rather than the "init" function, and the generated temp variables need to be moved accordingly too. [Rest of description is as before.] This CL moves order.go's copyRet logic for rewriting f(g()) into t1, t2, ... := g(); f(t1, t2, ...) earlier into typecheck. This allows the rest of the compiler to stop worrying about multi-value functions appearing outside of OAS2FUNC nodes. This changes compiler behavior in a few observable ways: 1. Typechecking error messages for builtin functions now use general case error messages rather than unnecessarily differing ones. 2. Because f(g()) is rewritten before inlining, saved inline bodies now see the rewritten form too. This could be addressed, but doesn't seem worthwhile. 3. Most notably, this simplifies escape analysis and fixes a memory corruption issue in esc.go. See #29197 for details. Fixes #15992. Fixes #29197. Change-Id: I930b10f7e27af68a0944d6c9bfc8707c3fab27a4 Reviewed-on: https://go-review.googlesource.com/c/go/+/166983 Run-TryBot: Matthew Dempsky <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Robert Griesemer <[email protected]>
1 parent 032acb3 commit c0cfe96

File tree

14 files changed

+167
-268
lines changed

14 files changed

+167
-268
lines changed

src/cmd/compile/internal/gc/esc.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1604,13 +1604,6 @@ func (e *EscState) esccall(call *Node, parent *Node) {
16041604
}
16051605

16061606
argList := call.List
1607-
if argList.Len() == 1 {
1608-
arg := argList.First()
1609-
if arg.Type.IsFuncArgStruct() { // f(g())
1610-
argList = e.nodeEscState(arg).Retval
1611-
}
1612-
}
1613-
16141607
args := argList.Slice()
16151608

16161609
if indirect {

src/cmd/compile/internal/gc/fmt.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1404,14 +1404,11 @@ func (n *Node) exprfmt(s fmt.State, prec int, mode fmtMode) {
14041404
}
14051405
mode.Fprintf(s, "sliceheader{%v,%v,%v}", n.Left, n.List.First(), n.List.Second())
14061406

1407-
case OCOPY:
1408-
mode.Fprintf(s, "%#v(%v, %v)", n.Op, n.Left, n.Right)
1409-
1410-
case OCOMPLEX:
1411-
if n.List.Len() == 1 {
1412-
mode.Fprintf(s, "%#v(%v)", n.Op, n.List.First())
1413-
} else {
1407+
case OCOMPLEX, OCOPY:
1408+
if n.Left != nil {
14141409
mode.Fprintf(s, "%#v(%v, %v)", n.Op, n.Left, n.Right)
1410+
} else {
1411+
mode.Fprintf(s, "%#v(%.v)", n.Op, n.List)
14151412
}
14161413

14171414
case OCONV,
@@ -1540,6 +1537,8 @@ func (n *Node) nodefmt(s fmt.State, flag FmtFlag, mode fmtMode) {
15401537
if flag&FmtLong != 0 && t != nil {
15411538
if t.Etype == TNIL {
15421539
fmt.Fprint(s, "nil")
1540+
} else if n.Op == ONAME && n.Name.AutoTemp() {
1541+
mode.Fprintf(s, "%v value", t)
15431542
} else {
15441543
mode.Fprintf(s, "%v (type %v)", n, t)
15451544
}

src/cmd/compile/internal/gc/iexport.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1277,6 +1277,7 @@ func (w *exportWriter) expr(n *Node) {
12771277
case OCALL, OCALLFUNC, OCALLMETH, OCALLINTER, OGETG:
12781278
w.op(OCALL)
12791279
w.pos(n.Pos)
1280+
w.stmtList(n.Ninit)
12801281
w.expr(n.Left)
12811282
w.exprList(n.List)
12821283
w.bool(n.IsDDD())
@@ -1387,7 +1388,8 @@ func (w *exportWriter) localIdent(s *types.Sym, v int32) {
13871388
return
13881389
}
13891390

1390-
if i := strings.LastIndex(name, "."); i >= 0 {
1391+
// TODO(mdempsky): Fix autotmp hack.
1392+
if i := strings.LastIndex(name, "."); i >= 0 && !strings.HasPrefix(name, ".autotmp_") {
13911393
Fatalf("unexpected dot in identifier: %v", name)
13921394
}
13931395

src/cmd/compile/internal/gc/iimport.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -907,7 +907,9 @@ func (r *importReader) node() *Node {
907907
// unreachable - mapped to OCALL case below by exporter
908908

909909
case OCALL:
910-
n := nodl(r.pos(), OCALL, r.expr(), nil)
910+
n := nodl(r.pos(), OCALL, nil, nil)
911+
n.Ninit.Set(r.stmtList())
912+
n.Left = r.expr()
911913
n.List.Set(r.exprList())
912914
n.SetIsDDD(r.bool())
913915
return n

src/cmd/compile/internal/gc/init.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ import (
1414
// the name, normally "pkg.init", is altered to "pkg.init.0".
1515
var renameinitgen int
1616

17+
// Dummy function for autotmps generated during typechecking.
18+
var dummyInitFn = nod(ODCLFUNC, nil, nil)
19+
1720
func renameinit() *types.Sym {
1821
s := lookupN("init.", renameinitgen)
1922
renameinitgen++
@@ -93,6 +96,12 @@ func fninit(n []*Node) {
9396
initializers = lookup("init.ializers")
9497
disableExport(initializers)
9598
fn := dclfunc(initializers, nod(OTFUNC, nil, nil))
99+
for _, dcl := range dummyInitFn.Func.Dcl {
100+
dcl.Name.Curfn = fn
101+
}
102+
fn.Func.Dcl = append(fn.Func.Dcl, dummyInitFn.Func.Dcl...)
103+
dummyInitFn.Func.Dcl = nil
104+
96105
fn.Nbody.Set(nf)
97106
funcbody()
98107

@@ -103,6 +112,12 @@ func fninit(n []*Node) {
103112
funccompile(fn)
104113
lineno = autogeneratedPos
105114
}
115+
if dummyInitFn.Func.Dcl != nil {
116+
// We only generate temps using dummyInitFn if there
117+
// are package-scope initialization statements, so
118+
// something's weird if we get here.
119+
Fatalf("dummyInitFn still has declarations")
120+
}
106121

107122
var r []*Node
108123

src/cmd/compile/internal/gc/inl.go

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -589,24 +589,13 @@ func inlnode(n *Node, maxCost int32) *Node {
589589
}
590590

591591
inlnodelist(n.List, maxCost)
592-
switch n.Op {
593-
case OBLOCK:
592+
if n.Op == OBLOCK {
594593
for _, n2 := range n.List.Slice() {
595594
if n2.Op == OINLCALL {
596595
inlconv2stmt(n2)
597596
}
598597
}
599-
600-
case ORETURN, OCALLFUNC, OCALLMETH, OCALLINTER, OAPPEND, OCOMPLEX:
601-
// if we just replaced arg in f(arg()) or return arg with an inlined call
602-
// and arg returns multiple values, glue as list
603-
if n.List.Len() == 1 && n.List.First().Op == OINLCALL && n.List.First().Rlist.Len() > 1 {
604-
n.List.Set(inlconv2list(n.List.First()))
605-
break
606-
}
607-
fallthrough
608-
609-
default:
598+
} else {
610599
s := n.List.Slice()
611600
for i1, n1 := range s {
612601
if n1 != nil && n1.Op == OINLCALL {
@@ -1016,9 +1005,6 @@ func mkinlcall(n, fn *Node, maxCost int32) *Node {
10161005
// to pass as a slice.
10171006

10181007
numvals := n.List.Len()
1019-
if numvals == 1 && n.List.First().Type.IsFuncArgStruct() {
1020-
numvals = n.List.First().Type.NumFields()
1021-
}
10221008

10231009
x := as.List.Len()
10241010
for as.List.Len() < numvals {

src/cmd/compile/internal/gc/order.go

Lines changed: 3 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -380,66 +380,12 @@ func (o *Order) init(n *Node) {
380380
n.Ninit.Set(nil)
381381
}
382382

383-
// Ismulticall reports whether the list l is f() for a multi-value function.
384-
// Such an f() could appear as the lone argument to a multi-arg function.
385-
func ismulticall(l Nodes) bool {
386-
// one arg only
387-
if l.Len() != 1 {
388-
return false
389-
}
390-
n := l.First()
391-
392-
// must be call
393-
switch n.Op {
394-
default:
395-
return false
396-
case OCALLFUNC, OCALLMETH, OCALLINTER:
397-
// call must return multiple values
398-
return n.Left.Type.NumResults() > 1
399-
}
400-
}
401-
402-
// copyRet emits t1, t2, ... = n, where n is a function call,
403-
// and then returns the list t1, t2, ....
404-
func (o *Order) copyRet(n *Node) []*Node {
405-
if !n.Type.IsFuncArgStruct() {
406-
Fatalf("copyret %v %d", n.Type, n.Left.Type.NumResults())
407-
}
408-
409-
slice := n.Type.Fields().Slice()
410-
l1 := make([]*Node, len(slice))
411-
l2 := make([]*Node, len(slice))
412-
for i, t := range slice {
413-
tmp := temp(t.Type)
414-
l1[i] = tmp
415-
l2[i] = tmp
416-
}
417-
418-
as := nod(OAS2, nil, nil)
419-
as.List.Set(l1)
420-
as.Rlist.Set1(n)
421-
as = typecheck(as, ctxStmt)
422-
o.stmt(as)
423-
424-
return l2
425-
}
426-
427-
// callArgs orders the list of call arguments *l.
428-
func (o *Order) callArgs(l *Nodes) {
429-
if ismulticall(*l) {
430-
// return f() where f() is multiple values.
431-
l.Set(o.copyRet(l.First()))
432-
} else {
433-
o.exprList(*l)
434-
}
435-
}
436-
437383
// call orders the call expression n.
438384
// n.Op is OCALLMETH/OCALLFUNC/OCALLINTER or a builtin like OCOPY.
439385
func (o *Order) call(n *Node) {
440386
n.Left = o.expr(n.Left, nil)
441387
n.Right = o.expr(n.Right, nil) // ODDDARG temp
442-
o.callArgs(&n.List)
388+
o.exprList(n.List)
443389

444390
if n.Op != OCALLFUNC {
445391
return
@@ -811,7 +757,7 @@ func (o *Order) stmt(n *Node) {
811757
o.cleanTemp(t)
812758

813759
case ORETURN:
814-
o.callArgs(&n.List)
760+
o.exprList(n.List)
815761
o.out = append(o.out, n)
816762

817763
// Special: clean case temporaries in each block entry.
@@ -1200,7 +1146,7 @@ func (o *Order) expr(n, lhs *Node) *Node {
12001146
n.List.SetFirst(o.expr(n.List.First(), nil)) // order x
12011147
n.List.Second().Left = o.expr(n.List.Second().Left, nil) // order y
12021148
} else {
1203-
o.callArgs(&n.List)
1149+
o.exprList(n.List)
12041150
}
12051151

12061152
if lhs == nil || lhs.Op != ONAME && !samesafeexpr(lhs, n.List.First()) {

0 commit comments

Comments
 (0)