]> Cypherpunks repositories - gostls13.git/commitdiff
[dev.regabi] cmd/compile: cleanup label handling
authorRuss Cox <rsc@golang.org>
Sat, 28 Nov 2020 04:52:37 +0000 (23:52 -0500)
committerRuss Cox <rsc@golang.org>
Mon, 30 Nov 2020 18:33:47 +0000 (18:33 +0000)
- The use of a label's Name.Defn to point at the named for/select/switch
  means that any rewrite of the for/select/switch must overwrite the original
  or else the pointer will dangle. Remove that pointer by adding the label
  name directly to the for/select/switch representation instead.

- The only uses of a label's Sym.Label were ephemeral values during
  markbreak and escape analysis. Use a map for each. Although in general
  we are not going to replace all computed fields with maps (too slow),
  the one in markbreak is only for labeled for/select/switch, and the one
  in escape is for all labels, but even so, labels are fairly rare.

In theory this cleanup should make it easy to allow labeled for/select/switch
in inlined bodies, but this CL does not attempt that. It's only concerned
with cleanup to enable a new Node representation.

Passes buildall w/ toolstash -cmp.

Change-Id: I7e36ee98d2ea40dbae94e6722d585f007b7afcfa
Reviewed-on: https://go-review.googlesource.com/c/go/+/274086
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
src/cmd/compile/internal/gc/escape.go
src/cmd/compile/internal/gc/inl.go
src/cmd/compile/internal/gc/noder.go
src/cmd/compile/internal/gc/ssa.go
src/cmd/compile/internal/gc/subr.go
src/cmd/compile/internal/gc/typecheck.go
src/cmd/compile/internal/types/sizeof_test.go
src/cmd/compile/internal/types/sym.go

