cmd/compile: preserve statements better in expandCalls

Arg/Load/Dereference rewriting was not using the best Pos for
translated values.  I also investigated whether OpCopy processing
was losing statements, and though they flood the debugging output,
doing the "obvious" thing of moving statement marks from copi-er to
copy-ee actually makes the resulting binary score slightly worse on
statement-boundary measures.
(for -N -l, 0.9994 vs 0.9995 from "nostmt -c sqle.test")

Fixes #47793.

Change-Id: I65cb878d0e5a3ceb5da4ef679020ca5f40e9b02b
Reviewed-on: https://go-review.googlesource.com/c/go/+/344769
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>

2 files changed, 59 insertions, 42 deletions

diff --git a/src/cmd/compile/internal/ssa/expand_calls.go b/src/cmd/compile/internal/ssa/expand_calls.go
index b37d3b8c9c..12c7b16acd 100644
--- a/src/cmd/compile/internal/ssa/expand_calls.go
+++ b/src/cmd/compile/internal/ssa/expand_calls.go
@@ -176,7 +176,7 @@ func (c *registerCursor) hasRegs() bool {
 type expandState struct {
 	f                  *Func
 	abi1               *abi.ABIConfig
-	debug              bool
+	debug              int // odd values log lost statement markers, so likely settings are 1 (stmts), 2 (expansion), and 3 (both)
 	canSSAType         func(*types.Type) bool
 	regSize            int64
 	sp                 *Value
@@ -302,7 +302,7 @@ func (x *expandState) Printf(format string, a ...interface{}) (n int, err error)
 //
 // TODO when registers really arrive, must also decompose anything split across two registers or registers and memory.
 func (x *expandState) rewriteSelect(leaf *Value, selector *Value, offset int64, regOffset Abi1RO) []*LocalSlot {
-	if x.debug {
+	if x.debug > 1 {
 		x.indent(3)
 		defer x.indent(-3)
 		x.Printf("rewriteSelect(%s; %s; memOff=%d; regOff=%d)\n", leaf.LongString(), selector.LongString(), offset, regOffset)
@@ -325,7 +325,7 @@ func (x *expandState) rewriteSelect(leaf *Value, selector *Value, offset int64,
 		} else {
 			x.f.Fatalf("Unexpected %s type, selector=%s, leaf=%s\n", selector.Op.String(), selector.LongString(), leaf.LongString())
 		}
-		if x.debug {
+		if x.debug > 1 {
 			x.Printf("---%s, break\n", selector.Op.String())
 		}
 	case OpArg:
@@ -335,7 +335,7 @@ func (x *expandState) rewriteSelect(leaf *Value, selector *Value, offset int64,
 			} else {
 				x.f.Fatalf("Unexpected OpArg type, selector=%s, leaf=%s\n", selector.LongString(), leaf.LongString())
 			}
-			if x.debug {
+			if x.debug > 1 {
 				x.Printf("---OpArg, break\n")
 			}
 			break
@@ -381,7 +381,7 @@ func (x *expandState) rewriteSelect(leaf *Value, selector *Value, offset int64,
 		// This case removes that StructSelect.
 		if leafType != selector.Type {
 			if x.f.Config.SoftFloat && selector.Type.IsFloat() {
-				if x.debug {
+				if x.debug > 1 {
 					x.Printf("---OpLoad, break\n")
 				}
 				break // softfloat pass will take care of that
@@ -468,7 +468,7 @@ func (x *expandState) rewriteSelect(leaf *Value, selector *Value, offset int64,
 					} else {
 						w := call.Block.NewValue2(leaf.Pos, OpLoad, leafType, off, call)
 						leaf.copyOf(w)
-						if x.debug {
+						if x.debug > 1 {
 							x.Printf("---new %s\n", w.LongString())
 						}
 					}
@@ -687,7 +687,7 @@ func (x *expandState) decomposeArg(pos src.XPos, b *Block, source, mem *Value, t
 			panic(fmt.Errorf("offset %d of requested register %d should be zero, source=%s", offs[loadRegOffset], loadRegOffset, source.LongString()))
 		}
 
-		if x.debug {
+		if x.debug > 1 {
 			x.Printf("decompose arg %s has %d locs\n", source.LongString(), len(locs))
 		}
 
@@ -836,7 +836,7 @@ func (x *expandState) decomposeLoad(pos src.XPos, b *Block, source, mem *Value,
 // pos and b locate the store instruction, source is the "base" of the value input,
 // mem is the input mem, t is the type in question, and offArg and offStore are the offsets from the respective bases.
 func storeOneArg(x *expandState, pos src.XPos, b *Block, locs []*LocalSlot, suffix string, source, mem *Value, t *types.Type, argOffset, storeOffset int64, loadRegOffset Abi1RO, storeRc registerCursor) *Value {
-	if x.debug {
+	if x.debug > 1 {
 		x.indent(3)
 		defer x.indent(-3)
 		x.Printf("storeOneArg(%s;  %s;  %s; aO=%d; sO=%d; lrO=%d; %s)\n", source.LongString(), mem.String(), t.String(), argOffset, storeOffset, loadRegOffset, storeRc.String())
@@ -877,7 +877,7 @@ func storeTwoLoad(x *expandState, pos src.XPos, b *Block, source, mem *Value, t1
 // stores of non-aggregate types.  It recursively walks up a chain of selectors until it reaches a Load or an Arg.
 // If it does not reach a Load or an Arg, nothing happens; this allows a little freedom in phase ordering.
 func (x *expandState) storeArgOrLoad(pos src.XPos, b *Block, source, mem *Value, t *types.Type, storeOffset int64, loadRegOffset Abi1RO, storeRc registerCursor) *Value {
-	if x.debug {
+	if x.debug > 1 {
 		x.indent(3)
 		defer x.indent(-3)
 		x.Printf("storeArgOrLoad(%s;  %s;  %s; %d; %s)\n", source.LongString(), mem.String(), t.String(), storeOffset, storeRc.String())
@@ -1060,7 +1060,7 @@ func (x *expandState) storeArgOrLoad(pos src.XPos, b *Block, source, mem *Value,
 		dst := x.offsetFrom(b, storeRc.storeDest, storeOffset, types.NewPtr(t))
 		s = b.NewValue3A(pos, OpStore, types.TypeMem, t, dst, source, mem)
 	}
-	if x.debug {
+	if x.debug > 1 {
 		x.Printf("-->storeArg returns %s, storeRc=%s\n", s.LongString(), storeRc.String())
 	}
 	return s
@@ -1071,14 +1071,13 @@ func (x *expandState) storeArgOrLoad(pos src.XPos, b *Block, source, mem *Value,
 // to account for any parameter stores required.
 // Any of the old Args that have their use count fall to zero are marked OpInvalid.
 func (x *expandState) rewriteArgs(v *Value, firstArg int) {
-	if x.debug {
+	if x.debug > 1 {
 		x.indent(3)
 		defer x.indent(-3)
 		x.Printf("rewriteArgs(%s; %d)\n", v.LongString(), firstArg)
 	}
 	// Thread the stores on the memory arg
 	aux := v.Aux.(*AuxCall)
-	pos := v.Pos.WithNotStmt()
 	m0 := v.MemoryArg()
 	mem := m0
 	newArgs := []*Value{}
@@ -1095,7 +1094,7 @@ func (x *expandState) rewriteArgs(v *Value, firstArg int) {
 			}
 			// "Dereference" of addressed (probably not-SSA-eligible) value becomes Move
 			// TODO(register args) this will be more complicated with registers in the picture.
-			mem = x.rewriteDereference(v.Block, x.sp, a, mem, aOffset, aux.SizeOfArg(auxI), aType, pos)
+			mem = x.rewriteDereference(v.Block, x.sp, a, mem, aOffset, aux.SizeOfArg(auxI), aType, a.Pos)
 		} else {
 			var rc registerCursor
 			var result *[]*Value
@@ -1105,11 +1104,11 @@ func (x *expandState) rewriteArgs(v *Value, firstArg int) {
 			} else {
 				aOffset = aux.OffsetOfArg(auxI)
 			}
-			if x.debug {
+			if x.debug > 1 {
 				x.Printf("...storeArg %s, %v, %d\n", a.LongString(), aType, aOffset)
 			}
 			rc.init(aRegs, aux.abiInfo, result, x.sp)
-			mem = x.storeArgOrLoad(pos, v.Block, a, mem, aType, aOffset, 0, rc)
+			mem = x.storeArgOrLoad(a.Pos, v.Block, a, mem, aType, aOffset, 0, rc)
 		}
 	}
 	var preArgStore [2]*Value
@@ -1120,16 +1119,31 @@ func (x *expandState) rewriteArgs(v *Value, firstArg int) {
 	v.AddArg(mem)
 	for _, a := range oldArgs {
 		if a.Uses == 0 {
-			if x.debug {
-				x.Printf("...marking %v unused\n", a.LongString())
-			}
-			a.invalidateRecursively()
+			x.invalidateRecursively(a)
 		}
 	}
 
 	return
 }
 
+func (x *expandState) invalidateRecursively(a *Value) {
+	var s string
+	if x.debug > 0 {
+		plus := " "
+		if a.Pos.IsStmt() == src.PosIsStmt {
+			plus = " +"
+		}
+		s = a.String() + plus + a.Pos.LineNumber() + " " + a.LongString()
+		if x.debug > 1 {
+			x.Printf("...marking %v unused\n", s)
+		}
+	}
+	lost := a.invalidateRecursively()
+	if x.debug&1 != 0 && lost { // For odd values of x.debug, do this.
+		x.Printf("Lost statement marker in %s on former %s\n", base.Ctxt.Pkgpath+"."+x.f.Name, s)
+	}
+}
+
 // expandCalls converts LE (Late Expansion) calls that act like they receive value args into a lower-level form
 // that is more oriented to a platform's ABI.  The SelectN operations that extract results are rewritten into
 // more appropriate forms, and any StructMake or ArrayMake inputs are decomposed until non-struct values are
@@ -1148,7 +1162,7 @@ func expandCalls(f *Func) {
 	x := &expandState{
 		f:                  f,
 		abi1:               f.ABI1,
-		debug:              f.pass.debug > 0,
+		debug:              f.pass.debug,
 		canSSAType:         f.fe.CanSSA,
 		regSize:            f.Config.RegSize,
 		sp:                 sp,
@@ -1170,7 +1184,7 @@ func expandCalls(f *Func) {
 		x.loRo, x.hiRo = 0, 1
 	}
 
-	if x.debug {
+	if x.debug > 1 {
 		x.Printf("\nexpandsCalls(%s)\n", f.Name)
 	}
 
@@ -1210,9 +1224,8 @@ func expandCalls(f *Func) {
 			m0 := v.MemoryArg()
 			mem := m0
 			aux := f.OwnAux
-			pos := v.Pos.WithNotStmt()
 			allResults := []*Value{}
-			if x.debug {
+			if x.debug > 1 {
 				x.Printf("multiValueExit rewriting %s\n", v.LongString())
 			}
 			var oldArgs []*Value
@@ -1233,7 +1246,7 @@ func expandCalls(f *Func) {
 						}
 						continue
 					}
-					mem = x.rewriteDereference(v.Block, auxBase, a, mem, auxOffset, auxSize, auxType, pos)
+					mem = x.rewriteDereference(v.Block, auxBase, a, mem, auxOffset, auxSize, auxType, a.Pos)
 				} else {
 					if a.Op == OpLoad && a.Args[0].Op == OpLocalAddr {
 						addr := a.Args[0] // This is a self-move. // TODO(register args) do what here for registers?
@@ -1257,13 +1270,13 @@ func expandCalls(f *Func) {
 			b.SetControl(v)
 			for _, a := range oldArgs {
 				if a.Uses == 0 {
-					if x.debug {
+					if x.debug > 1 {
 						x.Printf("...marking %v unused\n", a.LongString())
 					}
-					a.invalidateRecursively()
+					x.invalidateRecursively(a)
 				}
 			}
-			if x.debug {
+			if x.debug > 1 {
 				x.Printf("...multiValueExit new result %s\n", v.LongString())
 			}
 			x.indent(-3)
@@ -1317,7 +1330,7 @@ func expandCalls(f *Func) {
 				switch w.Op {
 				case OpStructSelect, OpArraySelect, OpSelectN, OpArg:
 					val2Preds[w] += 1
-					if x.debug {
+					if x.debug > 1 {
 						x.Printf("v2p[%s] = %d\n", w.LongString(), val2Preds[w])
 					}
 				}
@@ -1326,7 +1339,7 @@ func expandCalls(f *Func) {
 			case OpSelectN:
 				if _, ok := val2Preds[v]; !ok {
 					val2Preds[v] = 0
-					if x.debug {
+					if x.debug > 1 {
 						x.Printf("v2p[%s] = %d\n", v.LongString(), val2Preds[v])
 					}
 				}
@@ -1337,7 +1350,7 @@ func expandCalls(f *Func) {
 				}
 				if _, ok := val2Preds[v]; !ok {
 					val2Preds[v] = 0
-					if x.debug {
+					if x.debug > 1 {
 						x.Printf("v2p[%s] = %d\n", v.LongString(), val2Preds[v])
 					}
 				}
@@ -1451,7 +1464,7 @@ func expandCalls(f *Func) {
 		if dupe == nil {
 			x.commonSelectors[sk] = v
 		} else if x.sdom.IsAncestorEq(dupe.Block, v.Block) {
-			if x.debug {
+			if x.debug > 1 {
 				x.Printf("Duplicate, make %s copy of %s\n", v, dupe)
 			}
 			v.copyOf(dupe)
@@ -1467,12 +1480,12 @@ func expandCalls(f *Func) {
 
 	// Rewrite selectors.
 	for i, v := range allOrdered {
-		if x.debug {
+		if x.debug > 1 {
 			b := v.Block
 			x.Printf("allOrdered[%d] = b%d, %s, uses=%d\n", i, b.ID, v.LongString(), v.Uses)
 		}
 		if v.Uses == 0 {
-			v.invalidateRecursively()
+			x.invalidateRecursively(v)
 			continue
 		}
 		if v.Op == OpCopy {
@@ -1583,7 +1596,7 @@ func expandCalls(f *Func) {
 				v.SetArg(i, aa)
 				for a.Uses == 0 {
 					b := a.Args[0]
-					a.invalidateRecursively()
+					x.invalidateRecursively(a)
 					a = b
 				}
 			}
@@ -1619,7 +1632,7 @@ func expandCalls(f *Func) {
 // rewriteArgToMemOrRegs converts OpArg v in-place into the register version of v,
 // if that is appropriate.
 func (x *expandState) rewriteArgToMemOrRegs(v *Value) *Value {
-	if x.debug {
+	if x.debug > 1 {
 		x.indent(3)
 		defer x.indent(-3)
 		x.Printf("rewriteArgToMemOrRegs(%s)\n", v.LongString())
@@ -1650,7 +1663,7 @@ func (x *expandState) rewriteArgToMemOrRegs(v *Value) *Value {
 	default:
 		panic(badVal("Saw unexpanded OpArg", v))
 	}
-	if x.debug {
+	if x.debug > 1 {
 		x.Printf("-->%s\n", v.LongString())
 	}
 	return v
@@ -1660,7 +1673,7 @@ func (x *expandState) rewriteArgToMemOrRegs(v *Value) *Value {
 // or rewrites it into a copy of the appropriate OpArgXXX.  The actual OpArgXXX is determined by combining baseArg (an OpArg)
 // with offset, regOffset, and t to determine which portion of it to reference (either all or a part, in memory or in registers).
 func (x *expandState) newArgToMemOrRegs(baseArg, toReplace *Value, offset int64, regOffset Abi1RO, t *types.Type, pos src.XPos) *Value {
-	if x.debug {
+	if x.debug > 1 {
 		x.indent(3)
 		defer x.indent(-3)
 		x.Printf("newArgToMemOrRegs(base=%s; toReplace=%s; t=%s; memOff=%d; regOff=%d)\n", baseArg.String(), toReplace.LongString(), t.String(), offset, regOffset)
@@ -1696,7 +1709,7 @@ func (x *expandState) newArgToMemOrRegs(baseArg, toReplace *Value, offset int64,
 		if toReplace != nil {
 			toReplace.copyOf(w)
 		}
-		if x.debug {
+		if x.debug > 1 {
 			x.Printf("-->%s\n", w.LongString())
 		}
 		return w
@@ -1727,7 +1740,7 @@ func (x *expandState) newArgToMemOrRegs(baseArg, toReplace *Value, offset int64,
 	if toReplace != nil {
 		toReplace.copyOf(w)
 	}
-	if x.debug {
+	if x.debug > 1 {
 		x.Printf("-->%s\n", w.LongString())
 	}
 	return w
diff --git a/src/cmd/compile/internal/ssa/value.go b/src/cmd/compile/internal/ssa/value.go
index 630e4814b9..630143cc50 100644
--- a/src/cmd/compile/internal/ssa/value.go
+++ b/src/cmd/compile/internal/ssa/value.go
@@ -351,11 +351,13 @@ func (v *Value) reset(op Op) {
 // invalidateRecursively marks a value as invalid (unused)
 // and after decrementing reference counts on its Args,
 // also recursively invalidates any of those whose use
-// count goes to zero.
+// count goes to zero.  It returns whether any of the
+// invalidated values was marked with IsStmt.
 //
 // BEWARE of doing this *before* you've applied intended
 // updates to SSA.
-func (v *Value) invalidateRecursively() {
+func (v *Value) invalidateRecursively() bool {
+	lostStmt := v.Pos.IsStmt() == src.PosIsStmt
 	if v.InCache {
 		v.Block.Func.unCache(v)
 	}
@@ -364,7 +366,8 @@ func (v *Value) invalidateRecursively() {
 	for _, a := range v.Args {
 		a.Uses--
 		if a.Uses == 0 {
-			a.invalidateRecursively()
+			lost := a.invalidateRecursively()
+			lostStmt = lost || lostStmt
 		}
 	}
 
@@ -375,6 +378,7 @@ func (v *Value) invalidateRecursively() {
 
 	v.AuxInt = 0
 	v.Aux = nil
+	return lostStmt
 }
 
 // copyOf is called from rewrite rules.