]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: standardize on outer-to-inner for pos lists
authorRuss Cox <rsc@golang.org>
Tue, 9 May 2023 00:23:40 +0000 (20:23 -0400)
committerRuss Cox <rsc@golang.org>
Tue, 9 May 2023 16:07:01 +0000 (16:07 +0000)
The call sites that cared all reversed inner-to-outer to outer-to-inner already.
The ones that didn't care left it alone. No one explicitly wanted inner-to-outer.
Also change to a callback-based interface, so that call sites aren't required
to accumulate the results in a slice (the main reason for that before was to
reverse the slice!).

There were three places where these lists were printed:

1. -d=ssa/genssa/dump, explicitly reversing to outer-to-inner
2. node dumps like -W, leaving the default inner-to-outer
3. file positions for HashDebugs, explicitly reversing to outer-to-inner

It makes no sense that (1) and (2) would differ. The reason they do is that
the code for (2) was too lazy to bother to fix it to be the right way.

Consider this program:

package p

func f() {
g()
}

func g() {
println()
}

Both before and after this change, the ssa dump for f looks like:

# x.go:3
        00000 (3)  TEXT <unlinkable>.f(SB), ABIInternal
        00001 (3)  FUNCDATA $0, gclocals·g2BeySu+wFnoycgXfElmcg==(SB)
        00002 (3)  FUNCDATA $1, gclocals·g2BeySu+wFnoycgXfElmcg==(SB)
 v4     00003 (-4)  XCHGL AX, AX
# x.go:4
# x.go:8
 v5     00004 (+8)  PCDATA $1, $0
 v5     00005 (+8)  CALL runtime.printlock(SB)
 v7     00006 (-8)  CALL runtime.printnl(SB)
 v9     00007 (-8)  CALL runtime.printunlock(SB)
# x.go:5
 b2     00008 (5)  RET
        00009 (?)  END

Note # x.go:4 (f) then # x.go:8 (g, called from f) between v4 and v5.

The -W node dumps used the opposite order:

before walk f
.   AS2 Def tc(1) # x.go:4:3
.   INLMARK # +x.go:4:3
.   PRINTN tc(1) # x.go:8:9,x.go:4:3
.   LABEL p..i0 # x.go:4:3

Now they match the ssa dump order, and they use spaces as separators,
to avoid potential problems with commas in some editors.

before walk f
.   AS2 Def tc(1) # x.go:4:3
.   INLMARK # +x.go:4:3
.   PRINTN tc(1) # x.go:4:3 x.go:8:9
.   LABEL p..i0 # x.go:4:3

I'm unaware of any argument for the old order other than it was easier
to compute without allocation. The new code uses recursion to reverse
the order without allocation.

Now that the callers get the results outer-to-inner, most don't need
any slices at all.

This change is particularly important for HashDebug, which had been
using a locked temporary slice to walk the inline stack without allocation.
Now the temporary slice is gone.

Change-Id: I5cb6d76b2f950db67b248acc928e47a0460569f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/493735
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>

src/cmd/compile/internal/base/hashdebug.go
src/cmd/compile/internal/ir/fmt.go
src/cmd/compile/internal/logopt/log_opts.go
src/cmd/compile/internal/ssagen/ssa.go
src/cmd/internal/obj/inl.go
src/cmd/internal/obj/util.go

index 0d0b3f31232c495601ee7547be9cace7a50b82a2..5492d9cda2eb265b45a05fb81be9f72a4201c6aa 100644 (file)
@@ -351,24 +351,24 @@ func (d *HashDebug) debugHashMatchPos(ctxt *obj.Link, pos src.XPos) bool {
 // bytesForPos renders a position, including inlining, into d.bytesTmp
 // and returns the byte array.  d.mu must be locked.
 func (d *HashDebug) bytesForPos(ctxt *obj.Link, pos src.XPos) []byte {
-       d.posTmp = ctxt.AllPos(pos, d.posTmp)
-       // Reverse posTmp to put outermost first.
        b := &d.bytesTmp
        b.Reset()
-       start := len(d.posTmp) - 1
-       if d.inlineSuffixOnly {
-               start = 0
-       }
-       for i := start; i >= 0; i-- {
-               p := &d.posTmp[i]
+       format := func(p src.Pos) {
                f := p.Filename()
                if d.fileSuffixOnly {
                        f = filepath.Base(f)
                }
                fmt.Fprintf(b, "%s:%d:%d", f, p.Line(), p.Col())
-               if i != 0 {
-                       b.WriteByte(';')
-               }
+       }
+       if d.inlineSuffixOnly {
+               format(ctxt.InnermostPos(pos))
+       } else {
+               ctxt.AllPos(pos, func(p src.Pos) {
+                       if b.Len() > 0 {
+                               b.WriteByte(';')
+                       }
+                       format(p)
+               })
        }
        return b.Bytes()
 }
index dcb8988b66e2d6137c544ffcf332a4159bd57ce2..a9cf716dffa1581980ac1fc8334cb1fecca4ea8e 100644 (file)
@@ -1085,15 +1085,15 @@ func dumpNodeHeader(w io.Writer, n Node) {
                case src.PosIsStmt:
                        fmt.Fprint(w, "+")
                }
-               for i, pos := range base.Ctxt.AllPos(n.Pos(), nil) {
-                       if i > 0 {
-                               fmt.Fprint(w, ",")
-                       }
+               sep := ""
+               base.Ctxt.AllPos(n.Pos(), func(pos src.Pos) {
+                       fmt.Fprint(w, sep)
+                       sep = " "
                        // TODO(mdempsky): Print line pragma details too.
                        file := filepath.Base(pos.Filename())
                        // Note: this output will be parsed by ssa/html.go:(*HTMLWriter).WriteAST. Keep in sync.
                        fmt.Fprintf(w, "%s:%d:%d", file, pos.Line(), pos.Col())
-               }
+               })
        }
 }
 
