From cca23a73733ff166722c69359f0bb45e12ccaa2b Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Fri, 21 May 2021 10:53:10 -0400 Subject: [PATCH] cmd/compile: revert CL/316890 This is a revert of https://go-review.googlesource.com/c/go/+/316890, which has positive effects on debugging + DWARF variable locations for register parameters when the reg abi is in effect, but also turns out to interact badly with the register allocator. Fixes #46304. Change-Id: I624bd980493411a9cde45d44fcd3c46cad796909 Reviewed-on: https://go-review.googlesource.com/c/go/+/321830 Trust: Than McIntosh Run-TryBot: Than McIntosh Reviewed-by: Cherry Mui TryBot-Result: Go Bot --- src/cmd/compile/internal/ssa/expand_calls.go | 16 ----- test/fixedbugs/issue46304.go | 76 ++++++++++++++++++++ 2 files changed, 76 insertions(+), 16 deletions(-) create mode 100644 test/fixedbugs/issue46304.go diff --git a/src/cmd/compile/internal/ssa/expand_calls.go b/src/cmd/compile/internal/ssa/expand_calls.go index d37d06f8e71..7e973ab2059 100644 --- a/src/cmd/compile/internal/ssa/expand_calls.go +++ b/src/cmd/compile/internal/ssa/expand_calls.go @@ -1717,22 +1717,6 @@ func (x *expandState) newArgToMemOrRegs(baseArg, toReplace *Value, offset int64, } else { w = baseArg.Block.NewValue0IA(pos, op, t, auxInt, aux) } - // If we are creating an OpArgIntReg/OpArgFloatReg that - // corresponds to an in-param that fits entirely in a register, - // then enter it into the name/value table. The LocalSlot - // is somewhat fictitious, since there is no incoming live - // memory version of the parameter, but we need an entry in - // NamedValues in order for ssa debug tracking to include - // the value in the tracking analysis. - if len(pa.Registers) == 1 { - loc := LocalSlot{N: aux.Name, Type: t, Off: 0} - values, ok := x.f.NamedValues[loc] - if !ok { - ploc := x.f.localSlotAddr(loc) - x.f.Names = append(x.f.Names, ploc) - } - x.f.NamedValues[loc] = append(values, w) - } x.commonArgs[key] = w if toReplace != nil { toReplace.copyOf(w) diff --git a/test/fixedbugs/issue46304.go b/test/fixedbugs/issue46304.go new file mode 100644 index 00000000000..b8ecfc93a59 --- /dev/null +++ b/test/fixedbugs/issue46304.go @@ -0,0 +1,76 @@ +// run + +// Copyright 2021 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. + +// This testcase caused a crash when the register ABI was in effect, +// on amd64 (problem with register allocation). + +package main + +type Op struct { + tag string + _x []string + _q [20]uint64 + plist []P +} + +type P struct { + tag string + _x [10]uint64 + b bool +} + +type M int + +//go:noinline +func (w *M) walkP(p *P) *P { + np := &P{} + *np = *p + np.tag += "new" + return np +} + +func (w *M) walkOp(op *Op) *Op { + if op == nil { + return nil + } + + orig := op + cloned := false + clone := func() { + if !cloned { + cloned = true + op = &Op{} + *op = *orig + } + } + + pCloned := false + for i := range op.plist { + if s := w.walkP(&op.plist[i]); s != &op.plist[i] { + if !pCloned { + pCloned = true + clone() + op.plist = make([]P, len(orig.plist)) + copy(op.plist, orig.plist) + } + op.plist[i] = *s + } + } + + return op +} + +func main() { + var ww M + w := &ww + p1 := P{tag: "a"} + p1._x[1] = 9 + o := Op{tag: "old", plist: []P{p1}} + no := w.walkOp(&o) + if no.plist[0].tag != "anew" { + panic("bad") + } +}