]> Cypherpunks.ru repositories - gostls13.git/commitdiff
Revert "cmd/compile: rework marking of dead hidden closure functions"
authorThan McIntosh <thanm@google.com>
Tue, 18 Apr 2023 15:30:15 +0000 (15:30 +0000)
committerThan McIntosh <thanm@google.com>
Tue, 18 Apr 2023 16:03:22 +0000 (16:03 +0000)
This reverts commit http://go.dev/cl//484859

Reason for revert: causes linker errors in a number of google-internal tests.

Change-Id: I322252f784a46d2b1d447ebcdca86ce14bc0cc91
Reviewed-on: https://go-review.googlesource.com/c/go/+/485755
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>

src/cmd/compile/internal/inline/inl.go
test/fixedbugs/issue59638.go [deleted file]

index 24812c8c0d67f715de07f36e02bc12a4f63c68fe..1a65e16f5199f6a139db27470d1c3bde12907c11 100644 (file)
@@ -229,68 +229,21 @@ func InlineDecls(p *pgo.Profile, decls []ir.Node, doInline bool) {
                }
        })
 
-       // Perform a garbage collection of hidden closures functions that
-       // are no longer reachable from top-level functions following
-       // inlining. See #59404 and #59638 for more context.
-       garbageCollectUnreferencedHiddenClosures()
-
-       if p != nil {
-               pgoInlineEpilogue(p, decls)
-       }
-}
-
-// garbageCollectUnreferencedHiddenClosures makes a pass over all the
-// top-level (non-hidden-closure) functions looking for nested closure
-// functions that are reachable, then sweeps through the Target.Decls
-// list and marks any non-reachable hidden closure function as dead.
-// See issues #59404 and #59638 for more context.
-func garbageCollectUnreferencedHiddenClosures() {
-
-       liveFuncs := make(map[*ir.Func]bool)
-
-       var markLiveFuncs func(fn *ir.Func)
-       markLiveFuncs = func(fn *ir.Func) {
-               liveFuncs[fn] = true
-               var vis func(node ir.Node)
-               vis = func(node ir.Node) {
-                       if clo, ok := node.(*ir.ClosureExpr); ok {
-                               if !liveFuncs[clo.Func] {
-                                       liveFuncs[clo.Func] = true
-                                       markLiveFuncs(clo.Func)
+       // Rewalk post-inlining functions to check for closures that are
+       // still visible but were (over-agressively) marked as dead, and
+       // undo that marking here. See #59404 for more context.
+       ir.VisitFuncsBottomUp(decls, func(list []*ir.Func, recursive bool) {
+               for _, n := range list {
+                       ir.Visit(n, func(node ir.Node) {
+                               if clo, ok := node.(*ir.ClosureExpr); ok && clo.Func.IsHiddenClosure() {
+                                       clo.Func.SetIsDeadcodeClosure(false)
                                }
-                       }
+                       })
                }
-               ir.Visit(fn, vis)
-       }
-
-       for i := 0; i < len(typecheck.Target.Decls); i++ {
-               if fn, ok := typecheck.Target.Decls[i].(*ir.Func); ok {
-                       if fn.IsHiddenClosure() {
-                               continue
-                       }
-                       markLiveFuncs(fn)
-               }
-       }
+       })
 
-       for i := 0; i < len(typecheck.Target.Decls); i++ {
-               if fn, ok := typecheck.Target.Decls[i].(*ir.Func); ok {
-                       if !fn.IsHiddenClosure() {
-                               continue
-                       }
-                       if fn.IsDeadcodeClosure() {
-                               continue
-                       }
-                       if liveFuncs[fn] {
-                               continue
-                       }
-                       fn.SetIsDeadcodeClosure(true)
-                       if base.Flag.LowerM > 2 {
-                               fmt.Printf("%v: unreferenced closure %v marked as dead\n", ir.Line(fn), fn)
-                       }
-                       if fn.Inl != nil && fn.LSym == nil {
-                               ir.InitLSym(fn, true)
-                       }
-               }
+       if p != nil {
+               pgoInlineEpilogue(p, decls)
        }
 }
 
@@ -940,6 +893,30 @@ func inlnode(n ir.Node, bigCaller bool, inlCalls *[]*ir.InlinedCallExpr, edit fu
                }
                if fn := inlCallee(call.X, profile); fn != nil && typecheck.HaveInlineBody(fn) {
                        n = mkinlcall(call, fn, bigCaller, inlCalls, edit)
+                       if fn.IsHiddenClosure() {
+                               // Visit function to pick out any contained hidden
+                               // closures to mark them as dead, since they will no
+                               // longer be reachable (if we leave them live, they
+                               // will get skipped during escape analysis, which
+                               // could mean that go/defer statements don't get
+                               // desugared, causing later problems in walk). See
+                               // #59404 for more context. Note also that the code
+                               // below can sometimes be too aggressive (marking a closure
+                               // dead even though it was captured by a local var).
+                               // In this case we'll undo the dead marking in a cleanup
+                               // pass that happens at the end of InlineDecls.
+                               var vis func(node ir.Node)
+                               vis = func(node ir.Node) {
+                                       if clo, ok := node.(*ir.ClosureExpr); ok && clo.Func.IsHiddenClosure() && !clo.Func.IsDeadcodeClosure() {
+                                               if base.Flag.LowerM > 2 {
+                                                       fmt.Printf("%v: closure %v marked as dead\n", ir.Line(clo.Func), clo.Func)
+                                               }
+                                               clo.Func.SetIsDeadcodeClosure(true)
+                                               ir.Visit(clo.Func, vis)
+                                       }
+                               }
+                               ir.Visit(fn, vis)
+                       }
                }
        }
 
diff --git a/test/fixedbugs/issue59638.go b/test/fixedbugs/issue59638.go
deleted file mode 100644 (file)
index bba6265..0000000
+++ /dev/null
@@ -1,65 +0,0 @@
-// build -gcflags=-l=4
-
-// 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 p
-
-type Interface interface {
-       MonitoredResource() (resType string, labels map[string]string)
-       Done()
-}
-
-func Autodetect(x int) Interface {
-       return func() Interface {
-               func() Interface {
-                       x++
-                       Do(func() {
-                               var ad, gd Interface
-
-                               go func() {
-                                       defer gd.Done()
-                                       ad = aad()
-                               }()
-                               go func() {
-                                       defer ad.Done()
-                                       gd = aad()
-                                       defer func() { recover() }()
-                               }()
-
-                               autoDetected = ad
-                               if gd != nil {
-                                       autoDetected = gd
-                               }
-                       })
-                       return autoDetected
-               }()
-               return nil
-       }()
-}
-
-var autoDetected Interface
-var G int
-
-type If int
-
-func (x If) MonitoredResource() (resType string, labels map[string]string) {
-       return "", nil
-}
-
-//go:noinline
-func (x If) Done() {
-       G++
-}
-
-//go:noinline
-func Do(fn func()) {
-       fn()
-}
-
-//go:noinline
-func aad() Interface {
-       var x If
-       return x
-}