From 9094e3ada2de3cc8129b70730c2c0782a4040201 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Mon, 4 Jan 2016 13:34:54 -0800 Subject: [PATCH] [dev.ssa] cmd/compile: fix spill sizes In code that does: var x, z int32 var y int64 z = phi(x, int32(y)) We silently drop the int32 cast because truncation is a no-op. The phi operation needs to make sure it uses the size of the phi, not the size of its arguments, when generating spills. Change-Id: I1f7baf44f019256977a46fdd3dad1972be209042 Reviewed-on: https://go-review.googlesource.com/18390 Reviewed-by: David Chase --- src/cmd/compile/internal/gc/ssa.go | 3 + src/cmd/compile/internal/gc/ssa_test.go | 2 + .../compile/internal/gc/testdata/phi_ssa.go | 103 ++++++++++++++++++ src/cmd/compile/internal/ssa/regalloc.go | 9 +- 4 files changed, 114 insertions(+), 3 deletions(-) create mode 100644 src/cmd/compile/internal/gc/testdata/phi_ssa.go diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go index 55ab8ce283..eee3051c39 100644 --- a/src/cmd/compile/internal/gc/ssa.go +++ b/src/cmd/compile/internal/gc/ssa.go @@ -4536,6 +4536,9 @@ func regnum(v *ssa.Value) int16 { // where v should be spilled. func autoVar(v *ssa.Value) (*Node, int64) { loc := v.Block.Func.RegAlloc[v.ID].(ssa.LocalSlot) + if v.Type.Size() > loc.Type.Size() { + v.Fatalf("spill/restore type %s doesn't fit in slot type %s", v.Type, loc.Type) + } return loc.N.(*Node), loc.Off } diff --git a/src/cmd/compile/internal/gc/ssa_test.go b/src/cmd/compile/internal/gc/ssa_test.go index 74fa847c92..d0c44b5dce 100644 --- a/src/cmd/compile/internal/gc/ssa_test.go +++ b/src/cmd/compile/internal/gc/ssa_test.go @@ -95,3 +95,5 @@ func TestAddressed(t *testing.T) { runTest(t, "addressed_ssa.go") } func TestCopy(t *testing.T) { runTest(t, "copy_ssa.go") } func TestUnsafe(t *testing.T) { runTest(t, "unsafe_ssa.go") } + +func TestPhi(t *testing.T) { runTest(t, "phi_ssa.go") } diff --git a/src/cmd/compile/internal/gc/testdata/phi_ssa.go b/src/cmd/compile/internal/gc/testdata/phi_ssa.go new file mode 100644 index 0000000000..e855070fc3 --- /dev/null +++ b/src/cmd/compile/internal/gc/testdata/phi_ssa.go @@ -0,0 +1,103 @@ +// Copyright 2016 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. + +package main + +// Test to make sure spills of cast-shortened values +// don't end up spilling the pre-shortened size instead +// of the post-shortened size. + +import ( + "fmt" + "runtime" +) + +// unfoldable true +var true_ = true + +var data1 [26]int32 +var data2 [26]int64 + +func init() { + for i := 0; i < 26; i++ { + // If we spill all 8 bytes of this datum, the 1 in the high-order 4 bytes + // will overwrite some other variable in the stack frame. + data2[i] = 0x100000000 + } +} + +func foo() int32 { + var a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w, x, y, z int32 + if true_ { + a = data1[0] + b = data1[1] + c = data1[2] + d = data1[3] + e = data1[4] + f = data1[5] + g = data1[6] + h = data1[7] + i = data1[8] + j = data1[9] + k = data1[10] + l = data1[11] + m = data1[12] + n = data1[13] + o = data1[14] + p = data1[15] + q = data1[16] + r = data1[17] + s = data1[18] + t = data1[19] + u = data1[20] + v = data1[21] + w = data1[22] + x = data1[23] + y = data1[24] + z = data1[25] + } else { + a = int32(data2[0]) + b = int32(data2[1]) + c = int32(data2[2]) + d = int32(data2[3]) + e = int32(data2[4]) + f = int32(data2[5]) + g = int32(data2[6]) + h = int32(data2[7]) + i = int32(data2[8]) + j = int32(data2[9]) + k = int32(data2[10]) + l = int32(data2[11]) + m = int32(data2[12]) + n = int32(data2[13]) + o = int32(data2[14]) + p = int32(data2[15]) + q = int32(data2[16]) + r = int32(data2[17]) + s = int32(data2[18]) + t = int32(data2[19]) + u = int32(data2[20]) + v = int32(data2[21]) + w = int32(data2[22]) + x = int32(data2[23]) + y = int32(data2[24]) + z = int32(data2[25]) + } + // Lots of phis of the form phi(int32,int64) of type int32 happen here. + // Some will be stack phis. For those stack phis, make sure the spill + // of the second argument uses the phi's width (4 bytes), not its width + // (8 bytes). Otherwise, a random stack slot gets clobbered. + + runtime.Gosched() + return a + b + c + d + e + f + g + h + i + j + k + l + m + n + o + p + q + r + s + t + u + v + w + x + y + z +} + +func main() { + want := int32(0) + got := foo() + if got != want { + fmt.Printf("want %d, got %d\n", want, got) + panic("bad") + } +} diff --git a/src/cmd/compile/internal/ssa/regalloc.go b/src/cmd/compile/internal/ssa/regalloc.go index d7c4674cfd..27deeba718 100644 --- a/src/cmd/compile/internal/ssa/regalloc.go +++ b/src/cmd/compile/internal/ssa/regalloc.go @@ -1223,7 +1223,10 @@ func (e *edgeState) processDest(loc Location, vid ID, splice **Value) bool { r := e.findRegFor(v.Type) x = v.copyInto(e.p) e.set(r, vid, x, false) - x = e.p.NewValue1(x.Line, OpStoreReg, x.Type, x) + // Make sure we spill with the size of the slot, not the + // size of x (which might be wider due to our dropping + // of narrowing conversions). + x = e.p.NewValue1(x.Line, OpStoreReg, loc.(LocalSlot).Type, x) } } else { // Emit move from src to dst. @@ -1232,7 +1235,7 @@ func (e *edgeState) processDest(loc Location, vid ID, splice **Value) bool { if dstReg { x = e.p.NewValue1(c.Line, OpCopy, c.Type, c) } else { - x = e.p.NewValue1(c.Line, OpStoreReg, c.Type, c) + x = e.p.NewValue1(c.Line, OpStoreReg, loc.(LocalSlot).Type, c) } } else { if dstReg { @@ -1255,7 +1258,7 @@ func (e *edgeState) processDest(loc Location, vid ID, splice **Value) bool { r := e.findRegFor(c.Type) t := e.p.NewValue1(c.Line, OpLoadReg, c.Type, c) e.set(r, vid, t, false) - x = e.p.NewValue1(c.Line, OpStoreReg, c.Type, t) + x = e.p.NewValue1(c.Line, OpStoreReg, loc.(LocalSlot).Type, t) } } }