Skip to content

Commit

Permalink
cmd/compile/internal/ir: drop Inl.Body
Browse files Browse the repository at this point in the history
We don't actually depend on Inl.Body anywhere, except it implicitly
serves to indicate whether Inl.Dcl has been populated. So replace it
with a boolean so we don't need to keep a useless copy of every
inlinable function body in memory.

While here, also add some Fatalfs to make sure there are no unused
local variables. The unified frontend now omits unreachable code
during export data writing, so there shouldn't be unused local
variables.

Also, since unified IR uses the same code/data to construct the
original function as inlined and/or imported functions, the Dcl list
should always be the same, which addresses the real root issue (i.e.,
that export/import could skew the Dcl lists).

Change-Id: I6e3435f3a0352f6efbae787344006efac1891e84
Reviewed-on: https://go-review.googlesource.com/c/go/+/523315
Auto-Submit: Matthew Dempsky <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Reviewed-by: Cuong Manh Le <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
  • Loading branch information
mdempsky authored and gopherbot committed Aug 29, 2023
1 parent ad74bc4 commit 08bf6d1
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 55 deletions.
53 changes: 8 additions & 45 deletions src/cmd/compile/internal/inline/inl.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func CanInline(fn *ir.Func, profile *pgo.Profile) {
// when creating the "Inline.Dcl" field below; to accomplish this,
// the hairyVisitor below builds up a map of used/referenced
// locals, and we use this map to produce a pruned Inline.Dcl
// list. See issue 25249 for more context.
// list. See issue 25459 for more context.

visitor := hairyVisitor{
curFunc: fn,
Expand All @@ -354,15 +354,15 @@ func CanInline(fn *ir.Func, profile *pgo.Profile) {
}

n.Func.Inl = &ir.Inline{
Cost: budget - visitor.budget,
Dcl: pruneUnusedAutos(n.Defn.(*ir.Func).Dcl, &visitor),
Body: inlcopylist(fn.Body),
Cost: budget - visitor.budget,
Dcl: pruneUnusedAutos(n.Func.Dcl, &visitor),
HaveDcl: true,

CanDelayResults: canDelayResults(fn),
}

if base.Flag.LowerM > 1 {
fmt.Printf("%v: can inline %v with cost %d as: %v { %v }\n", ir.Line(fn), n, budget-visitor.budget, fn.Type(), ir.Nodes(n.Func.Inl.Body))
fmt.Printf("%v: can inline %v with cost %d as: %v { %v }\n", ir.Line(fn), n, budget-visitor.budget, fn.Type(), ir.Nodes(fn.Body))
} else if base.Flag.LowerM != 0 {
fmt.Printf("%v: can inline %v\n", ir.Line(fn), n)
}
Expand Down Expand Up @@ -789,46 +789,6 @@ func isBigFunc(fn *ir.Func) bool {
})
}

// inlcopylist (together with inlcopy) recursively copies a list of nodes, except
// that it keeps the same ONAME, OTYPE, and OLITERAL nodes. It is used for copying
// the body and dcls of an inlineable function.
func inlcopylist(ll []ir.Node) []ir.Node {
s := make([]ir.Node, len(ll))
for i, n := range ll {
s[i] = inlcopy(n)
}
return s
}

// inlcopy is like DeepCopy(), but does extra work to copy closures.
func inlcopy(n ir.Node) ir.Node {
var edit func(ir.Node) ir.Node
edit = func(x ir.Node) ir.Node {
switch x.Op() {
case ir.ONAME, ir.OTYPE, ir.OLITERAL, ir.ONIL:
return x
}
m := ir.Copy(x)
ir.EditChildren(m, edit)
if x.Op() == ir.OCLOSURE {
x := x.(*ir.ClosureExpr)
// Need to save/duplicate x.Func.Nname,
// x.Func.Nname.Ntype, x.Func.Dcl, x.Func.ClosureVars, and
// x.Func.Body for iexport and local inlining.
oldfn := x.Func
newfn := ir.NewFunc(oldfn.Pos(), oldfn.Nname.Pos(), oldfn.Nname.Sym(), oldfn.Nname.Type())
m.(*ir.ClosureExpr).Func = newfn
// XXX OK to share fn.Type() ??
newfn.Body = inlcopylist(oldfn.Body)
// Make shallow copy of the Dcl and ClosureVar slices
newfn.Dcl = append([]*ir.Name(nil), oldfn.Dcl...)
newfn.ClosureVars = append([]*ir.Name(nil), oldfn.ClosureVars...)
}
return m
}
return edit(n)
}

