cmd/compile: don't let spills clobber arguments

The compiler allows code to have multiple differently-typed views of a
single argument. For instance, if we have

func f(x float64) {
   y := *(*int64)(unsafe.Pointer(&x))
   ...
}

Then in SSA we get two OpArg ops, one with float64 type and one with
int64 type.

The compiler will try to reuse argument slots for spill slots. It
checks that the argument slot is dead by consulting an interference
graph.

When building the interference graph, we normally ignore cross-type
edges because the values on either end of that edge can't be allocated
to the same slot. (This is just a space-saving optimization.) This
rule breaks down when one of the values is an argument, because of the
multiple views described above. If we're spilling a float64, it is not
enough that the float64 version of x is dead; the int64 version of x
has to be dead also.

Remove the optimization of not recording interference edges if types
don't match. That optimization is incorrect if one of the values
connected by the edge is an argument.

Fixes #23522

Change-Id: I361f85d80fe3bc7249014ca2c3ec887c3dc30271
Reviewed-on: https://go-review.googlesource.com/89335
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

2 files changed, 51 insertions, 1 deletions

diff --git a/src/cmd/compile/internal/ssa/stackalloc.go b/src/cmd/compile/internal/ssa/stackalloc.go
index ca7f95dee1..aadc6c4cfe 100644
--- a/src/cmd/compile/internal/ssa/stackalloc.go
+++ b/src/cmd/compile/internal/ssa/stackalloc.go
@@ -72,6 +72,7 @@ type stackValState struct {
 	typ      *types.Type
 	spill    *Value
 	needSlot bool
+	isArg    bool
 }
 
 // stackalloc allocates storage in the stack frame for
@@ -110,6 +111,7 @@ func (s *stackAllocState) init(f *Func, spillLive [][]ID) {
 		for _, v := range b.Values {
 			s.values[v.ID].typ = v.Type
 			s.values[v.ID].needSlot = !v.Type.IsMemory() && !v.Type.IsVoid() && !v.Type.IsFlags() && f.getHome(v.ID) == nil && !v.rematerializeable()
+			s.values[v.ID].isArg = v.Op == OpArg
 			if f.pass.debug > stackDebug && s.values[v.ID].needSlot {
 				fmt.Printf("%s needs a stack slot\n", v)
 			}
@@ -377,7 +379,9 @@ func (s *stackAllocState) buildInterferenceGraph() {
 			if s.values[v.ID].needSlot {
 				live.remove(v.ID)
 				for _, id := range live.contents() {
-					if s.values[v.ID].typ.Compare(s.values[id].typ) == types.CMPeq {
+					// Note: args can have different types and still interfere
+					// (with each other or with other values). See issue 23522.
+					if s.values[v.ID].typ.Compare(s.values[id].typ) == types.CMPeq || v.Op == OpArg || s.values[id].isArg {
 						s.interfere[v.ID] = append(s.interfere[v.ID], id)
 						s.interfere[id] = append(s.interfere[id], v.ID)
 					}
diff --git a/test/fixedbugs/issue23522.go b/test/fixedbugs/issue23522.go
new file mode 100644
index 0000000000..cace86c8ab
--- /dev/null
+++ b/test/fixedbugs/issue23522.go
@@ -0,0 +1,46 @@
+// run
+
+// Copyright 2018 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
+
+import (
+	"math"
+)
+
+type S struct {
+	u int64
+	n int32
+}
+
+func F1(f float64) *S {
+	s := f
+	pf := math.Copysign(f, 1)
+	u := math.Floor(pf)
+	return &S{
+		u: int64(math.Copysign(u, s)),
+		n: int32(math.Copysign((pf-u)*1e9, s)),
+	}
+}
+
+func F2(f float64) *S {
+	s := f
+	f = math.Copysign(f, 1)
+	u := math.Floor(f)
+	return &S{
+		u: int64(math.Copysign(u, s)),
+		n: int32(math.Copysign((f-u)*1e9, s)),
+	}
+}
+
+func main() {
+	s1 := F1(-1)
+	s2 := F2(-1)
+	if *s1 != *s2 {
+		println("F1:", s1.u, s1.n)
+		println("F2:", s2.u, s2.n)
+		panic("different")
+	}
+}