From 7c1ed1fa8f28a36ad0210cf44e97d7d1a88d8f0b Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Tue, 18 Apr 2023 15:30:15 +0000 Subject: [PATCH] Revert "cmd/compile: rework marking of dead hidden closure functions" 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 TryBot-Result: Gopher Robot Reviewed-by: Cherry Mui Run-TryBot: Than McIntosh --- src/cmd/compile/internal/inline/inl.go | 95 ++++++++++---------------- test/fixedbugs/issue59638.go | 65 ------------------ 2 files changed, 36 insertions(+), 124 deletions(-) delete mode 100644 test/fixedbugs/issue59638.go diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go index 24812c8c0d6..1a65e16f519 100644 --- a/src/cmd/compile/internal/inline/inl.go +++ b/src/cmd/compile/internal/inline/inl.go @@ -229,71 +229,24 @@ 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() + // 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) + } + }) + } + }) 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) - } - } - } - 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) - } - } - } -} - // CanInline determines whether fn is inlineable. // If so, CanInline saves copies of fn.Body and fn.Dcl in fn.Inl. // fn and fn.Body will already have been typechecked. @@ -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 index bba6265322c..00000000000 --- a/test/fixedbugs/issue59638.go +++ /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 -}