diff --git a/src/cmd/compile/internal/walk/order.go b/src/cmd/compile/internal/walk/order.go index 4d9b2fbee56..179fbdb99e9 100644 --- a/src/cmd/compile/internal/walk/order.go +++ b/src/cmd/compile/internal/walk/order.go @@ -11,6 +11,7 @@ import ( "cmd/compile/internal/base" "cmd/compile/internal/ir" "cmd/compile/internal/reflectdata" + "cmd/compile/internal/ssa" "cmd/compile/internal/staticinit" "cmd/compile/internal/typecheck" "cmd/compile/internal/types" @@ -231,14 +232,29 @@ func (o *orderState) addrTemp(n ir.Node) ir.Node { vstat = typecheck.Expr(vstat).(*ir.Name) return vstat } + + // Prevent taking the address of an SSA-able local variable (#63332). + // + // TODO(mdempsky): Note that OuterValue unwraps OCONVNOPs, but + // IsAddressable does not. It should be possible to skip copying for + // at least some of these OCONVNOPs (e.g., reinsert them after the + // OADDR operation), but at least walkCompare needs to be fixed to + // support that (see trybot failures on go.dev/cl/541715, PS1). if ir.IsAddressable(n) { + if name, ok := ir.OuterValue(n).(*ir.Name); ok && name.Op() == ir.ONAME { + if name.Class == ir.PAUTO && !name.Addrtaken() && ssa.CanSSA(name.Type()) { + goto Copy + } + } + return n } + +Copy: return o.copyExpr(n) } // mapKeyTemp prepares n to be a key in a map runtime call and returns n. -// It should only be used for map runtime calls which have *_fast* versions. // The first parameter is the position of n's containing node, for use in case // that n's position is not unique (e.g., if n is an ONAME). func (o *orderState) mapKeyTemp(outerPos src.XPos, t *types.Type, n ir.Node) ir.Node { @@ -603,8 +619,38 @@ func (o *orderState) stmt(n ir.Node) { case ir.OAS: n := n.(*ir.AssignStmt) t := o.markTemp() + + // There's a delicate interaction here between two OINDEXMAP + // optimizations. + // + // First, we want to handle m[k] = append(m[k], ...) with a single + // runtime call to mapassign. This requires the m[k] expressions to + // satisfy ir.SameSafeExpr in walkAssign. + // + // But if k is a slow map key type that's passed by reference (e.g., + // byte), then we want to avoid marking user variables as addrtaken, + // if that might prevent the compiler from keeping k in a register. + // + // TODO(mdempsky): It would be better if walk was responsible for + // inserting temporaries as needed. + mapAppend := n.X.Op() == ir.OINDEXMAP && n.Y.Op() == ir.OAPPEND && + ir.SameSafeExpr(n.X, n.Y.(*ir.CallExpr).Args[0]) + n.X = o.expr(n.X, nil) - n.Y = o.expr(n.Y, n.X) + if mapAppend { + indexLHS := n.X.(*ir.IndexExpr) + indexLHS.X = o.cheapExpr(indexLHS.X) + indexLHS.Index = o.cheapExpr(indexLHS.Index) + + call := n.Y.(*ir.CallExpr) + indexRHS := call.Args[0].(*ir.IndexExpr) + indexRHS.X = indexLHS.X + indexRHS.Index = indexLHS.Index + + o.exprList(call.Args[1:]) + } else { + n.Y = o.expr(n.Y, n.X) + } o.mapAssign(n) o.popTemp(t) @@ -1158,7 +1204,7 @@ func (o *orderState) expr1(n, lhs ir.Node) ir.Node { } } - // key must be addressable + // key may need to be be addressable n.Index = o.mapKeyTemp(n.Pos(), n.X.Type(), n.Index) if needCopy { return o.copyExpr(n) diff --git a/test/codegen/issue63332.go b/test/codegen/issue63332.go new file mode 100644 index 00000000000..dbe671d247e --- /dev/null +++ b/test/codegen/issue63332.go @@ -0,0 +1,14 @@ +// asmcheck + +// 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. + +package codegen + +func issue63332(c chan int) { + x := 0 + // amd64:-`MOVQ` + x += 2 + c <- x +}