index 783bc8c41dd1283ec799ea80e32321c1b781dac8..34f52c743abb8b42ceb17fa957b02e7a036104d8 100644 (file)
@@ -85,6 +85,7 @@ import (
 
 type Escape struct {
        allLocs []*EscLocation
+       labels  map[*types.Sym]labelState // known labels
 
        curfn ir.Node
 
@@ -229,13 +230,16 @@ func (e *Escape) walkFunc(fn ir.Node) {
        ir.InspectList(fn.Body(), func(n ir.Node) bool {
                switch n.Op() {
                case ir.OLABEL:
-                       n.Sym().Label = nonlooping
+                       if e.labels == nil {
+                               e.labels = make(map[*types.Sym]labelState)
+                       }
+                       e.labels[n.Sym()] = nonlooping
 
                case ir.OGOTO:
                        // If we visited the label before the goto,
                        // then this is a looping label.
-                       if n.Sym().Label == nonlooping {
-                               n.Sym().Label = looping
+                       if e.labels[n.Sym()] == nonlooping {
+                               e.labels[n.Sym()] = looping
                        }
                }
 
@@ -245,6 +249,10 @@ func (e *Escape) walkFunc(fn ir.Node) {
        e.curfn = fn
        e.loopDepth = 1
        e.block(fn.Body())
+
+       if len(e.labels) != 0 {
+               base.FatalfAt(fn.Pos(), "leftover labels after walkFunc")
+       }
 }
 
 // Below we implement the methods for walking the AST and recording
@@ -310,7 +318,7 @@ func (e *Escape) stmt(n ir.Node) {
                }
 
        case ir.OLABEL:
-               switch ir.AsNode(n.Sym().Label) {
+               switch e.labels[n.Sym()] {
                case nonlooping:
                        if base.Flag.LowerM > 2 {
                                fmt.Printf("%v:%v non-looping label\n", base.FmtPos(base.Pos), n)
@@ -323,7 +331,7 @@ func (e *Escape) stmt(n ir.Node) {
                default:
                        base.Fatalf("label missing tag")
                }
-               n.Sym().Label = nil
+               delete(e.labels, n.Sym())
 
        case ir.OIF:
                e.discard(n.Left())
@@ -1615,11 +1623,11 @@ func funcSym(fn ir.Node) *types.Sym {
 }
 
 // Mark labels that have no backjumps to them as not increasing e.loopdepth.
-// Walk hasn't generated (goto|label).Left.Sym.Label yet, so we'll cheat
-// and set it to one of the following two. Then in esc we'll clear it again.
-var (
-       looping    = ir.Nod(ir.OXXX, nil, nil)
-       nonlooping = ir.Nod(ir.OXXX, nil, nil)
+type labelState int
+
+const (
+       looping labelState = 1 + iota
+       nonlooping
 )
 
 func isSliceSelfAssign(dst, src ir.Node) bool {
index 6310762c1f4d2cc5d9b14ff3eb349a55df1c261b..d43d0d06af4595a5de53a49fdeb68e94e5a81269 100644 (file)
@@ -405,16 +405,16 @@ func (v *hairyVisitor) visit(n ir.Node) bool {
                // These nodes don't produce code; omit from inlining budget.
                return false
 
-       case ir.OLABEL:
-               // TODO(mdempsky): Add support for inlining labeled control statements.
-               if labeledControl(n) != nil {
+       case ir.OFOR, ir.OFORUNTIL, ir.OSWITCH:
+               // ORANGE, OSELECT in "unhandled" above
+               if n.Sym() != nil {
                        v.reason = "labeled control"
                        return true
                }
 
        case ir.OBREAK, ir.OCONTINUE:
                if n.Sym() != nil {
-                       // Should have short-circuited due to labeledControl above.
+                       // Should have short-circuited due to labeled control error above.
                        base.Fatalf("unexpected labeled break/continue: %v", n)
                }
 
index 950d50904795d05942cc15af5ca2579c8bb19dfa..ecd50b87f69ddcf4f540f805a19808b006d4a616 100644 (file)
@@ -1302,14 +1302,19 @@ func (p *noder) commClauses(clauses []*syntax.CommClause, rbrace syntax.Pos) []i
 }
 
 func (p *noder) labeledStmt(label *syntax.LabeledStmt, fallOK bool) ir.Node {
-       lhs := p.nodSym(label, ir.OLABEL, nil, p.name(label.Label))
+       sym := p.name(label.Label)
+       lhs := p.nodSym(label, ir.OLABEL, nil, sym)
 
        var ls ir.Node
        if label.Stmt != nil { // TODO(mdempsky): Should always be present.
                ls = p.stmtFall(label.Stmt, fallOK)
+               switch label.Stmt.(type) {
+               case *syntax.ForStmt, *syntax.SwitchStmt, *syntax.SelectStmt:
+                       // Attach label directly to control statement too.
+                       ls.SetSym(sym)
+               }
        }
 
-       lhs.Name().Defn = ls
        l := []ir.Node{lhs}
        if ls != nil {
                if ls.Op() == ir.OBLOCK && ls.Init().Len() == 0 {
index cb73532b48524ebb95e5fe8f20c2a0a927e493fa..bcc126f82e5210a3eca19cd3c1ccd07359946d7f 100644 (file)
@@ -356,7 +356,6 @@ func buildssa(fn ir.Node, worker int) *ssa.Func {
 
        // Allocate starting values
        s.labels = map[string]*ssaLabel{}
-       s.labeledNodes = map[ir.Node]*ssaLabel{}
        s.fwdVars = map[ir.Node]*ssa.Value{}
        s.startmem = s.entryNewValue0(ssa.OpInitMem, types.TypeMem)
 
@@ -596,9 +595,8 @@ type state struct {
        // Node for function
        curfn ir.Node
 
-       // labels and labeled control flow nodes (OFOR, OFORUNTIL, OSWITCH, OSELECT) in f
-       labels       map[string]*ssaLabel
-       labeledNodes map[ir.Node]*ssaLabel
+       // labels in f
+       labels map[string]*ssaLabel
 
        // unlabeled break and continue statement tracking
        breakTo    *ssa.Block // current target for plain break statement
@@ -1169,11 +1167,6 @@ func (s *state) stmt(n ir.Node) {
                sym := n.Sym()
                lab := s.label(sym)
 
-               // Associate label with its control flow node, if any
-               if ctl := labeledControl(n); ctl != nil {
-                       s.labeledNodes[ctl] = lab
-               }
-
                // The label might already have a target block via a goto.
                if lab.target == nil {
                        lab.target = s.f.NewBlock(ssa.BlockPlain)
@@ -1431,9 +1424,10 @@ func (s *state) stmt(n ir.Node) {
                prevBreak := s.breakTo
                s.continueTo = bIncr
                s.breakTo = bEnd
-               lab := s.labeledNodes[n]
-               if lab != nil {
+               var lab *ssaLabel
+               if sym := n.Sym(); sym != nil {
                        // labeled for loop
+                       lab = s.label(sym)
                        lab.continueTarget = bIncr
                        lab.breakTarget = bEnd
                }
@@ -1489,9 +1483,10 @@ func (s *state) stmt(n ir.Node) {
 
                prevBreak := s.breakTo
                s.breakTo = bEnd
-               lab := s.labeledNodes[n]
-               if lab != nil {
+               var lab *ssaLabel
+               if sym := n.Sym(); sym != nil {
                        // labeled
+                       lab = s.label(sym)
                        lab.breakTarget = bEnd
                }
 
index fcda219737f5ff92f63b49f30316761ee983380a..d174ebd582b5c0bd2a74e263ac969fd5f7cdd9c5 100644 (file)
@@ -582,23 +582,6 @@ func backingArrayPtrLen(n ir.Node) (ptr, len ir.Node) {
        return ptr, len
 }
 
-// labeledControl returns the control flow Node (for, switch, select)
-// associated with the label n, if any.
-func labeledControl(n ir.Node) ir.Node {
-       if n.Op() != ir.OLABEL {
-               base.Fatalf("labeledControl %v", n.Op())
-       }
-       ctl := n.Name().Defn
-       if ctl == nil {
-               return nil
-       }
-       switch ctl.Op() {
-       case ir.OFOR, ir.OFORUNTIL, ir.OSWITCH, ir.OSELECT:
-               return ctl
-       }
-       return nil
-}
-
 func syslook(name string) ir.Node {
        s := Runtimepkg.Lookup(name)
        if s == nil || s.Def == nil {
index 4e2f205312110dc5f4e0c706b3086dbc527fd060..ede3778184bbbb41b240511d678775a07fcbe2fc 100644 (file)
@@ -3759,7 +3759,7 @@ func checkmake(t *types.Type, arg string, np *ir.Node) bool {
        return true
 }
 
-func markbreak(n ir.Node, implicit ir.Node) {
+func markbreak(labels *map[*types.Sym]ir.Node, n ir.Node, implicit ir.Node) {
        if n == nil {
                return
        }
@@ -3771,43 +3771,35 @@ func markbreak(n ir.Node, implicit ir.Node) {
                                implicit.SetHasBreak(true)
                        }
                } else {
-                       lab := ir.AsNode(n.Sym().Label)
-                       if lab != nil {
+                       if lab := (*labels)[n.Sym()]; lab != nil {
                                lab.SetHasBreak(true)
                        }
                }
        case ir.OFOR, ir.OFORUNTIL, ir.OSWITCH, ir.OTYPESW, ir.OSELECT, ir.ORANGE:
                implicit = n
+               if sym := n.Sym(); sym != nil {
+                       if *labels == nil {
+                               // Map creation delayed until we need it - most functions don't.
+                               *labels = make(map[*types.Sym]ir.Node)
+                       }
+                       (*labels)[sym] = n
+                       defer delete(*labels, sym)
+               }
                fallthrough
        default:
-               markbreak(n.Left(), implicit)
-               markbreak(n.Right(), implicit)
-               markbreaklist(n.Init(), implicit)
-               markbreaklist(n.Body(), implicit)
-               markbreaklist(n.List(), implicit)
-               markbreaklist(n.Rlist(), implicit)
+               markbreak(labels, n.Left(), implicit)
+               markbreak(labels, n.Right(), implicit)
+               markbreaklist(labels, n.Init(), implicit)
+               markbreaklist(labels, n.Body(), implicit)
+               markbreaklist(labels, n.List(), implicit)
+               markbreaklist(labels, n.Rlist(), implicit)
        }
 }
 
-func markbreaklist(l ir.Nodes, implicit ir.Node) {
+func markbreaklist(labels *map[*types.Sym]ir.Node, l ir.Nodes, implicit ir.Node) {
        s := l.Slice()
        for i := 0; i < len(s); i++ {
-               n := s[i]
-               if n == nil {
-                       continue
-               }
-               if n.Op() == ir.OLABEL && i+1 < len(s) && n.Name().Defn == s[i+1] {
-                       switch n.Name().Defn.Op() {
-                       case ir.OFOR, ir.OFORUNTIL, ir.OSWITCH, ir.OTYPESW, ir.OSELECT, ir.ORANGE:
-                               n.Sym().Label = n.Name().Defn
-                               markbreak(n.Name().Defn, n.Name().Defn)
-                               n.Sym().Label = nil
-                               i++
-                               continue
-                       }
-               }
-
-               markbreak(n, implicit)
+               markbreak(labels, s[i], implicit)
        }
 }
 
@@ -3874,7 +3866,8 @@ func isTermNode(n ir.Node) bool {
 // checkreturn makes sure that fn terminates appropriately.
 func checkreturn(fn ir.Node) {
        if fn.Type().NumResults() != 0 && fn.Body().Len() != 0 {
-               markbreaklist(fn.Body(), nil)
+               var labels map[*types.Sym]ir.Node
+               markbreaklist(&labels, fn.Body(), nil)
                if !isTermNodes(fn.Body()) {
                        base.ErrorfAt(fn.Func().Endlineno, "missing return at end of function")
                }
index 2821d9a3c70541dc90a68e29fbd939d2792047c6..88a2fbba2f5eb8953f866bd6da582234c5c7d25d 100644 (file)
@@ -20,7 +20,7 @@ func TestSizeof(t *testing.T) {
                _32bit uintptr     // size on 32bit platforms
                _64bit uintptr     // size on 64bit platforms
        }{
-               {Sym{}, 60, 104},
+               {Sym{}, 52, 88},
                {Type{}, 56, 96},
                {Map{}, 20, 40},
                {Forward{}, 20, 32},
index 046104d0dcf790bf66e5b3f51cf2d1dc0e42acd6..7272f1f7861e5951e857159739ca4a3227120f07 100644 (file)
@@ -33,13 +33,12 @@ type Sym struct {
        Name string // object name
 
        // saved and restored by dcopy
-       Def        IRNode    // definition: ONAME OTYPE OPACK or OLITERAL
+       Def        IRNode   // definition: ONAME OTYPE OPACK or OLITERAL
        Block      int32    // blocknumber to catch redeclaration
        Lastlineno src.XPos // last declaration for diagnostic
 
        flags   bitset8
-       Label   IRNode // corresponding label (ephemeral)
-       Origpkg *Pkg  // original package for . import
+       Origpkg *Pkg // original package for . import
 }
 
 const (