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>

Author: dr2chase

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))
 			}

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

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 {