]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: add "deadlocals" pass to remove unused locals
authorDavid Chase <drchase@google.com>
Wed, 22 May 2024 21:00:43 +0000 (14:00 -0700)
committerDavid Chase <drchase@google.com>
Tue, 30 Jul 2024 15:46:27 +0000 (15:46 +0000)
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
src/cmd/compile/internal/deadlocals/deadlocals.go [new file with mode: 0644]
src/cmd/compile/internal/gc/main.go
src/runtime/race/testdata/mop_test.go
test/closure3.dir/main.go
test/fixedbugs/issue54159.go
test/inline.go
test/newinline.go

index c1b62f27ca2b41c139a75a7a431dd3e77bda0ab8..f53a32f8fa8f6a1fa09b2cf7450a6a56dc835be6 100644 (file)
@@ -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"`
diff --git a/src/cmd/compile/internal/deadlocals/deadlocals.go b/src/cmd/compile/internal/deadlocals/deadlocals.go
new file mode 100644 (file)
index 0000000..f40ca71
--- /dev/null
@@ -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))
+}
index f68cf4deaf041a92d34ba3ac610adb519e2bf2a6..174c609e44521714179d872c3ce718713ef7eed2 100644 (file)
@@ -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,
index 6b1069fcca2a81bae7e3558ff176d0f241c16e48..0d7d879d834d36ce18196a4b727be53a9c65eaf7 100644 (file)
@@ -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()
index e3981a5161a8ce93d81e8b1a2e0789cf87bdf61f..7bed78c30829d0da3b4aed6f3db80ff0b9e198ae 100644 (file)
@@ -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"
index 8ef0e684837edda5cfe76168b52457a449f3b97a..0f607b38e12020bef668b5c0f0d896c88a2e5a3a 100644 (file)
@@ -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()
 }
 
index fd14f25983355597230030735903f98b6a890268..4714c795c2fdbaa787eb2a8e23142576299979c6 100644 (file)
@@ -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"
 }
 
index 69f1310ab2f3f41fedd88cdcf25a2d31ea7549ee..a7288691cd24fa816bcded335e7b5f9bf25d02fc 100644 (file)
@@ -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"
 }