cmd/compile: refine statement marking in numberlines

1) An empty block is treated as not-a-statement unless its line differs
from at least one of its predecessors (it might make sense to
rearrange branches in predecessors, but that is a different issue).

2) When iterating forward to choose a "good" place for a statement,
actually check that the chosen place is in fact good.

3) Refactor same line and same file into methods on XPos and Pos.

This reduces the failure rate of ssa/stmtlines_test by 7-ish lines.
(And interacts favorably with later debugging CLs.)

Change-Id: Idb7cca7068f6fc9fbfdbe25bc0da15bcfc7b9d4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/188217
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>

diff --git a/src/cmd/compile/internal/ssa/numberlines.go b/src/cmd/compile/internal/ssa/numberlines.go
index 4807da731c3687d255c54b797a3b3384d025c18b..a39e597d59e9ce2dc4064fd9cbb1fd4c8d40abb9 100644 (file)
--- a/src/cmd/compile/internal/ssa/numberlines.go
+++ b/src/cmd/compile/internal/ssa/numberlines.go
@@ -43,16 +43,20 @@ func nextGoodStatementIndex(v *Value, i int, b *Block) int {
        if i >= len(b.Values)-1 {
                return i
        }
-       // Only consider the likely-ephemeral/fragile opcodes expected to vanish in a rewrite.
+       // Skip the likely-ephemeral/fragile opcodes expected to vanish in a rewrite.
        if !isPoorStatementOp(v.Op) {
                return i
        }
        // Look ahead to see what the line number is on the next thing that could be a boundary.
        for j := i + 1; j < len(b.Values); j++ {
-               if b.Values[j].Pos.IsStmt() == src.PosNotStmt { // ignore non-statements
+               u := b.Values[j]
+               if u.Pos.IsStmt() == src.PosNotStmt { // ignore non-statements
                        continue
                }
-               if b.Values[j].Pos.Line() == v.Pos.Line() && v.Pos.SameFile(b.Values[j].Pos) {
+               if u.Pos.SameFileAndLine(v.Pos) {
+                       if isPoorStatementOp(u.Op) {
+                               continue // Keep looking, this is also not a good statement op
+                       }
                        return j
                }
                return i
@@ -156,18 +160,10 @@ func numberLines(f *Func) {
                }
 
                if firstPosIndex == -1 { // Effectively empty block, check block's own Pos, consider preds.
-                       if b.Pos.IsStmt() != src.PosNotStmt {
-                               b.Pos = b.Pos.WithIsStmt()
-                               endlines[b.ID] = b.Pos
-                               if f.pass.debug > 0 {
-                                       fmt.Printf("Mark stmt effectively-empty-block %s %s %s\n", f.Name, b, flc(b.Pos))
-                               }
-                               continue
-                       }
                        line := src.NoXPos
                        for _, p := range b.Preds {
                                pbi := p.Block().ID
-                               if endlines[pbi] != line {
+                               if !endlines[pbi].SameFileAndLine(line) {
                                        if line == src.NoXPos {
                                                line = endlines[pbi]
                                                continue
@@ -178,7 +174,20 @@ func numberLines(f *Func) {
 
                                }
                        }
-                       endlines[b.ID] = line
+                       // If the block has no statement itself and is effectively empty, tag it w/ predecessor(s) but not as a statement
+                       if b.Pos.IsStmt() == src.PosNotStmt {
+                               b.Pos = line
+                               endlines[b.ID] = line
+                               continue
+                       }
+                       // If the block differs from its predecessors, mark it as a statement
+                       if line == src.NoXPos || !line.SameFileAndLine(b.Pos) {
+                               b.Pos = b.Pos.WithIsStmt()
+                               if f.pass.debug > 0 {
+                                       fmt.Printf("Mark stmt effectively-empty-block %s %s %s\n", f.Name, b, flc(b.Pos))
+                               }
+                       }
+                       endlines[b.ID] = b.Pos
                        continue
                }
                // check predecessors for any difference; if firstPos differs, then it is a boundary.
@@ -190,7 +199,7 @@ func numberLines(f *Func) {
                } else { // differing pred
                        for _, p := range b.Preds {
                                pbi := p.Block().ID
-                               if endlines[pbi].Line() != firstPos.Line() || !endlines[pbi].SameFile(firstPos) {
+                               if !endlines[pbi].SameFileAndLine(firstPos) {
                                        b.Values[firstPosIndex].Pos = firstPos.WithIsStmt()
                                        if f.pass.debug > 0 {
                                                fmt.Printf("Mark stmt differing-pred %s %s %s %s, different=%s ending %s\n",
@@ -210,7 +219,7 @@ func numberLines(f *Func) {
                        // skip ahead if possible
                        i = nextGoodStatementIndex(v, i, b)
                        v = b.Values[i]
-                       if v.Pos.Line() != firstPos.Line() || !v.Pos.SameFile(firstPos) {
+                       if !v.Pos.SameFileAndLine(firstPos) {
                                if f.pass.debug > 0 {
                                        fmt.Printf("Mark stmt new line %s %s %s %s prev pos = %s\n", f.Name, b, v, flc(v.Pos), flc(firstPos))
                                }
@@ -220,7 +229,7 @@ func numberLines(f *Func) {
                                v.Pos = v.Pos.WithDefaultStmt()
                        }
                }
-               if b.Pos.IsStmt() != src.PosNotStmt && (b.Pos.Line() != firstPos.Line() || !b.Pos.SameFile(firstPos)) {
+               if b.Pos.IsStmt() != src.PosNotStmt && !b.Pos.SameFileAndLine(firstPos) {
                        if f.pass.debug > 0 {
                                fmt.Printf("Mark stmt end of block differs %s %s %s prev pos = %s\n", f.Name, b, flc(b.Pos), flc(firstPos))
                        }

diff --git a/src/cmd/internal/src/pos.go b/src/cmd/internal/src/pos.go
index c9d3d347dbdaf22e5ccaaaf27cecb2c33de42f6e..8c0b6d277ba9fe85b2c3de4124dc0c0e5d7f8863 100644 (file)
--- a/src/cmd/internal/src/pos.go
+++ b/src/cmd/internal/src/pos.go
@@ -381,8 +381,9 @@ func makeLico(line, col uint) lico {
        return makeLicoRaw(line, col)
 }
 
-func (x lico) Line() uint { return uint(x) >> lineShift }
-func (x lico) Col() uint  { return uint(x) >> colShift & colMax }
+func (x lico) Line() uint           { return uint(x) >> lineShift }
+func (x lico) SameLine(y lico) bool { return 0 == (x^y)&^lico(1 << lineShift-1) }
+func (x lico) Col() uint            { return uint(x) >> colShift & colMax }
 func (x lico) IsStmt() uint {
        if x == 0 {
                return PosNotStmt

diff --git a/src/cmd/internal/src/xpos.go b/src/cmd/internal/src/xpos.go
index da90ccdb78e1ae48541a6e0bb664dce7565267bc..54fe64cf8629d56fa451bc5d428972b1d2cde516 100644 (file)
--- a/src/cmd/internal/src/xpos.go
+++ b/src/cmd/internal/src/xpos.go
@@ -35,6 +35,11 @@ func (p XPos) SameFile(q XPos) bool {
        return p.index == q.index
 }
 
+// SameFileAndLine reports whether p and q are positions on the same line in the same file.
+func (p XPos) SameFileAndLine(q XPos) bool {
+       return p.index == q.index && p.lico.SameLine(q.lico)
+}
+
 // After reports whether the position p comes after q in the source.
 // For positions with different bases, ordering is by base index.
 func (p XPos) After(q XPos) bool {