From 88cb17e1069bef854ead49c703262abdf93c9458 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Thu, 24 Aug 2023 03:04:01 -0700 Subject: [PATCH] cmd/compile: create "init" function during noding This CL arranges for package-scope initialization statements to be constructed directly into their eventual "init" function, so we can eliminate the roundabout solution of using InitTodoFunc. While here, somewhat simplify and generalize the logic for outlining map initialization statements. Change-Id: I8aff042e6b266f7024de436424ec6711b8b69129 Reviewed-on: https://go-review.googlesource.com/c/go/+/522318 Run-TryBot: Matthew Dempsky Reviewed-by: Than McIntosh Reviewed-by: Cuong Manh Le Auto-Submit: Matthew Dempsky TryBot-Result: Gopher Robot --- src/cmd/compile/internal/gc/main.go | 7 +- src/cmd/compile/internal/ir/package.go | 4 - src/cmd/compile/internal/noder/reader.go | 77 +++++++-------- src/cmd/compile/internal/noder/writer.go | 9 +- src/cmd/compile/internal/pkginit/init.go | 59 ------------ src/cmd/compile/internal/staticinit/sched.go | 96 +++++++++---------- .../compile/internal/typecheck/typecheck.go | 27 ------ 7 files changed, 86 insertions(+), 193 deletions(-) diff --git a/src/cmd/compile/internal/gc/main.go b/src/cmd/compile/internal/gc/main.go index 1314a207de..abc4ea561c 100644 --- a/src/cmd/compile/internal/gc/main.go +++ b/src/cmd/compile/internal/gc/main.go @@ -205,14 +205,9 @@ func Main(archInit func(*ssagen.ArchInfo)) { dwarfgen.RecordPackageName() - // Prepare for backend processing. This must happen before pkginit, - // because it generates itabs for initializing global variables. + // Prepare for backend processing. ssagen.InitConfig() - // Create "init" function for package-scope variable initialization - // statements, if any. - pkginit.MakeInit() - // Apply coverage fixups, if applicable. coverage.Fixup() diff --git a/src/cmd/compile/internal/ir/package.go b/src/cmd/compile/internal/ir/package.go index ba8b0d8707..3b70a9281a 100644 --- a/src/cmd/compile/internal/ir/package.go +++ b/src/cmd/compile/internal/ir/package.go @@ -12,10 +12,6 @@ type Package struct { // See golang.org/issue/31636. Imports []*types.Pkg - // InitOrder is the list of package-level initializers in the order - // in which they must be executed. - InitOrder []Node - // Init functions, listed in source order. Inits []*Func diff --git a/src/cmd/compile/internal/noder/reader.go b/src/cmd/compile/internal/noder/reader.go index 0efe2ea2d5..01f001f199 100644 --- a/src/cmd/compile/internal/noder/reader.go +++ b/src/cmd/compile/internal/noder/reader.go @@ -2988,52 +2988,12 @@ func (r *reader) multiExpr() []ir.Node { // temp returns a new autotemp of the specified type. func (r *reader) temp(pos src.XPos, typ *types.Type) *ir.Name { - // See typecheck.typecheckargs. - curfn := r.curfn - if curfn == nil { - curfn = typecheck.InitTodoFunc - } - - return typecheck.TempAt(pos, curfn, typ) + return typecheck.TempAt(pos, r.curfn, typ) } // tempCopy declares and returns a new autotemp initialized to the // value of expr. func (r *reader) tempCopy(pos src.XPos, expr ir.Node, init *ir.Nodes) *ir.Name { - if r.curfn == nil { - // Escape analysis doesn't know how to handle package-scope - // function literals with free variables (i.e., that capture - // temporary variables added to typecheck.InitTodoFunc). - // - // stencil.go works around this limitation by spilling values to - // global variables instead, but that causes the value to stay - // alive indefinitely; see go.dev/issue/54343. - // - // This code path (which implements the same workaround) isn't - // actually needed by unified IR, because it creates uses normal - // OMETHEXPR/OMETHVALUE nodes when statically-known instantiated - // types are used. But it's kept around for now because it's handy - // for testing that the generic fallback paths work correctly. - base.Fatalf("tempCopy called at package scope") - - tmp := staticinit.StaticName(expr.Type()) - - assign := ir.NewAssignStmt(pos, tmp, expr) - assign.Def = true - tmp.Defn = assign - - // TODO(mdempsky): This code doesn't work anymore, because we now - // rely on types2 to compute InitOrder. If it's going to be used - // for testing again, the assignment here probably needs to be - // added to typecheck.Target.InitOrder somewhere. - // - // Probably just easier to address the escape analysis limitation. - // - // typecheck.Target.Decls = append(typecheck.Target.Decls, typecheck.Stmt(assign)) - - return tmp - } - tmp := r.temp(pos, expr.Type()) init.Append(typecheck.Stmt(ir.NewDecl(pos, ir.ODCL, tmp))) @@ -3328,9 +3288,32 @@ func (r *reader) pkgInit(self *types.Pkg, target *ir.Package) { } target.CgoPragmas = cgoPragmas + r.pkgInitOrder(target) + r.pkgDecls(target) + r.Sync(pkgbits.SyncEOF) +} + +// pkgInitOrder creates a synthetic init function to handle any +// package-scope initialization statements. +func (r *reader) pkgInitOrder(target *ir.Package) { initOrder := make([]ir.Node, r.Len()) + if len(initOrder) == 0 { + return + } + + // Make a function that contains all the initialization statements. + pos := base.AutogeneratedPos + base.Pos = pos + + fn := ir.NewFunc(pos, pos, typecheck.Lookup("init"), types.NewSignature(nil, nil, nil)) + fn.SetIsPackageInit(true) + fn.SetInlinabilityChecked(true) // suppress useless "can inline" diagnostics + + typecheck.DeclFunc(fn) + r.curfn = fn + for i := range initOrder { lhs := make([]ir.Node, r.Len()) for j := range lhs { @@ -3352,9 +3335,17 @@ func (r *reader) pkgInit(self *types.Pkg, target *ir.Package) { initOrder[i] = as } - target.InitOrder = initOrder - r.Sync(pkgbits.SyncEOF) + fn.Body = initOrder + + typecheck.FinishFuncBody() + r.curfn = nil + r.locals = nil + + // Outline (if legal/profitable) global map inits. + staticinit.OutlineMapInits(fn) + + target.Inits = append(target.Inits, fn) } func (r *reader) pkgDecls(target *ir.Package) { diff --git a/src/cmd/compile/internal/noder/writer.go b/src/cmd/compile/internal/noder/writer.go index 10cf46f3f2..5982e714a3 100644 --- a/src/cmd/compile/internal/noder/writer.go +++ b/src/cmd/compile/internal/noder/writer.go @@ -2597,6 +2597,8 @@ func (w *writer) pkgInit(noders []*noder) { w.Strings(cgoPragma) } + w.pkgInitOrder() + w.Sync(pkgbits.SyncDecls) for _, p := range noders { for _, decl := range p.file.DeclList { @@ -2605,6 +2607,11 @@ func (w *writer) pkgInit(noders []*noder) { } w.Code(declEnd) + w.Sync(pkgbits.SyncEOF) +} + +func (w *writer) pkgInitOrder() { + // TODO(mdempsky): Write as a function body instead? w.Len(len(w.p.info.InitOrder)) for _, init := range w.p.info.InitOrder { w.Len(len(init.Lhs)) @@ -2613,8 +2620,6 @@ func (w *writer) pkgInit(noders []*noder) { } w.expr(init.Rhs) } - - w.Sync(pkgbits.SyncEOF) } func (w *writer) pkgDecl(decl syntax.Decl) { diff --git a/src/cmd/compile/internal/pkginit/init.go b/src/cmd/compile/internal/pkginit/init.go index 4d4896d447..9278890b63 100644 --- a/src/cmd/compile/internal/pkginit/init.go +++ b/src/cmd/compile/internal/pkginit/init.go @@ -15,67 +15,8 @@ import ( "cmd/internal/obj" "cmd/internal/objabi" "cmd/internal/src" - "fmt" - "os" ) -// MakeInit creates a synthetic init function to handle any -// package-scope initialization statements. -func MakeInit() { - nf := typecheck.Target.InitOrder - if len(nf) == 0 { - return - } - - // Make a function that contains all the initialization statements. - pos := nf[0].Pos() // prolog/epilog gets line number of first init stmt - base.Pos = pos - - sym := typecheck.Lookup("init") - fn := ir.NewFunc(pos, pos, sym, types.NewSignature(nil, nil, nil)) - typecheck.DeclFunc(fn) - - for _, dcl := range typecheck.InitTodoFunc.Dcl { - dcl.Curfn = fn - } - fn.Dcl = append(fn.Dcl, typecheck.InitTodoFunc.Dcl...) - typecheck.InitTodoFunc.Dcl = nil - fn.SetIsPackageInit(true) - - // Outline (if legal/profitable) global map inits. - nf, newfuncs := staticinit.OutlineMapInits(nf) - - // Suppress useless "can inline" diagnostics. - // Init functions are only called dynamically. - fn.SetInlinabilityChecked(true) - for _, nfn := range newfuncs { - nfn.SetInlinabilityChecked(true) - } - - fn.Body = nf - typecheck.FinishFuncBody() - - ir.WithFunc(fn, func() { - typecheck.Stmts(nf) - }) - if base.Debug.WrapGlobalMapDbg > 1 { - fmt.Fprintf(os.Stderr, "=-= len(newfuncs) is %d for %v\n", - len(newfuncs), fn) - } - - // Prepend to Inits, so it runs first, before any user-declared init - // functions. - typecheck.Target.Inits = append([]*ir.Func{fn}, typecheck.Target.Inits...) - - if typecheck.InitTodoFunc.Dcl != nil { - // We only generate temps using InitTodoFunc if there - // are package-scope initialization statements, so - // something's weird if we get here. - base.Fatalf("InitTodoFunc still has declarations") - } - typecheck.InitTodoFunc = nil -} - // MakeTask makes an initialization record for the package, if necessary. // See runtime/proc.go:initTask for its layout. // The 3 tasks for initialization are: diff --git a/src/cmd/compile/internal/staticinit/sched.go b/src/cmd/compile/internal/staticinit/sched.go index 52d3d029ad..dd370a305c 100644 --- a/src/cmd/compile/internal/staticinit/sched.go +++ b/src/cmd/compile/internal/staticinit/sched.go @@ -980,26 +980,26 @@ func addStr(n *ir.AddStringExpr) ir.Node { const wrapGlobalMapInitSizeThreshold = 20 -// tryWrapGlobalMapInit examines the node 'n' to see if it is a map -// variable initialization, and if so, possibly returns the mapvar -// being assigned, a new function containing the init code, and a call -// to the function passing the mapvar. Returns will be nil if the -// assignment is not to a map, or the map init is not big enough, -// or if the expression being assigned to the map has side effects. -func tryWrapGlobalMapInit(n ir.Node) (mapvar *ir.Name, genfn *ir.Func, call ir.Node) { +// tryWrapGlobalInit returns a new outlined function to contain global +// initializer statement n, if possible and worthwhile. Otherwise, it +// returns nil. +// +// Currently, it outlines map assignment statements with large, +// side-effect-free RHS expressions. +func tryWrapGlobalInit(n ir.Node) *ir.Func { // Look for "X = ..." where X has map type. // FIXME: might also be worth trying to look for cases where // the LHS is of interface type but RHS is map type. if n.Op() != ir.OAS { - return nil, nil, nil + return nil } as := n.(*ir.AssignStmt) if ir.IsBlank(as.X) || as.X.Op() != ir.ONAME { - return nil, nil, nil + return nil } nm := as.X.(*ir.Name) if !nm.Type().IsMap() { - return nil, nil, nil + return nil } // Determine size of RHS. @@ -1019,7 +1019,7 @@ func tryWrapGlobalMapInit(n ir.Node) (mapvar *ir.Name, genfn *ir.Func, call ir.N fmt.Fprintf(os.Stderr, "=-= skipping %v size too small at %d\n", nm, rsiz) } - return nil, nil, nil + return nil } // Reject right hand sides with side effects. @@ -1027,7 +1027,7 @@ func tryWrapGlobalMapInit(n ir.Node) (mapvar *ir.Name, genfn *ir.Func, call ir.N if base.Debug.WrapGlobalMapDbg > 0 { fmt.Fprintf(os.Stderr, "=-= rejected %v due to side effects\n", nm) } - return nil, nil, nil + return nil } if base.Debug.WrapGlobalMapDbg > 1 { @@ -1036,17 +1036,19 @@ func tryWrapGlobalMapInit(n ir.Node) (mapvar *ir.Name, genfn *ir.Func, call ir.N // Create a new function that will (eventually) have this form: // - // func map.init.%d() { - // globmapvar = - // } + // func map.init.%d() { + // globmapvar = + // } // + // Note: cmd/link expects the function name to contain "map.init". minitsym := typecheck.LookupNum("map.init.", mapinitgen) mapinitgen++ - newfn := ir.NewFunc(base.Pos, base.Pos, minitsym, types.NewSignature(nil, nil, nil)) - typecheck.DeclFunc(newfn) + fn := ir.NewFunc(n.Pos(), n.Pos(), minitsym, types.NewSignature(nil, nil, nil)) + fn.SetInlinabilityChecked(true) // suppress inlining (which would defeat the point) + typecheck.DeclFunc(fn) if base.Debug.WrapGlobalMapDbg > 0 { - fmt.Fprintf(os.Stderr, "=-= generated func is %v\n", newfn) + fmt.Fprintf(os.Stderr, "=-= generated func is %v\n", fn) } // NB: we're relying on this phase being run before inlining; @@ -1054,24 +1056,17 @@ func tryWrapGlobalMapInit(n ir.Node) (mapvar *ir.Name, genfn *ir.Func, call ir.N // need code here that relocates or duplicates inline temps. // Insert assignment into function body; mark body finished. - newfn.Body = append(newfn.Body, as) + fn.Body = []ir.Node{as} typecheck.FinishFuncBody() - const no = ` - // Register new function with decls. - typecheck.Target.Decls = append(typecheck.Target.Decls, newfn) -` - - // Create call to function, passing mapvar. - fncall := ir.NewCallExpr(n.Pos(), ir.OCALL, newfn.Nname, nil) - if base.Debug.WrapGlobalMapDbg > 1 { fmt.Fprintf(os.Stderr, "=-= mapvar is %v\n", nm) - fmt.Fprintf(os.Stderr, "=-= newfunc is %+v\n", newfn) - fmt.Fprintf(os.Stderr, "=-= call is %+v\n", fncall) + fmt.Fprintf(os.Stderr, "=-= newfunc is %+v\n", fn) } - return nm, newfn, typecheck.Stmt(fncall) + recordFuncForVar(nm, fn) + + return fn } // mapinitgen is a counter used to uniquify compiler-generated @@ -1108,31 +1103,28 @@ func AddKeepRelocations() { varToMapInit = nil } -// OutlineMapInits walks through a list of init statements (candidates -// for inclusion in the package "init" function) and returns an -// updated list in which items corresponding to map variable -// initializations have been replaced with calls to outline "map init" -// functions (if legal/profitable). Return value is an updated list -// and a list of any newly generated "map init" functions. -func OutlineMapInits(stmts []ir.Node) ([]ir.Node, []*ir.Func) { +// OutlineMapInits replaces global map initializers with outlined +// calls to separate "map init" functions (where possible and +// profitable), to facilitate better dead-code elimination by the +// linker. +func OutlineMapInits(fn *ir.Func) { if base.Debug.WrapGlobalMapCtl == 1 { - return stmts, nil + return } - newfuncs := []*ir.Func{} - for i := range stmts { - s := stmts[i] - // Call the helper tryWrapGlobalMapInit to see if the LHS of - // this assignment is to a map var, and if so whether the RHS - // should be outlined into a separate init function. If the - // outline goes through, then replace the original init - // statement with the call to the outlined func, and append - // the new outlined func to our return list. - if mapvar, genfn, call := tryWrapGlobalMapInit(s); call != nil { - stmts[i] = call - newfuncs = append(newfuncs, genfn) - recordFuncForVar(mapvar, genfn) + + outlined := 0 + for i, stmt := range fn.Body { + // Attempt to outline stmt. If successful, replace it with a call + // to the returned wrapper function. + if wrapperFn := tryWrapGlobalInit(stmt); wrapperFn != nil { + ir.WithFunc(fn, func() { + fn.Body[i] = typecheck.Call(stmt.Pos(), wrapperFn.Nname, nil, false) + }) + outlined++ } } - return stmts, newfuncs + if base.Debug.WrapGlobalMapDbg > 1 { + fmt.Fprintf(os.Stderr, "=-= outlined %v map initializations\n", outlined) + } } diff --git a/src/cmd/compile/internal/typecheck/typecheck.go b/src/cmd/compile/internal/typecheck/typecheck.go index a06fa7f5cd..1dc827d1fe 100644 --- a/src/cmd/compile/internal/typecheck/typecheck.go +++ b/src/cmd/compile/internal/typecheck/typecheck.go @@ -16,10 +16,6 @@ import ( "cmd/internal/src" ) -// Function collecting autotmps generated during typechecking, -// to be included in the package-level init function. -var InitTodoFunc = ir.NewFunc(base.Pos, base.Pos, Lookup("$InitTodo"), types.NewSignature(nil, nil, nil)) - func AssignExpr(n ir.Node) ir.Node { return typecheck(n, ctxExpr|ctxAssign) } func Expr(n ir.Node) ir.Node { return typecheck(n, ctxExpr) } func Stmt(n ir.Node) ir.Node { return typecheck(n, ctxStmt) } @@ -656,37 +652,17 @@ func RewriteNonNameCall(n *ir.CallExpr) { return } - // See comment (1) in RewriteMultiValueCall. - static := ir.CurFunc == nil - if static { - ir.CurFunc = InitTodoFunc - } - tmp := TempAt(base.Pos, ir.CurFunc, (*np).Type()) as := ir.NewAssignStmt(base.Pos, tmp, *np) as.PtrInit().Append(Stmt(ir.NewDecl(n.Pos(), ir.ODCL, tmp))) *np = tmp - if static { - ir.CurFunc = nil - } - n.PtrInit().Append(Stmt(as)) } // RewriteMultiValueCall rewrites multi-valued f() to use temporaries, // so the backend wouldn't need to worry about tuple-valued expressions. func RewriteMultiValueCall(n ir.InitNode, call ir.Node) { - // If we're outside of function context, then this call will - // be executed during the generated init function. However, - // init.go hasn't yet created it. Instead, associate the - // temporary variables with InitTodoFunc for now, and init.go - // will reassociate them later when it's appropriate. (1) - static := ir.CurFunc == nil - if static { - ir.CurFunc = InitTodoFunc - } - as := ir.NewAssignListStmt(base.Pos, ir.OAS2, nil, []ir.Node{call}) results := call.Type().Fields() list := make([]ir.Node, len(results)) @@ -696,9 +672,6 @@ func RewriteMultiValueCall(n ir.InitNode, call ir.Node) { as.Lhs.Append(tmp) list[i] = tmp } - if static { - ir.CurFunc = nil - } n.PtrInit().Append(Stmt(as)) -- 2.48.1