From 4e336b8e1ee9bde0d8b797ea0131f6859361e368 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Tue, 15 Aug 2023 14:02:37 -0700 Subject: [PATCH] cmd/compile/internal/escape: flip transient to !persists I want to add more location properties (e.g., to track indirect stores and calls), and it's easier to reason about them if they're all consistent that "true" means more consequences than less. Change-Id: I3f8674bb11877ba33082a0f5f7d8e55ad6d7a4cc Reviewed-on: https://go-review.googlesource.com/c/go/+/520257 Run-TryBot: Matthew Dempsky TryBot-Result: Gopher Robot Auto-Submit: Matthew Dempsky Reviewed-by: Cuong Manh Le Reviewed-by: Than McIntosh --- src/cmd/compile/internal/escape/call.go | 2 +- src/cmd/compile/internal/escape/escape.go | 8 ++++---- src/cmd/compile/internal/escape/expr.go | 4 ++-- src/cmd/compile/internal/escape/graph.go | 16 ++++++++-------- src/cmd/compile/internal/escape/solve.go | 11 +++++------ src/cmd/compile/internal/escape/stmt.go | 5 +++-- 6 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/cmd/compile/internal/escape/call.go b/src/cmd/compile/internal/escape/call.go index bfba651cef..2dd3fe8375 100644 --- a/src/cmd/compile/internal/escape/call.go +++ b/src/cmd/compile/internal/escape/call.go @@ -430,7 +430,7 @@ func (e *escape) copyExpr(pos src.XPos, expr ir.Node, init *ir.Nodes, fn *ir.Fun init.Append(stmts...) if analyze { - e.newLoc(tmp, false) + e.newLoc(tmp, true) e.stmts(stmts) } diff --git a/src/cmd/compile/internal/escape/escape.go b/src/cmd/compile/internal/escape/escape.go index be02f1b6b0..ea9287712c 100644 --- a/src/cmd/compile/internal/escape/escape.go +++ b/src/cmd/compile/internal/escape/escape.go @@ -130,7 +130,7 @@ func Batch(fns []*ir.Func, recursive bool) { var b batch b.heapLoc.escapes = true - b.blankLoc.transient = true + b.heapLoc.persists = true // Construct data-flow graph from syntax trees. for _, fn := range fns { @@ -185,14 +185,14 @@ func (b *batch) initFunc(fn *ir.Func) { // Allocate locations for local variables. for _, n := range fn.Dcl { - e.newLoc(n, false) + e.newLoc(n, true) } // Also for hidden parameters (e.g., the ".this" parameter to a // method value wrapper). if fn.OClosure == nil { for _, n := range fn.ClosureVars { - e.newLoc(n.Canonical(), false) + e.newLoc(n.Canonical(), true) } } @@ -324,7 +324,7 @@ func (b *batch) finish(fns []*ir.Func) { base.WarnfAt(n.Pos(), "%v does not escape", n) } n.SetEsc(ir.EscNone) - if loc.transient { + if !loc.persists { switch n.Op() { case ir.OCLOSURE: n := n.(*ir.ClosureExpr) diff --git a/src/cmd/compile/internal/escape/expr.go b/src/cmd/compile/internal/escape/expr.go index d3f963d40a..81c0528f1b 100644 --- a/src/cmd/compile/internal/escape/expr.go +++ b/src/cmd/compile/internal/escape/expr.go @@ -250,7 +250,7 @@ func (e *escape) exprSkipInit(k hole, n ir.Node) { // analysis (happens for escape analysis called // from reflectdata.methodWrapper) if n.Op() == ir.ONAME && n.Opt == nil { - e.with(fn).newLoc(n, false) + e.with(fn).newLoc(n, true) } } e.walkFunc(fn) @@ -335,7 +335,7 @@ func (e *escape) discards(l ir.Nodes) { // its address to k, and returns a hole that flows values to it. It's // intended for use with most expressions that allocate storage. func (e *escape) spill(k hole, n ir.Node) hole { - loc := e.newLoc(n, true) + loc := e.newLoc(n, false) e.flow(k.addr(n, "spill"), loc) return loc.asHole() } diff --git a/src/cmd/compile/internal/escape/graph.go b/src/cmd/compile/internal/escape/graph.go index fc18f7715f..ad97b7c28c 100644 --- a/src/cmd/compile/internal/escape/graph.go +++ b/src/cmd/compile/internal/escape/graph.go @@ -71,10 +71,10 @@ type location struct { // allocated. escapes bool - // transient reports whether the represented expression's - // address does not outlive the statement; that is, whether - // its storage can be immediately reused. - transient bool + // persists reports whether the represented expression's address + // outlives the statement; that is, whether its storage cannot be + // immediately reused. + persists bool // paramEsc records the represented parameter's leak set. paramEsc leaks @@ -213,7 +213,7 @@ func (b *batch) oldLoc(n *ir.Name) *location { return n.Canonical().Opt.(*location) } -func (e *escape) newLoc(n ir.Node, transient bool) *location { +func (e *escape) newLoc(n ir.Node, persists bool) *location { if e.curfn == nil { base.Fatalf("e.curfn isn't set") } @@ -230,7 +230,7 @@ func (e *escape) newLoc(n ir.Node, transient bool) *location { n: n, curfn: e.curfn, loopDepth: e.loopDepth, - transient: transient, + persists: persists, } e.allLocs = append(e.allLocs, loc) if n != nil { @@ -265,7 +265,7 @@ func (e *escape) teeHole(ks ...hole) hole { // Given holes "l1 = _", "l2 = **_", "l3 = *_", ..., create a // new temporary location ltmp, wire it into place, and return // a hole for "ltmp = _". - loc := e.newLoc(nil, true) + loc := e.newLoc(nil, false) for _, k := range ks { // N.B., "p = &q" and "p = &tmp; tmp = q" are not // semantically equivalent. To combine holes like "l1 @@ -285,7 +285,7 @@ func (e *escape) teeHole(ks ...hole) hole { // Its main effect is to prevent immediate reuse of temporary // variables introduced during Order. func (e *escape) later(k hole) hole { - loc := e.newLoc(nil, false) + loc := e.newLoc(nil, true) e.flow(k, loc) return loc.asHole() } diff --git a/src/cmd/compile/internal/escape/solve.go b/src/cmd/compile/internal/escape/solve.go index a2d3b6d2fd..2856c9c131 100644 --- a/src/cmd/compile/internal/escape/solve.go +++ b/src/cmd/compile/internal/escape/solve.go @@ -21,7 +21,7 @@ func (b *batch) walkAll() { // // We walk once from each location (including the heap), and // then re-enqueue each location on its transition from - // transient->!transient and !escapes->escapes, which can each + // !persists->persists and !escapes->escapes, which can each // happen at most once. So we take Θ(len(e.allLocs)) walks. // LIFO queue, has enough room for e.allLocs and e.heapLoc. @@ -77,11 +77,10 @@ func (b *batch) walkOne(root *location, walkgen uint32, enqueue func(*location)) // derefs at 0. derefs = 0 - // If l's address flows to a non-transient - // location, then l can't be transiently - // allocated. - if !root.transient && l.transient { - l.transient = false + // If l's address flows to a persistent location, then l needs + // to persist too. + if root.persists && !l.persists { + l.persists = true enqueue(l) } } diff --git a/src/cmd/compile/internal/escape/stmt.go b/src/cmd/compile/internal/escape/stmt.go index 4752e561e2..cb2b72fa6b 100644 --- a/src/cmd/compile/internal/escape/stmt.go +++ b/src/cmd/compile/internal/escape/stmt.go @@ -92,8 +92,9 @@ func (e *escape) stmt(n ir.Node) { n := n.(*ir.RangeStmt) base.Assert(!n.DistinctVars) // Should all be rewritten before escape analysis - // X is evaluated outside the loop. - tmp := e.newLoc(nil, false) + // X is evaluated outside the loop and persists until the loop + // terminates. + tmp := e.newLoc(nil, true) e.expr(tmp.asHole(), n.X) e.loopDepth++ -- 2.50.0