From f47cfd6cb55ce0ba6a3f77eeb33c60713f8493e9 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Fri, 11 Aug 2023 12:04:22 -0700 Subject: [PATCH] cmd/compile: simplify asmhdr and plugin exports handling This CL removes a bunch of obsolete code, which made the overall possible data flow of the compiler much harder to understand. In particular, it: 1. Removes typecheck.Declare by inlining its only two remaining uses, and simplifying them down to just the couple of relevant assignments for each remaining caller. 2. Renames ir.Package.{Asms,Exports} to {AsmHdrDecls,PluginExports}, respectively, to better describe what they're used for. In particular, PluginExports now actually holds only the subset of Exports that used to be confusingly called "ptabs" in package reflectdata. 3. Renames reflectdata.WriteTabs to reflectdata.WritePluginTable, to make it clearer what it does. 4. Removes the consistency checks on len(Exports) and len(ptabs), since now it's plainly obvious that only the unified importer ever appends to PluginExports. Change-Id: Iedc9d0a4e7648de4e734f7e3e7df302580fed542 Reviewed-on: https://go-review.googlesource.com/c/go/+/518757 Reviewed-by: Keith Randall Auto-Submit: Matthew Dempsky TryBot-Result: Gopher Robot Reviewed-by: Keith Randall Run-TryBot: Matthew Dempsky --- src/cmd/compile/internal/gc/export.go | 2 +- src/cmd/compile/internal/gc/main.go | 4 +- src/cmd/compile/internal/gc/obj.go | 13 +-- src/cmd/compile/internal/ir/package.go | 13 ++- src/cmd/compile/internal/noder/reader.go | 8 +- src/cmd/compile/internal/pkginit/init.go | 7 +- .../compile/internal/reflectdata/reflect.go | 92 +++++++----------- src/cmd/compile/internal/staticinit/sched.go | 10 +- src/cmd/compile/internal/typecheck/dcl.go | 95 ++----------------- 9 files changed, 68 insertions(+), 176 deletions(-) diff --git a/src/cmd/compile/internal/gc/export.go b/src/cmd/compile/internal/gc/export.go index c9acfc1710..6d73c38d0b 100644 --- a/src/cmd/compile/internal/gc/export.go +++ b/src/cmd/compile/internal/gc/export.go @@ -21,7 +21,7 @@ func dumpasmhdr() { base.Fatalf("%v", err) } fmt.Fprintf(b, "// generated by compile -asmhdr from package %s\n\n", types.LocalPkg.Name) - for _, n := range typecheck.Target.Asms { + for _, n := range typecheck.Target.AsmHdrDecls { if n.Sym().IsBlank() { continue } diff --git a/src/cmd/compile/internal/gc/main.go b/src/cmd/compile/internal/gc/main.go index c1090c58c1..94043719aa 100644 --- a/src/cmd/compile/internal/gc/main.go +++ b/src/cmd/compile/internal/gc/main.go @@ -269,9 +269,7 @@ func Main(archInit func(*ssagen.ArchInfo)) { ir.CurFunc = nil // Build init task, if needed. - if initTask := pkginit.Task(); initTask != nil { - typecheck.Export(initTask) - } + pkginit.MakeTask() // Generate ABI wrappers. Must happen before escape analysis // and doesn't benefit from dead-coding or inlining. diff --git a/src/cmd/compile/internal/gc/obj.go b/src/cmd/compile/internal/gc/obj.go index a246177aa5..4ff249ca2e 100644 --- a/src/cmd/compile/internal/gc/obj.go +++ b/src/cmd/compile/internal/gc/obj.go @@ -113,12 +113,9 @@ func dumpdata() { numExterns := len(typecheck.Target.Externs) numDecls := len(typecheck.Target.Funcs) dumpglobls(typecheck.Target.Externs) - reflectdata.CollectPTabs() - numExports := len(typecheck.Target.Exports) addsignats(typecheck.Target.Externs) reflectdata.WriteRuntimeTypes() - reflectdata.WriteTabs() - numPTabs := reflectdata.CountPTabs() + reflectdata.WritePluginTable() reflectdata.WriteImportStrings() reflectdata.WriteBasicTypes() dumpembeds() @@ -154,14 +151,6 @@ func dumpdata() { staticdata.WriteFuncSyms() addGCLocals() - - if numExports != len(typecheck.Target.Exports) { - base.Fatalf("Target.Exports changed after compile functions loop") - } - newNumPTabs := reflectdata.CountPTabs() - if newNumPTabs != numPTabs { - base.Fatalf("ptabs changed after compile functions loop") - } } func dumpLinkerObj(bout *bio.Writer) { diff --git a/src/cmd/compile/internal/ir/package.go b/src/cmd/compile/internal/ir/package.go index 6efdfb5e1f..ba8b0d8707 100644 --- a/src/cmd/compile/internal/ir/package.go +++ b/src/cmd/compile/internal/ir/package.go @@ -27,8 +27,10 @@ type Package struct { // declared at package scope. Externs []*Name - // Assembly function declarations. - Asms []*Name + // AsmHdrDecls holds declared constants and struct types that should + // be included in -asmhdr output. It's only populated when -asmhdr + // is set. + AsmHdrDecls []*Name // Cgo directives. CgoPragmas [][]string @@ -36,6 +38,9 @@ type Package struct { // Variables with //go:embed lines. Embeds []*Name - // Exported (or re-exported) symbols. - Exports []*Name + // PluginExports holds exported functions and variables that are + // accessible through the package plugin API. It's only populated + // for -buildmode=plugin (i.e., compiling package main and -dynlink + // is set). + PluginExports []*Name } diff --git a/src/cmd/compile/internal/noder/reader.go b/src/cmd/compile/internal/noder/reader.go index 0f936b4764..9448f234b7 100644 --- a/src/cmd/compile/internal/noder/reader.go +++ b/src/cmd/compile/internal/noder/reader.go @@ -3418,15 +3418,15 @@ func (r *reader) pkgObjs(target *ir.Package) []*ir.Name { } } - if types.IsExported(sym.Name) { + if base.Ctxt.Flag_dynlink && types.LocalPkg.Name == "main" && types.IsExported(sym.Name) && name.Op() == ir.ONAME { assert(!sym.OnExportList()) - target.Exports = append(target.Exports, name) + target.PluginExports = append(target.PluginExports, name) sym.SetOnExportList(true) } - if base.Flag.AsmHdr != "" { + if base.Flag.AsmHdr != "" && (name.Op() == ir.OLITERAL || name.Op() == ir.OTYPE) { assert(!sym.Asm()) - target.Asms = append(target.Asms, name) + target.AsmHdrDecls = append(target.AsmHdrDecls, name) sym.SetAsm(true) } } diff --git a/src/cmd/compile/internal/pkginit/init.go b/src/cmd/compile/internal/pkginit/init.go index dbd88dcda9..95e3b5cee3 100644 --- a/src/cmd/compile/internal/pkginit/init.go +++ b/src/cmd/compile/internal/pkginit/init.go @@ -81,13 +81,13 @@ func MakeInit() { typecheck.InitTodoFunc = nil } -// Task makes and returns an initialization record for the package. +// MakeTask makes an initialization record for the package, if necessary. // See runtime/proc.go:initTask for its layout. // The 3 tasks for initialization are: // 1. Initialize all of the packages the current package depends on. // 2. Initialize all the variables that have initializers. // 3. Run any init functions. -func Task() *ir.Name { +func MakeTask() { var deps []*obj.LSym // initTask records for packages the current package depends on var fns []*obj.LSym // functions to call for package initialization @@ -188,7 +188,7 @@ func Task() *ir.Name { } if len(deps) == 0 && len(fns) == 0 && types.LocalPkg.Path != "main" && types.LocalPkg.Path != "runtime" { - return nil // nothing to initialize + return // nothing to initialize } // Make an .inittask structure. @@ -216,7 +216,6 @@ func Task() *ir.Name { // An initTask has pointers, but none into the Go heap. // It's not quite read only, the state field must be modifiable. objw.Global(lsym, int32(ot), obj.NOPTR) - return task } // initRequiredForCoverage returns TRUE if we need to force creation diff --git a/src/cmd/compile/internal/reflectdata/reflect.go b/src/cmd/compile/internal/reflectdata/reflect.go index 88a233842e..728976f48e 100644 --- a/src/cmd/compile/internal/reflectdata/reflect.go +++ b/src/cmd/compile/internal/reflectdata/reflect.go @@ -32,10 +32,6 @@ type ptabEntry struct { t *types.Type } -func CountPTabs() int { - return len(ptabs) -} - // runtime interface and reflection data structures var ( // protects signatset and signatslice @@ -47,8 +43,6 @@ var ( gcsymmu sync.Mutex // protects gcsymset and gcsymslice gcsymset = make(map[*types.Type]struct{}) - - ptabs []*ir.Name ) type typeSig struct { @@ -1352,39 +1346,41 @@ func writeITab(lsym *obj.LSym, typ, iface *types.Type, allowNonImplement bool) { lsym.Set(obj.AttrContentAddressable, true) } -func WriteTabs() { - // process ptabs - if types.LocalPkg.Name == "main" && len(ptabs) > 0 { - ot := 0 - s := base.Ctxt.Lookup("go:plugin.tabs") - for _, p := range ptabs { - // Dump ptab symbol into go.pluginsym package. - // - // type ptab struct { - // name nameOff - // typ typeOff // pointer to symbol - // } - nsym := dname(p.Sym().Name, "", nil, true, false) - t := p.Type() - if p.Class != ir.PFUNC { - t = types.NewPtr(t) - } - tsym := writeType(t) - ot = objw.SymPtrOff(s, ot, nsym) - ot = objw.SymPtrOff(s, ot, tsym) - // Plugin exports symbols as interfaces. Mark their types - // as UsedInIface. - tsym.Set(obj.AttrUsedInIface, true) - } - objw.Global(s, int32(ot), int16(obj.RODATA)) +func WritePluginTable() { + ptabs := typecheck.Target.PluginExports + if len(ptabs) == 0 { + return + } - ot = 0 - s = base.Ctxt.Lookup("go:plugin.exports") - for _, p := range ptabs { - ot = objw.SymPtr(s, ot, p.Linksym(), 0) + lsym := base.Ctxt.Lookup("go:plugin.tabs") + ot := 0 + for _, p := range ptabs { + // Dump ptab symbol into go.pluginsym package. + // + // type ptab struct { + // name nameOff + // typ typeOff // pointer to symbol + // } + nsym := dname(p.Sym().Name, "", nil, true, false) + t := p.Type() + if p.Class != ir.PFUNC { + t = types.NewPtr(t) } - objw.Global(s, int32(ot), int16(obj.RODATA)) + tsym := writeType(t) + ot = objw.SymPtrOff(lsym, ot, nsym) + ot = objw.SymPtrOff(lsym, ot, tsym) + // Plugin exports symbols as interfaces. Mark their types + // as UsedInIface. + tsym.Set(obj.AttrUsedInIface, true) + } + objw.Global(lsym, int32(ot), int16(obj.RODATA)) + + lsym = base.Ctxt.Lookup("go:plugin.exports") + ot = 0 + for _, p := range ptabs { + ot = objw.SymPtr(lsym, ot, p.Linksym(), 0) } + objw.Global(lsym, int32(ot), int16(obj.RODATA)) } func WriteImportStrings() { @@ -1754,30 +1750,6 @@ func ZeroAddr(size int64) ir.Node { return typecheck.Expr(typecheck.NodAddr(x)) } -func CollectPTabs() { - if !base.Ctxt.Flag_dynlink || types.LocalPkg.Name != "main" { - return - } - for _, exportn := range typecheck.Target.Exports { - s := exportn.Sym() - nn := ir.AsNode(s.Def) - if nn == nil { - continue - } - if nn.Op() != ir.ONAME { - continue - } - n := nn.(*ir.Name) - if !types.IsExported(s.Name) { - continue - } - if s.Pkg.Name != "main" { - continue - } - ptabs = append(ptabs, n) - } -} - // NeedEmit reports whether typ is a type that we need to emit code // for (e.g., runtime type descriptors, method wrappers). func NeedEmit(typ *types.Type) bool { diff --git a/src/cmd/compile/internal/staticinit/sched.go b/src/cmd/compile/internal/staticinit/sched.go index d71f7475ee..ca70591cd9 100644 --- a/src/cmd/compile/internal/staticinit/sched.go +++ b/src/cmd/compile/internal/staticinit/sched.go @@ -681,9 +681,15 @@ var statuniqgen int // name generator for static temps // Use readonlystaticname for read-only node. func StaticName(t *types.Type) *ir.Name { // Don't use LookupNum; it interns the resulting string, but these are all unique. - n := typecheck.NewName(typecheck.Lookup(fmt.Sprintf("%s%d", obj.StaticNamePref, statuniqgen))) + sym := typecheck.Lookup(fmt.Sprintf("%s%d", obj.StaticNamePref, statuniqgen)) statuniqgen++ - typecheck.Declare(n, ir.PEXTERN) + + n := typecheck.NewName(sym) + sym.Def = n + + n.Class = ir.PEXTERN + typecheck.Target.Externs = append(typecheck.Target.Externs, n) + n.SetType(t) n.Linksym().Set(obj.AttrStatic, true) return n diff --git a/src/cmd/compile/internal/typecheck/dcl.go b/src/cmd/compile/internal/typecheck/dcl.go index cd31c5fea4..8bd1d03222 100644 --- a/src/cmd/compile/internal/typecheck/dcl.go +++ b/src/cmd/compile/internal/typecheck/dcl.go @@ -27,10 +27,10 @@ func DeclFunc(sym *types.Sym, recv *ir.Field, params, results []*ir.Field) *ir.F var recv1 *types.Field if recv != nil { - recv1 = declareParam(ir.PPARAM, -1, recv) + recv1 = declareParam(fn, ir.PPARAM, -1, recv) } - typ := types.NewSignature(recv1, declareParams(ir.PPARAM, params), declareParams(ir.PPARAMOUT, results)) + typ := types.NewSignature(recv1, declareParams(fn, ir.PPARAM, params), declareParams(fn, ir.PPARAMOUT, results)) checkdupfields("argument", typ.Recvs().FieldSlice(), typ.Params().FieldSlice(), typ.Results().FieldSlice()) fn.Nname.SetType(typ) fn.Nname.SetTypecheck(1) @@ -38,66 +38,6 @@ func DeclFunc(sym *types.Sym, recv *ir.Field, params, results []*ir.Field) *ir.F return fn } -// Declare records that Node n declares symbol n.Sym in the specified -// declaration context. -func Declare(n *ir.Name, ctxt ir.Class) { - if ir.IsBlank(n) { - return - } - - s := n.Sym() - - // kludgy: TypecheckAllowed means we're past parsing. Eg reflectdata.methodWrapper may declare out of package names later. - if !inimport && !TypecheckAllowed && s.Pkg != types.LocalPkg { - base.ErrorfAt(n.Pos(), 0, "cannot declare name %v", s) - } - - if ctxt == ir.PEXTERN { - if s.Name == "init" { - base.ErrorfAt(n.Pos(), errors.InvalidInitDecl, "cannot declare init - must be func") - } - if s.Name == "main" && s.Pkg.Name == "main" { - base.ErrorfAt(n.Pos(), errors.InvalidMainDecl, "cannot declare main - must be func") - } - Target.Externs = append(Target.Externs, n) - s.Def = n - } else { - if ir.CurFunc == nil && ctxt == ir.PAUTO { - base.Pos = n.Pos() - base.Fatalf("automatic outside function") - } - if ir.CurFunc != nil && ctxt != ir.PFUNC && n.Op() == ir.ONAME { - ir.CurFunc.Dcl = append(ir.CurFunc.Dcl, n) - } - n.Curfn = ir.CurFunc - } - - if ctxt == ir.PAUTO { - n.SetFrameOffset(0) - } - - n.Class = ctxt - if ctxt == ir.PFUNC { - n.Sym().SetFunc(true) - } - - autoexport(n, ctxt) -} - -// Export marks n for export (or reexport). -func Export(n *ir.Name) { - if n.Sym().OnExportList() { - return - } - n.Sym().SetOnExportList(true) - - if base.Flag.E != 0 { - fmt.Printf("export symbol %v\n", n.Sym()) - } - - Target.Exports = append(Target.Exports, n) -} - // declare the function proper // and declare the arguments. // called in extern-declaration context @@ -125,26 +65,6 @@ func CheckFuncStack() { } } -func autoexport(n *ir.Name, ctxt ir.Class) { - if n.Sym().Pkg != types.LocalPkg { - return - } - if (ctxt != ir.PEXTERN && ctxt != ir.PFUNC) || DeclContext != ir.PEXTERN { - return - } - if n.Type() != nil && n.Type().IsKind(types.TFUNC) && ir.IsMethod(n) { - return - } - - if types.IsExported(n.Sym().Name) || n.Sym().Name == "init" { - Export(n) - } - if base.Flag.AsmHdr != "" && !n.Sym().Asm() { - n.Sym().SetAsm(true) - Target.Asms = append(Target.Asms, n) - } -} - // checkdupfields emits errors for duplicately named fields or methods in // a list of struct or interface types. func checkdupfields(what string, fss ...[]*types.Field) { @@ -170,15 +90,15 @@ type funcStackEnt struct { dclcontext ir.Class } -func declareParams(ctxt ir.Class, l []*ir.Field) []*types.Field { +func declareParams(fn *ir.Func, ctxt ir.Class, l []*ir.Field) []*types.Field { fields := make([]*types.Field, len(l)) for i, n := range l { - fields[i] = declareParam(ctxt, i, n) + fields[i] = declareParam(fn, ctxt, i, n) } return fields } -func declareParam(ctxt ir.Class, i int, param *ir.Field) *types.Field { +func declareParam(fn *ir.Func, ctxt ir.Class, i int, param *ir.Field) *types.Field { f := types.NewField(param.Pos, param.Sym, param.Type) f.SetIsDDD(param.IsDDD) @@ -202,7 +122,10 @@ func declareParam(ctxt ir.Class, i int, param *ir.Field) *types.Field { name := ir.NewNameAt(param.Pos, sym) name.SetType(f.Type) name.SetTypecheck(1) - Declare(name, ctxt) + + name.Class = ctxt + fn.Dcl = append(fn.Dcl, name) + name.Curfn = fn f.Nname = name } -- 2.50.0