index f74be6a63cbdaed014ce1776cba9b928dd2e23b3..b731e5593833267d0a05144d997a73214badf0ee 100644 (file)
@@ -532,12 +532,9 @@ func appendInlinedPos(posTmp, lastTmp []src.Pos, diagnostic *Diagnostic) {
 // parsePos expands a src.XPos into a slice of src.Pos, with the outermost first.
 // It returns the slice, and the outermost.
 func parsePos(ctxt *obj.Link, pos src.XPos, posTmp []src.Pos) ([]src.Pos, src.Pos) {
-       posTmp = ctxt.AllPos(pos, posTmp)
-       // Reverse posTmp to put outermost first.
-       l := len(posTmp)
-       for i := 0; i < l/2; i++ {
-               posTmp[i], posTmp[l-i-1] = posTmp[l-i-1], posTmp[i]
-       }
-       p0 := posTmp[0]
-       return posTmp, p0
+       posTmp = posTmp[:0]
+       ctxt.AllPos(pos, func(p src.Pos) {
+               posTmp = append(posTmp, p)
+       })
+       return posTmp, posTmp[0]
 }
index 37b5a26d5cbe0a10cd09767cdc80fd88a5acbf8d..a037b7494d517d7e8c200c92ca0dcef386ce54f2 100644 (file)
@@ -7331,7 +7331,7 @@ func genssa(f *ssa.Func, pp *objw.Progs) {
                fi := f.DumpFileForPhase("genssa")
                if fi != nil {
 
-                       // inliningDiffers if any filename changes or if any line number except the innermost (index 0) changes.
+                       // inliningDiffers if any filename changes or if any line number except the innermost (last index) changes.
                        inliningDiffers := func(a, b []src.Pos) bool {
                                if len(a) != len(b) {
                                        return true
@@ -7340,7 +7340,7 @@ func genssa(f *ssa.Func, pp *objw.Progs) {
                                        if a[i].Filename() != b[i].Filename() {
                                                return true
                                        }
-                                       if i > 0 && a[i].Line() != b[i].Line() {
+                                       if i != len(a)-1 && a[i].Line() != b[i].Line() {
                                                return true
                                        }
                                }
@@ -7352,10 +7352,10 @@ func genssa(f *ssa.Func, pp *objw.Progs) {
 
                        for p := pp.Text; p != nil; p = p.Link {
                                if p.Pos.IsKnown() {
-                                       allPos = p.AllPos(allPos)
+                                       allPos = allPos[:0]
+                                       p.Ctxt.AllPos(p.Pos, func(pos src.Pos) { allPos = append(allPos, pos) })
                                        if inliningDiffers(allPos, allPosOld) {
-                                               for i := len(allPos) - 1; i >= 0; i-- {
-                                                       pos := allPos[i]
+                                               for _, pos := range allPos {
                                                        fmt.Fprintf(fi, "# %s:%d\n", pos.Filename(), pos.Line())
                                                }
                                                allPos, allPosOld = allPosOld, allPos // swap, not copy, so that they do not share slice storage.
index 934f1c26571136a8bb739b60d98b4161712abb33..7a22eb1efdc2adae0f9f35d5450453d1130c8914 100644 (file)
@@ -108,20 +108,21 @@ func (ctxt *Link) InnermostPos(xpos src.XPos) src.Pos {
        return ctxt.PosTable.Pos(xpos)
 }
 
-// AllPos returns a slice of the positions inlined at xpos, from
-// innermost (index zero) to outermost.  To avoid allocation
-// the input slice is truncated, and used for the result, extended
-// as necessary.
-func (ctxt *Link) AllPos(xpos src.XPos, result []src.Pos) []src.Pos {
+// AllPos invokes do with every position in the inlining call stack for xpos,
+// from outermost to innermost. That is, xpos corresponds to f inlining g inlining h,
+// AllPos invokes do with the position in f, then the position in g, then the position in h.
+func (ctxt *Link) AllPos(xpos src.XPos, do func(src.Pos)) {
        pos := ctxt.InnermostPos(xpos)
-       result = result[:0]
-       result = append(result, ctxt.PosTable.Pos(xpos))
-       for ix := pos.Base().InliningIndex(); ix >= 0; {
+       ctxt.forAllPos(pos.Base().InliningIndex(), do)
+       do(ctxt.PosTable.Pos(xpos))
+}
+
+func (ctxt *Link) forAllPos(ix int, do func(src.Pos)) {
+       if ix >= 0 {
                call := ctxt.InlTree.nodes[ix]
-               ix = call.Parent
-               result = append(result, ctxt.PosTable.Pos(call.Pos))
+               ctxt.forAllPos(call.Parent, do)
+               do(ctxt.PosTable.Pos(call.Pos))
        }
-       return result
 }
 
 func dumpInlTree(ctxt *Link, tree InlTree) {
index 14b09f43d4a8c0794fce64d7c7f034545e9c0706..3a071c21d48725d863cf920d783a98cf01faa844 100644 (file)
@@ -6,7 +6,6 @@ package obj
 
 import (
        "bytes"
-       "cmd/internal/src"
        "fmt"
        "internal/abi"
        "internal/buildcfg"
@@ -48,10 +47,6 @@ func (p *Prog) InnermostFilename() string {
        return pos.Filename()
 }
 
-func (p *Prog) AllPos(result []src.Pos) []src.Pos {
-       return p.Ctxt.AllPos(p.Pos, result)
-}
-
 var armCondCode = []string{
        ".EQ",
        ".NE",