cmd/compile: add "deadlocals" pass to remove unused locals

This CL adds a "deadlocals" pass, which runs after inlining and before
escape analysis, to prune any unneeded local variables and
assignments. In particular, this helps avoid unnecessary Addrtaken
markings from unreachable closures.

Deadlocals is sensitive to "_ = ..." as a signal of explicit
use for testing.  This signal occurs only if the entire
left-hand-side is "_" targets; if it is
  `_, ok := someInlinedFunc(args)`
then the first return value is eligible for dead code elimination.

Use this (`_ = x`) to fix tests broken by deadlocals elimination.

Includes a test, based on one of the tests that required modification.

Matthew Dempsky wrote this, changing ownership to allow rebases, commits, tweaks.

Fixes #65158.

Old-Change-Id: I723fb69ccd7baadaae04d415702ce6c8901eaf4e
Change-Id: I1f25f4293b19527f305c18c3680b214237a7714c
Reviewed-on: https://go-review.googlesource.com/c/go/+/600498
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: David Chase <drchase@google.com>
Commit-Queue: David Chase <drchase@google.com>

src/cmd/compile/internal/base/debug.go

@@ -48,6 +48,7 @@ type DebugFlags struct {
 	MergeLocalsTrace      int    `help:"trace debug output for locals merging"`
 	MergeLocalsHTrace     int    `help:"hash-selected trace debug output for locals merging"`
 	Nil                   int    `help:"print information about nil checks"`
+	NoDeadLocals          int    `help:"disable deadlocals pass" concurrent:"ok"`
 	NoOpenDefer           int    `help:"disable open-coded defers" concurrent:"ok"`
 	NoRefName             int    `help:"do not include referenced symbol names in object file" concurrent:"ok"`
 	PCTab                 string `help:"print named pc-value table\nOne of: pctospadj, pctofile, pctoline, pctoinline, pctopcdata"`

src/cmd/compile/internal/deadlocals/deadlocals.go

@@ -0,0 +1,187 @@
+// Copyright 2024 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.
+
+// The deadlocals pass removes assignments to unused local variables.
+package deadlocals
+
+import (
+	"cmd/compile/internal/base"
+	"cmd/compile/internal/ir"
+	"cmd/compile/internal/types"
+	"cmd/internal/src"
+	"fmt"
+	"go/constant"
+)
+
+// Funcs applies the deadlocals pass to fns.
+func Funcs(fns []*ir.Func) {
+	if base.Flag.N != 0 || base.Debug.NoDeadLocals != 0 {
+		return
+	}
+
+	zero := ir.NewBasicLit(base.AutogeneratedPos, types.Types[types.TINT], constant.MakeInt64(0))
+
+	for _, fn := range fns {
+		if fn.IsClosure() {
+			continue
+		}
+
+		v := newVisitor(fn)
+		v.nodes(fn.Body)
+
+		for _, assigns := range v.defs {
+			for _, as := range assigns {
+				// Kludge for "missing func info" linker panic.
+				// See also closureInitLSym in inline/inl.go.
+				if clo, ok := (*as.rhs).(*ir.ClosureExpr); ok && clo.Op() == ir.OCLOSURE {
+					if !ir.IsTrivialClosure(clo) {
+						ir.InitLSym(clo.Func, true)
+					}
+				}
+
+				*as.lhs = ir.BlankNode
+				*as.rhs = zero
+			}
+		}
+	}
+}
+
+type visitor struct {
+	curfn *ir.Func
+	// defs[name] contains assignments that can be discarded if name can be discarded.
+	// if defs[name] is defined nil, then name is actually used.
+	defs map[*ir.Name][]assign
+
+	doNode func(ir.Node) bool
+}
+
+type assign struct {
+	pos      src.XPos
+	lhs, rhs *ir.Node
+}
+
+func newVisitor(fn *ir.Func) *visitor {
+	v := &visitor{
+		curfn: fn,
+		defs:  make(map[*ir.Name][]assign),
+	}
+	v.doNode = func(n ir.Node) bool {
+		v.node(n)
+		return false
+	}
+	return v
+}
+
+func (v *visitor) node(n ir.Node) {
+	if n == nil {
+		return
+	}
+
+	switch n.Op() {
+	default:
+		ir.DoChildrenWithHidden(n, v.doNode)
+	case ir.OCLOSURE:
+		n := n.(*ir.ClosureExpr)
+		v.nodes(n.Init())
+		for _, cv := range n.Func.ClosureVars {
+			v.node(cv)
+		}
+		v.nodes(n.Func.Body)
+
+	case ir.ODCL:
+		// ignore
+	case ir.ONAME:
+		n := n.(*ir.Name)
+		n = n.Canonical()
+		if isLocal(n, false) {
+			// Force any lazy definitions.
+			s := v.defs[n]
+			v.defs[n] = nil
+
+			for _, as := range s {
+				// do the visit that was skipped in v.assign when as was appended to v.defs[n]
+				v.node(*as.rhs)
+			}
+		}
+
+	case ir.OAS:
+		n := n.(*ir.AssignStmt)
+		v.assign(n.Pos(), &n.X, &n.Y, false)
+	case ir.OAS2:
+		n := n.(*ir.AssignListStmt)
+
+		// If all LHS vars are blank, treat them as intentional
+		// uses of corresponding RHS vars.  If any are non-blank
+		// then any blanks are discards.
+		hasNonBlank := false
+		for i := range n.Lhs {
+			if !ir.IsBlank(n.Lhs[i]) {
+				hasNonBlank = true
+				break
+			}
+		}
+		for i := range n.Lhs {
+			v.assign(n.Pos(), &n.Lhs[i], &n.Rhs[i], hasNonBlank)
+		}
+	}
+}
+
+func (v *visitor) nodes(list ir.Nodes) {
+	for _, n := range list {
+		v.node(n)
+	}
+}
+
+func hasEffects(n ir.Node) bool {
+	if n == nil {
+		return false
+	}
+	if len(n.Init()) != 0 {
+		return true
+	}
+
+	switch n.Op() {
+	// TODO(mdempsky): More.
+	case ir.ONAME, ir.OLITERAL, ir.ONIL, ir.OCLOSURE:
+		return false
+	}
+	return true
+}
+
+func (v *visitor) assign(pos src.XPos, lhs, rhs *ir.Node, blankIsNotUse bool) {
+	name, ok := (*lhs).(*ir.Name)
+	if !ok {
+		v.node(*lhs) // XXX: Interpret as variable, not value.
+		v.node(*rhs)
+		return
+	}
+	name = name.Canonical()
+
+	if isLocal(name, blankIsNotUse) && !hasEffects(*rhs) {
+		if s, ok := v.defs[name]; !ok || s != nil {
+			// !ok || s != nil is FALSE if previously "v.defs[name] = nil" -- that marks a use.
+			v.defs[name] = append(s, assign{pos, lhs, rhs})
+			return // don't visit rhs unless that node ends up live, later.
+		}
+	}
+
+	v.node(*rhs)
+}
+
+func isLocal(n *ir.Name, blankIsNotUse bool) bool {
+	if ir.IsBlank(n) {
+		// Treat single assignments as intentional use (false), anything else is a discard (true).
+		return blankIsNotUse
+	}
+
+	switch n.Class {
+	case ir.PAUTO, ir.PPARAM:
+		return true
+	case ir.PPARAMOUT:
+		return false
+	case ir.PEXTERN, ir.PFUNC:
+		return false
+	}
+	panic(fmt.Sprintf("unexpected Class: %+v", n))
+}

src/cmd/compile/internal/gc/main.go

@@ -9,6 +9,7 @@ import (
 	"bytes"
 	"cmd/compile/internal/base"
 	"cmd/compile/internal/coverage"
+	"cmd/compile/internal/deadlocals"
 	"cmd/compile/internal/dwarfgen"
 	"cmd/compile/internal/escape"
 	"cmd/compile/internal/inline"
@@ -247,6 +248,8 @@ func Main(archInit func(*ssagen.ArchInfo)) {
 	// and doesn't benefit from dead-coding or inlining.
 	symABIs.GenABIWrappers()
 
+	deadlocals.Funcs(typecheck.Target.Funcs)
+
 	// Escape analysis.
 	// Required for moving heap allocations onto stack,
 	// which in turn is required by the closure implementation,

src/runtime/race/testdata/mop_test.go

@@ -612,6 +612,8 @@ func TestNoRaceEnoughRegisters(t *testing.T) {
 }
 
 // emptyFunc should not be inlined.
+//
+//go:noinline
 func emptyFunc(x int) {
 	if false {
 		fmt.Println(x)
@@ -1176,7 +1178,7 @@ func TestNoRaceHeapReallocation(t *testing.T) {
 	// others.
 	const n = 2
 	done := make(chan bool, n)
-	empty := func(p *int) {}
+	empty := func(p *int) { _ = p }
 	for i := 0; i < n; i++ {
 		ms := i
 		go func() {
@@ -1417,7 +1419,7 @@ func TestRaceInterCall2(t *testing.T) {
 
 func TestRaceFuncCall(t *testing.T) {
 	c := make(chan bool, 1)
-	f := func(x, y int) {}
+	f := func(x, y int) { _ = y }
 	x, y := 0, 0
 	go func() {
 		y = 42
@@ -1804,6 +1806,7 @@ func TestRaceAsFunc2(t *testing.T) {
 	x := 0
 	go func() {
 		func(x int) {
+			_ = x
 		}(x)
 		c <- true
 	}()
@@ -1817,6 +1820,7 @@ func TestRaceAsFunc3(t *testing.T) {
 	x := 0
 	go func() {
 		func(x int) {
+			_ = x
 			mu.Lock()
 		}(x) // Read of x must be outside of the mutex.
 		mu.Unlock()

test/closure3.dir/main.go

@@ -20,6 +20,7 @@ func main() {
 		if x := func() int { // ERROR "can inline main.func2" "func literal does not escape"
 			return 1
 		}; x() != 1 { // ERROR "inlining call to main.func2"
+			_ = x // prevent simple deadcode elimination after inlining
 			ppanic("x() != 1")
 		}
 	}
@@ -33,6 +34,7 @@ func main() {
 		if y := func(x int) int { // ERROR "can inline main.func4" "func literal does not escape"
 			return x + 2
 		}; y(40) != 42 { // ERROR "inlining call to main.func4"
+			_ = y // prevent simple deadcode elimination after inlining
 			ppanic("y(40) != 42")
 		}
 	}
@@ -181,6 +183,7 @@ func main() {
 		if y := func() int { // ERROR "can inline main.func21" "func literal does not escape"
 			return x
 		}; y() != 42 { // ERROR "inlining call to main.func21"
+			_ = y // prevent simple deadcode elimination after inlining
 			ppanic("y() != 42")
 		}
 	}
@@ -199,6 +202,7 @@ func main() {
 				return x + y
 			}() // ERROR "inlining call to main.func23.1"
 		}; z(1) != 43 { // ERROR "inlining call to main.func23" "inlining call to main.main.func23.func31"
+			_ = z // prevent simple deadcode elimination after inlining
 			ppanic("z(1) != 43")
 		}
 	}
@@ -287,6 +291,25 @@ func main() {
 	}
 }
 
+//go:noinline
+func notmain() {
+	{
+		// This duplicates the first block in main, but without the "_ = x" for closure x.
+		// This allows dead code elimination of x before escape analysis,
+		// thus "func literal does not escape" should not appear.
+		if x := func() int { // ERROR "can inline notmain.func1"
+			return 1
+		}(); x != 1 { // ERROR "inlining call to notmain.func1"
+			ppanic("x != 1")
+		}
+		if x := func() int { // ERROR "can inline notmain.func2"
+			return 1
+		}; x() != 1 { // ERROR "inlining call to notmain.func2"
+			ppanic("x() != 1")
+		}
+	}
+}
+
 //go:noinline
 func ppanic(s string) { // ERROR "leaking param: s"
 	panic(s) // ERROR "s escapes to heap"

test/fixedbugs/issue54159.go

@@ -11,6 +11,7 @@ func run() { // ERROR "cannot inline run: recursive"
 		g() // ERROR "inlining call to g"
 	}
 	f() // ERROR "inlining call to run.func1" "inlining call to g"
+	_ = f
 	run()
 }
 

test/inline.go

@@ -73,6 +73,7 @@ func l(x, y int) (int, int, error) { // ERROR "can inline l"
 		f := e
 		f(nil) // ERROR "inlining call to l.func1"
 	}
+	_ = e // prevent simple deadcode elimination after inlining
 	return y, x, nil
 }
 
@@ -109,6 +110,7 @@ func p() int { // ERROR "can inline p"
 
 func q(x int) int { // ERROR "can inline q"
 	foo := func() int { return x * 2 } // ERROR "can inline q.func1" "func literal does not escape"
+	_ = foo                            // prevent simple deadcode elimination after inlining
 	return foo()                       // ERROR "inlining call to q.func1"
 }
 
@@ -121,6 +123,8 @@ func r(z int) int {
 			return 2*y + x*z
 		}(x) // ERROR "inlining call to r.func2.1"
 	}
+	_, _ = foo, bar // prevent simple deadcode elimination after inlining
+
 	return foo(42) + bar(42) // ERROR "inlining call to r.func1" "inlining call to r.func2" "inlining call to r.r.func2.func3"
 }
 
@@ -128,7 +132,8 @@ func s0(x int) int { // ERROR "can inline s0"
 	foo := func() { // ERROR "can inline s0.func1" "func literal does not escape"
 		x = x + 1
 	}
-	foo() // ERROR "inlining call to s0.func1"
+	foo()   // ERROR "inlining call to s0.func1"
+	_ = foo // prevent simple deadcode elimination after inlining
 	return x
 }
 
@@ -137,6 +142,7 @@ func s1(x int) int { // ERROR "can inline s1"
 		return x
 	}
 	x = x + 1
+	_ = foo      // prevent simple deadcode elimination after inlining
 	return foo() // ERROR "inlining call to s1.func1"
 }
 

test/newinline.go

@@ -73,6 +73,7 @@ func l(x, y int) (int, int, error) { // ERROR "can inline l"
 		f := e
 		f(nil) // ERROR "inlining call to l.func1"
 	}
+	_ = e // prevent simple deadcode elimination
 	return y, x, nil
 }
 
@@ -109,6 +110,7 @@ func p() int { // ERROR "can inline p"
 
 func q(x int) int { // ERROR "can inline q"
 	foo := func() int { return x * 2 } // ERROR "can inline q.func1" "func literal does not escape"
+	_ = foo                            // prevent simple deadcode elimination
 	return foo()                       // ERROR "inlining call to q.func1"
 }
 
@@ -121,6 +123,8 @@ func r(z int) int { // ERROR "can inline r"
 			return 2*y + x*z
 		}(x) // ERROR "inlining call to r.func2.1"
 	}
+	_ = foo                  // prevent simple deadcode elimination
+	_ = bar                  // prevent simple deadcode elimination
 	return foo(42) + bar(42) // ERROR "inlining call to r.func1" "inlining call to r.func2" "inlining call to r.r.func2.func3"
 }
 
@@ -128,7 +132,8 @@ func s0(x int) int { // ERROR "can inline s0"
 	foo := func() { // ERROR "can inline s0.func1" "func literal does not escape"
 		x = x + 1
 	}
-	foo() // ERROR "inlining call to s0.func1"
+	foo()   // ERROR "inlining call to s0.func1"
+	_ = foo // prevent simple deadcode elimination
 	return x
 }
 
@@ -137,6 +142,7 @@ func s1(x int) int { // ERROR "can inline s1"
 		return x
 	}
 	x = x + 1
+	_ = foo      // prevent simple deadcode elimination
 	return foo() // ERROR "inlining call to s1.func1"
 }