[dev.regabi] cmd/compile: cleanup OAS2FUNC ordering

Currently, to ensure OAS2FUNC results are assigned in the correct
order, they're always assigned to temporary variables. However, these
temporary variables are typed based on the destination type, which may
require an interface conversion. This means walk may have to then
introduce a second set of temporaries to ensure result parameters are
all copied out of the results area, before it emits calls to runtime
conversion functions.

That's just silly. Instead, this CL changes order to allocate the
result temporaries with the same type as the function returns in the
first place, and then assign them one at a time to their destinations,
with conversions as needed.

While here, also fix an order-of-evaluation issue with has-ok
assignments that I almost added to multi-value function call
assignments, and add tests for each.

Change-Id: I9f4e962425fe3c5e3305adbbfeae2c7f253ec365
Reviewed-on: https://go-review.googlesource.com/c/go/+/284220
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>

src/cmd/compile/internal/walk/assign.go

@@ -268,7 +268,7 @@ func ascompatet(nl ir.Nodes, nr *types.Type) []ir.Node {
 		base.Fatalf("ascompatet: assignment count mismatch: %d = %d", len(nl), nr.NumFields())
 	}
 
-	var nn, mm ir.Nodes
+	var nn ir.Nodes
 	for i, l := range nl {
 		if ir.IsBlank(l) {
 			continue
@@ -278,11 +278,7 @@ func ascompatet(nl ir.Nodes, nr *types.Type) []ir.Node {
 		// Any assignment to an lvalue that might cause a function call must be
 		// deferred until all the returned values have been read.
 		if fncall(l, r.Type) {
-			tmp := ir.Node(typecheck.Temp(r.Type))
+			base.FatalfAt(l.Pos(), "assigning %v to %+v", r.Type, l)
-			tmp = typecheck.Expr(tmp)
-			a := convas(ir.NewAssignStmt(base.Pos, l, tmp), &mm)
-			mm.Append(a)
-			l = tmp
 		}
 
 		res := ir.NewResultExpr(base.Pos, nil, types.BADWIDTH)
@@ -299,7 +295,7 @@ func ascompatet(nl ir.Nodes, nr *types.Type) []ir.Node {
 
 		nn.Append(a)
 	}
-	return append(nn, mm...)
+	return nn
 }
 
 // check assign expression list to

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

@@ -555,10 +555,6 @@ func (o *orderState) mapAssign(n ir.Node) {
 			n.Y = o.safeMapRHS(n.Y)
 		}
 		o.out = append(o.out, n)
-
-	case ir.OAS2, ir.OAS2DOTTYPE, ir.OAS2MAPR, ir.OAS2FUNC:
-		n := n.(*ir.AssignListStmt)
-		o.out = append(o.out, n)
 	}
 }
 