// InlineCalls/inlnode walks fn's statements and expressions and substitutes any
// calls made to inlineable functions. This is the external entry point.
func InlineCalls(fn *ir.Func, profile *pgo.Profile) {
Expand Down Expand Up @@ -1208,6 +1168,9 @@ func pruneUnusedAutos(ll []*ir.Name, vis *hairyVisitor) []*ir.Name {
for _, n := range ll {
if n.Class == ir.PAUTO {
if !vis.usedLocals.Has(n) {
// TODO(mdempsky): Simplify code after confident that this
// never happens anymore.
base.FatalfAt(n.Pos(), "unused auto: %v", n)
continue
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/cmd/compile/internal/ir/func.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,12 @@ func (f *Func) LinksymABI(abi obj.ABI) *obj.LSym { return f.Nname.LinksymABI(abi
type Inline struct {
Cost int32 // heuristic cost of inlining this function

// Copies of Func.Dcl and Func.Body for use during inlining. Copies are
// needed because the function's dcl/body may be changed by later compiler
// transformations. These fields are also populated when a function from
// another package is imported.
Dcl []*Name
Body []Node
// Copy of Func.Dcl for use during inlining. This copy is needed
// because the function's Dcl may change from later compiler
// transformations. This field is also populated when a function
// from another package is imported and inlined.
Dcl []*Name
HaveDcl bool // whether we've loaded Dcl

// CanDelayResults reports whether it's safe for the inliner to delay
// initializing the result parameters until immediately before the
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/noder/linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func (l *linker) exportBody(obj *ir.Name, local bool) {
//
// TODO(mdempsky): Reimplement the reachable method crawling logic
// from typecheck/crawler.go.
exportBody := local || fn.Inl.Body != nil
exportBody := local || fn.Inl.HaveDcl
if !exportBody {
return
}
Expand Down
10 changes: 7 additions & 3 deletions src/cmd/compile/internal/noder/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -3463,7 +3463,7 @@ func unifiedInlineCall(callerfn *ir.Func, call *ir.CallExpr, fn *ir.Func, inlInd
base.FatalfAt(call.Pos(), "cannot inline call to %v: missing inline body", fn)
}

if fn.Inl.Body == nil {
if !fn.Inl.HaveDcl {
expandInline(fn, pri)
}

Expand Down Expand Up @@ -3631,7 +3631,7 @@ func (r *reader) inlReturn(ret *ir.ReturnStmt) *ir.BlockStmt {
}

// expandInline reads in an extra copy of IR to populate
// fn.Inl.{Dcl,Body}.
// fn.Inl.Dcl.
func expandInline(fn *ir.Func, pri pkgReaderIndex) {
// TODO(mdempsky): Remove this function. It's currently needed by
// dwarfgen/dwarf.go:preInliningDcls, which requires fn.Inl.Dcl to
Expand Down Expand Up @@ -3659,9 +3659,13 @@ func expandInline(fn *ir.Func, pri pkgReaderIndex) {
if name.Class != ir.PAUTO || used.Has(name) {
name.Curfn = fn
fn.Inl.Dcl = append(fn.Inl.Dcl, name)
} else {
// TODO(mdempsky): Simplify code after confident that this never
// happens anymore.
base.FatalfAt(name.Pos(), "unused auto: %v", name)
}
}
fn.Inl.Body = tmpfn.Body
fn.Inl.HaveDcl = true

// Double check that we didn't change fn.Dcl by accident.
assert(fndcls == len(fn.Dcl))
Expand Down

0 comments on commit 08bf6d1

Please sign in to comment.