aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Chase <drchase@google.com>2023-08-15 17:45:50 -0400
committerCarlos Amedee <carlos@golang.org>2023-08-24 21:10:25 +0000
commit2b8026f02535d45a23445ab0e70b27ac839d0e68 (patch)
tree3fdbb36c58429f85f3c0cc7bda44122ce468e498
parent7c97cc7d979a918b8d60081794a846b50f6a7bbe (diff)
downloadgo-2b8026f02535d45a23445ab0e70b27ac839d0e68.tar.gz
go-2b8026f02535d45a23445ab0e70b27ac839d0e68.zip
[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 <khr@google.com> Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit e72ecc6a6becab9ca895c0108047db4723394296) Reviewed-on: https://go-review.googlesource.com/c/go/+/520058
-rw-r--r--src/cmd/compile/internal/ssa/expand_calls.go35
-rw-r--r--test/fixedbugs/issue61992.go26
2 files changed, 45 insertions, 16 deletions
diff --git a/src/cmd/compile/internal/ssa/expand_calls.go b/src/cmd/compile/internal/ssa/expand_calls.go
index e0b0b59992..3afd73eb6a 100644
--- a/src/cmd/compile/internal/ssa/expand_calls.go
+++ b/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)
diff --git a/test/fixedbugs/issue61992.go b/test/fixedbugs/issue61992.go
new file mode 100644
index 0000000000..d60605bb95
--- /dev/null
+++ b/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)
+}