@@ -637,7 +633,7 @@ func (o *orderState) stmt(n ir.Node) {
 		t := o.markTemp()
 		o.exprList(n.Lhs)
 		o.exprList(n.Rhs)
-		o.mapAssign(n)
+		o.out = append(o.out, n)
 		o.cleanTemp(t)
 
 	// Special: avoid copy of func call n.Right
@@ -647,7 +643,7 @@ func (o *orderState) stmt(n ir.Node) {
 		o.exprList(n.Lhs)
 		o.init(n.Rhs[0])
 		o.call(n.Rhs[0])
-		o.as2(n)
+		o.as2func(n)
 		o.cleanTemp(t)
 
 	// Special: use temporary variables to hold result,
@@ -679,7 +675,7 @@ func (o *orderState) stmt(n ir.Node) {
 			base.Fatalf("order.stmt: %v", r.Op())
 		}
 
-		o.okAs2(n)
+		o.as2ok(n)
 		o.cleanTemp(t)
 
 	// Special: does not save n onto out.
@@ -1390,57 +1386,54 @@ func (o *orderState) expr1(n, lhs ir.Node) ir.Node {
 	// No return - type-assertions above. Each case must return for itself.
 }
 
-// as2 orders OAS2XXXX nodes. It creates temporaries to ensure left-to-right assignment.
+// as2func orders OAS2FUNC nodes. It creates temporaries to ensure left-to-right assignment.
-// The caller should order the right-hand side of the assignment before calling order.as2.
+// The caller should order the right-hand side of the assignment before calling order.as2func.
 // It rewrites,
 //	a, b, a = ...
 // as
 //	tmp1, tmp2, tmp3 = ...
 //	a, b, a = tmp1, tmp2, tmp3
 // This is necessary to ensure left to right assignment order.
-func (o *orderState) as2(n *ir.AssignListStmt) {
+func (o *orderState) as2func(n *ir.AssignListStmt) {
-	tmplist := []ir.Node{}
+	results := n.Rhs[0].Type()
-	left := []ir.Node{}
+	as := ir.NewAssignListStmt(n.Pos(), ir.OAS2, nil, nil)
-	for ni, l := range n.Lhs {
+	for i, nl := range n.Lhs {
-		if !ir.IsBlank(l) {
+		if !ir.IsBlank(nl) {
-			tmp := o.newTemp(l.Type(), l.Type().HasPointers())
+			typ := results.Field(i).Type
-			n.Lhs[ni] = tmp
+			tmp := o.newTemp(typ, typ.HasPointers())
-			tmplist = append(tmplist, tmp)
+			n.Lhs[i] = tmp
-			left = append(left, l)
+			as.Lhs = append(as.Lhs, nl)
+			as.Rhs = append(as.Rhs, tmp)
 		}
 	}
 
 	o.out = append(o.out, n)
-
-	as := ir.NewAssignListStmt(base.Pos, ir.OAS2, nil, nil)
-	as.Lhs = left
-	as.Rhs = tmplist
 	o.stmt(typecheck.Stmt(as))
 }
 
-// okAs2 orders OAS2XXX with ok.
+// as2ok orders OAS2XXX with ok.
-// Just like as2, this also adds temporaries to ensure left-to-right assignment.
+// Just like as2func, this also adds temporaries to ensure left-to-right assignment.
-func (o *orderState) okAs2(n *ir.AssignListStmt) {
+func (o *orderState) as2ok(n *ir.AssignListStmt) {
-	var tmp1, tmp2 ir.Node
+	as := ir.NewAssignListStmt(n.Pos(), ir.OAS2, nil, nil)
-	if !ir.IsBlank(n.Lhs[0]) {
+
-		typ := n.Rhs[0].Type()
+	do := func(i int, typ *types.Type) {
-		tmp1 = o.newTemp(typ, typ.HasPointers())
+		if nl := n.Lhs[i]; !ir.IsBlank(nl) {
+			var tmp ir.Node = o.newTemp(typ, typ.HasPointers())
+			n.Lhs[i] = tmp
+			as.Lhs = append(as.Lhs, nl)
+			if i == 1 {
+				// The "ok" result is an untyped boolean according to the Go
+				// spec. We need to explicitly convert it to the LHS type in
+				// case the latter is a defined boolean type (#8475).
+				tmp = typecheck.Conv(tmp, nl.Type())
+			}
+			as.Rhs = append(as.Rhs, tmp)
+		}
 	}
 
-	if !ir.IsBlank(n.Lhs[1]) {
+	do(0, n.Rhs[0].Type())
-		tmp2 = o.newTemp(types.Types[types.TBOOL], false)
+	do(1, types.Types[types.TBOOL])
-	}
 
 	o.out = append(o.out, n)
-
+	o.stmt(typecheck.Stmt(as))
-	if tmp1 != nil {
-		r := ir.NewAssignStmt(base.Pos, n.Lhs[0], tmp1)
-		o.mapAssign(typecheck.Stmt(r))
-		n.Lhs[0] = tmp1
-	}
-	if tmp2 != nil {
-		r := ir.NewAssignStmt(base.Pos, n.Lhs[1], typecheck.Conv(tmp2, n.Lhs[1].Type()))
-		o.mapAssign(typecheck.Stmt(r))
-		n.Lhs[1] = tmp2
-	}
 }

test/reorder.go

@@ -20,6 +20,8 @@ func main() {
 	p7()
 	p8()
 	p9()
+	p10()
+	p11()
 }
 
 var gx []int
@@ -149,3 +151,17 @@ func checkOAS2XXX(x bool, s string) {
 		panic("failed")
 	}
 }
+
+//go:noinline
+func fp() (*int, int) { return nil, 42 }
+
+func p10() {
+	p := new(int)
+	p, *p = fp()
+}
+
+func p11() {
+	var i interface{}
+	p := new(bool)
+	p, *p = i.(*bool)
+}