[dev.regabi] cmd/compile: compile functions before closures

This CL reorders function compilation to ensure that functions are
always compiled before any enclosed function literals. The primary
goal of this is to reduce the risk of race conditions that arise due
to compilation of function literals needing to inspect data from their
closure variables. However, a pleasant side effect is that it allows
skipping the redundant, separate compilation of function literals that
were inlined into their enclosing function.

Change-Id: I03ee96212988cb578c2452162b7e99cc5e92918f
Reviewed-on: https://go-review.googlesource.com/c/go/+/282892
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>

diff --git a/src/cmd/compile/internal/gc/compile.go b/src/cmd/compile/internal/gc/compile.go
index c2894ab012d78f1e101a1d0aae9ca441f4ba66ec..410b3e90ea5514c6332e9c00786e061c9e554303 100644 (file)
--- a/src/cmd/compile/internal/gc/compile.go
+++ b/src/cmd/compile/internal/gc/compile.go
@@ -37,6 +37,10 @@ func enqueueFunc(fn *ir.Func) {
                return
        }
 
+       if clo := fn.OClosure; clo != nil && !ir.IsTrivialClosure(clo) {
+               return // we'll get this as part of its enclosing function
+       }
+
        if len(fn.Body) == 0 {
                // Initialize ABI wrappers if necessary.
                ssagen.InitLSym(fn, false)
@@ -45,11 +49,22 @@ func enqueueFunc(fn *ir.Func) {
        }
 
        errorsBefore := base.Errors()
-       prepareFunc(fn)
+
+       todo := []*ir.Func{fn}
+       for len(todo) > 0 {
+               next := todo[len(todo)-1]
+               todo = todo[:len(todo)-1]
+
+               prepareFunc(next)
+               todo = append(todo, next.Closures...)
+       }
+
        if base.Errors() > errorsBefore {
                return
        }
 
+       // Enqueue just fn itself. compileFunctions will handle
+       // scheduling compilation of its closures after it's done.
        compilequeue = append(compilequeue, fn)
 }
 
@@ -97,7 +112,6 @@ func compileFunctions() {
                return
        }
 
-       types.CalcSizeDisabled = true // not safe to calculate sizes concurrently
        if race.Enabled {
                // Randomize compilation order to try to shake out races.
                tmp := make([]*ir.Func, len(compilequeue))
@@ -114,22 +128,37 @@ func compileFunctions() {
                        return len(compilequeue[i].Body) > len(compilequeue[j].Body)
                })
        }
-       var wg sync.WaitGroup
-       base.Ctxt.InParallel = true
-       c := make(chan *ir.Func, base.Flag.LowerC)
+
+       // We queue up a goroutine per function that needs to be
+       // compiled, but require them to grab an available worker ID
+       // before doing any substantial work to limit parallelism.
+       workerIDs := make(chan int, base.Flag.LowerC)
        for i := 0; i < base.Flag.LowerC; i++ {
+               workerIDs <- i
+       }
+
+       var wg sync.WaitGroup
+       var asyncCompile func(*ir.Func)
+       asyncCompile = func(fn *ir.Func) {
                wg.Add(1)
-               go func(worker int) {
-                       for fn := range c {
-                               ssagen.Compile(fn, worker)
+               go func() {
+                       worker := <-workerIDs
+                       ssagen.Compile(fn, worker)
+                       workerIDs <- worker
+
+                       // Done compiling fn. Schedule it's closures for compilation.
+                       for _, closure := range fn.Closures {
+                               asyncCompile(closure)
                        }
                        wg.Done()
-               }(i)
+               }()
        }
+
+       types.CalcSizeDisabled = true // not safe to calculate sizes concurrently
+       base.Ctxt.InParallel = true
        for _, fn := range compilequeue {
-               c <- fn
+               asyncCompile(fn)
        }
-       close(c)
        compilequeue = nil
        wg.Wait()
        base.Ctxt.InParallel = false

diff --git a/src/cmd/compile/internal/ir/func.go b/src/cmd/compile/internal/ir/func.go
index d660fe3b406034903bfe8185bc1d6b54340b1d71..3fe23635f42859e09c5d39988a620e1aeed34737 100644 (file)
--- a/src/cmd/compile/internal/ir/func.go
+++ b/src/cmd/compile/internal/ir/func.go
@@ -81,6 +81,10 @@ type Func struct {
        // Byval set if they're captured by value.
        ClosureVars []*Name
 
+       // Enclosed functions that need to be compiled.
+       // Populated during walk.
+       Closures []*Func
+
        // Parents records the parent scope of each scope within a
        // function. The root scope (0) has no parent, so the i'th
        // scope's parent is stored at Parents[i-1].

diff --git a/src/cmd/compile/internal/ir/sizeof_test.go b/src/cmd/compile/internal/ir/sizeof_test.go
index 2ada7231aae3c09b6970d56dc7a1548eaf8a9dc9..f95f77d6a2f09274550d093beb3fd0b42f131028 100644 (file)
--- a/src/cmd/compile/internal/ir/sizeof_test.go
+++ b/src/cmd/compile/internal/ir/sizeof_test.go
@@ -20,7 +20,7 @@ func TestSizeof(t *testing.T) {
                _32bit uintptr     // size on 32bit platforms
                _64bit uintptr     // size on 64bit platforms
        }{
-               {Func{}, 184, 320},
+               {Func{}, 196, 344},
                {Name{}, 116, 208},
        }
 

diff --git a/src/cmd/compile/internal/walk/closure.go b/src/cmd/compile/internal/walk/closure.go
index acb74b9901538b970c7b261d4fe5e9056f1bebb3..7fa63ea9c7b86ef2f698ce0049affbe55ceba006 100644 (file)
--- a/src/cmd/compile/internal/walk/closure.go
+++ b/src/cmd/compile/internal/walk/closure.go
@@ -86,6 +86,8 @@ func walkClosure(clo *ir.ClosureExpr, init *ir.Nodes) ir.Node {
                }
                return fn.Nname
        }
+
+       ir.CurFunc.Closures = append(ir.CurFunc.Closures, fn)
        ir.ClosureDebugRuntimeCheck(clo)
 
        typ := typecheck.ClosureType(clo)

diff --git a/src/cmd/compile/internal/walk/expr.go b/src/cmd/compile/internal/walk/expr.go
index df575d698589b468380ce4aad851d049297b7f61..508cdd1d0640aab7e71069c40d044984296ef098 100644 (file)
--- a/src/cmd/compile/internal/walk/expr.go
+++ b/src/cmd/compile/internal/walk/expr.go
@@ -488,12 +488,15 @@ func walkCall(n *ir.CallExpr, init *ir.Nodes) ir.Node {
                reflectdata.MarkUsedIfaceMethod(n)
        }
 
-       if n.Op() == ir.OCALLFUNC && n.X.Op() == ir.OCLOSURE {
+       if n.Op() == ir.OCALLFUNC && n.X.Op() == ir.OCLOSURE && !ir.IsTrivialClosure(n.X.(*ir.ClosureExpr)) {
                // Transform direct call of a closure to call of a normal function.
                // transformclosure already did all preparation work.
+               // We leave trivial closures for walkClosure to handle.
 
-               // Prepend captured variables to argument list.
                clo := n.X.(*ir.ClosureExpr)
+               ir.CurFunc.Closures = append(ir.CurFunc.Closures, clo.Func)
+
+               // Prepend captured variables to argument list.
                n.Args.Prepend(closureArgs(clo)...)
 
                // Replace OCLOSURE with ONAME/PFUNC.