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>
This commit is contained in:
Keith Randall 2018-01-23 13:06:21 -08:00 committed by Keith Randall
parent f27a1ff2c8
commit 7eaa8efbe4
2 changed files with 51 additions and 1 deletions

View file

@ -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)
}

View file

@ -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")
}
}