[release-branch.go1.21] cmd/compile: in expandCalls, move all arg marshalling into call block

For aggregate-typed arguments passed to a call, expandCalls
decomposed them into parts in the same block where the value
was created.  This is not necessarily the call block, and in
the case where stores are involved, can change the memory
leaving that block, and getting that right is problematic.

Instead, do all the expanding in the same block as the call,
which avoids the problems of (1) not being able to reorder
loads/stores across a block boundary to conform to memory
order and (2) (incorrectly, not) exposing the new memory to
consumers in other blocks.  Putting it all in the same block
as the call allows reordering, and the call creates its own
new memory (which is already dealt with correctly).

Fixes #62057.
Updates #61992.

Change-Id: Icc7918f0d2dd3c480cc7f496cdcd78edeca7f297
Reviewed-on: https://go-review.googlesource.com/c/go/+/519276
Reviewed-by: Keith Randall <[email protected]>
Run-TryBot: David Chase <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
(cherry picked from commit e72ecc6)
Reviewed-on: https://go-review.googlesource.com/c/go/+/520058

Author: dr2chase

src/cmd/compile/internal/ssa/expand_calls.go

@@ -855,7 +855,7 @@ func storeOneArg(x *expandState, pos src.XPos, b *Block, locs []*LocalSlot, suff
 // storeOneLoad creates a decomposed (one step) load that is then stored.
 func storeOneLoad(x *expandState, pos src.XPos, b *Block, source, mem *Value, t *types.Type, offArg, offStore int64, loadRegOffset Abi1RO, storeRc registerCursor) *Value {
 	from := x.offsetFrom(source.Block, source.Args[0], offArg, types.NewPtr(t))
-	w := source.Block.NewValue2(source.Pos, OpLoad, t, from, mem)
+	w := b.NewValue2(source.Pos, OpLoad, t, from, mem)
 	return x.storeArgOrLoad(pos, b, w, mem, t, offStore, loadRegOffset, storeRc)
 }
 
@@ -962,7 +962,7 @@ func (x *expandState) storeArgOrLoad(pos src.XPos, b *Block, source, mem *Value,
 		eltRO := x.regWidth(elt)
 		source.Type = t
 		for i := int64(0); i < t.NumElem(); i++ {
-			sel := source.Block.NewValue1I(pos, OpArraySelect, elt, i, source)
+			sel := b.NewValue1I(pos, OpArraySelect, elt, i, source)
 			mem = x.storeArgOrLoad(pos, b, sel, mem, elt, storeOffset+i*elt.Size(), loadRegOffset, storeRc.at(t, 0))
 			loadRegOffset += eltRO
 			pos = pos.WithNotStmt()
@@ -997,7 +997,7 @@ func (x *expandState) storeArgOrLoad(pos src.XPos, b *Block, source, mem *Value,
 		source.Type = t
 		for i := 0; i < t.NumFields(); i++ {
 			fld := t.Field(i)
-			sel := source.Block.NewValue1I(pos, OpStructSelect, fld.Type, int64(i), source)
+			sel := b.NewValue1I(pos, OpStructSelect, fld.Type, int64(i), source)
 			mem = x.storeArgOrLoad(pos, b, sel, mem, fld.Type, storeOffset+fld.Offset, loadRegOffset, storeRc.next(fld.Type))
 			loadRegOffset += x.regWidth(fld.Type)
 			pos = pos.WithNotStmt()
@@ -1009,48 +1009,48 @@ func (x *expandState) storeArgOrLoad(pos src.XPos, b *Block, source, mem *Value,
 			break
 		}
 		tHi, tLo := x.intPairTypes(t.Kind())
-		sel := source.Block.NewValue1(pos, OpInt64Hi, tHi, source)
+		sel := b.NewValue1(pos, OpInt64Hi, tHi, source)
 		mem = x.storeArgOrLoad(pos, b, sel, mem, tHi, storeOffset+x.hiOffset, loadRegOffset+x.hiRo, storeRc.plus(x.hiRo))
 		pos = pos.WithNotStmt()
-		sel = source.Block.NewValue1(pos, OpInt64Lo, tLo, source)
+		sel = b.NewValue1(pos, OpInt64Lo, tLo, source)
 		return x.storeArgOrLoad(pos, b, sel, mem, tLo, storeOffset+x.lowOffset, loadRegOffset+x.loRo, storeRc.plus(x.hiRo))
 
 	case types.TINTER:
-		sel := source.Block.NewValue1(pos, OpITab, x.typs.BytePtr, source)
+		sel := b.NewValue1(pos, OpITab, x.typs.BytePtr, source)
 		mem = x.storeArgOrLoad(pos, b, sel, mem, x.typs.BytePtr, storeOffset, loadRegOffset, storeRc.next(x.typs.BytePtr))
 		pos = pos.WithNotStmt()
-		sel = source.Block.NewValue1(pos, OpIData, x.typs.BytePtr, source)
+		sel = b.NewValue1(pos, OpIData, x.typs.BytePtr, source)
 		return x.storeArgOrLoad(pos, b, sel, mem, x.typs.BytePtr, storeOffset+x.ptrSize, loadRegOffset+RO_iface_data, storeRc)
 
 	case types.TSTRING:
-		sel := source.Block.NewValue1(pos, OpStringPtr, x.typs.BytePtr, source)
+		sel := b.NewValue1(pos, OpStringPtr, x.typs.BytePtr, source)
 		mem = x.storeArgOrLoad(pos, b, sel, mem, x.typs.BytePtr, storeOffset, loadRegOffset, storeRc.next(x.typs.BytePtr))
 		pos = pos.WithNotStmt()
-		sel = source.Block.NewValue1(pos, OpStringLen, x.typs.Int, source)
+		sel = b.NewValue1(pos, OpStringLen, x.typs.Int, source)
 		return x.storeArgOrLoad(pos, b, sel, mem, x.typs.Int, storeOffset+x.ptrSize, loadRegOffset+RO_string_len, storeRc)
 
 	case types.TSLICE:
 		et := types.NewPtr(t.Elem())
-		sel := source.Block.NewValue1(pos, OpSlicePtr, et, source)
+		sel := b.NewValue1(pos, OpSlicePtr, et, source)
 		mem = x.storeArgOrLoad(pos, b, sel, mem, et, storeOffset, loadRegOffset, storeRc.next(et))
 		pos = pos.WithNotStmt()
-		sel = source.Block.NewValue1(pos, OpSliceLen, x.typs.Int, source)
+		sel = b.NewValue1(pos, OpSliceLen, x.typs.Int, source)
 		mem = x.storeArgOrLoad(pos, b, sel, mem, x.typs.Int, storeOffset+x.ptrSize, loadRegOffset+RO_slice_len, storeRc.next(x.typs.Int))
-		sel = source.Block.NewValue1(pos, OpSliceCap, x.typs.Int, source)
+		sel = b.NewValue1(pos, OpSliceCap, x.typs.Int, source)
 		return x.storeArgOrLoad(pos, b, sel, mem, x.typs.Int, storeOffset+2*x.ptrSize, loadRegOffset+RO_slice_cap, storeRc)
 
 	case types.TCOMPLEX64:
-		sel := source.Block.NewValue1(pos, OpComplexReal, x.typs.Float32, source)
+		sel := b.NewValue1(pos, OpComplexReal, x.typs.Float32, source)
 		mem = x.storeArgOrLoad(pos, b, sel, mem, x.typs.Float32, storeOffset, loadRegOffset, storeRc.next(x.typs.Float32))
 		pos = pos.WithNotStmt()
-		sel = source.Block.NewValue1(pos, OpComplexImag, x.typs.Float32, source)
+		sel = b.NewValue1(pos, OpComplexImag, x.typs.Float32, source)
 		return x.storeArgOrLoad(pos, b, sel, mem, x.typs.Float32, storeOffset+4, loadRegOffset+RO_complex_imag, storeRc)
 
 	case types.TCOMPLEX128:
-		sel := source.Block.NewValue1(pos, OpComplexReal, x.typs.Float64, source)
+		sel := b.NewValue1(pos, OpComplexReal, x.typs.Float64, source)
 		mem = x.storeArgOrLoad(pos, b, sel, mem, x.typs.Float64, storeOffset, loadRegOffset, storeRc.next(x.typs.Float64))
 		pos = pos.WithNotStmt()
-		sel = source.Block.NewValue1(pos, OpComplexImag, x.typs.Float64, source)
+		sel = b.NewValue1(pos, OpComplexImag, x.typs.Float64, source)
 		return x.storeArgOrLoad(pos, b, sel, mem, x.typs.Float64, storeOffset+8, loadRegOffset+RO_complex_imag, storeRc)
 	}
 
@@ -1113,6 +1113,9 @@ func (x *expandState) rewriteArgs(v *Value, firstArg int) {
 					}
 				}
 			}
+			if x.debug > 1 {
+				x.Printf("...storeArg %s, %v, %d\n", a.LongString(), aType, aOffset)
+			}
 			// "Dereference" of addressed (probably not-SSA-eligible) value becomes Move
 			// TODO(register args) this will be more complicated with registers in the picture.
 			mem = x.rewriteDereference(v.Block, sp, a, mem, aOffset, aux.SizeOfArg(auxI), aType, v.Pos)

test/fixedbugs/issue61992.go

@@ -0,0 +1,26 @@
+// compile
+
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Issue 61992, inconsistent 'mem' juggling in expandCalls
+
+package p
+
+type S1 struct {
+	a, b, c []int
+	i       int
+}
+
+type S2 struct {
+	a, b []int
+	m    map[int]int
+}
+
+func F(i int, f func(S1, S2, int) int) int {
+	return f(
+		S1{},
+		S2{m: map[int]int{}},
+		1<<i)
+}