cmd/compile: spos handling fixes to improve prolog debuggability

With the new register ABI, the compiler sometimes introduces spills of
argument registers in function prologs; depending on the positions
assigned to these spills and whether they have the IsStmt flag set,
this can degrade the debugging experience. For example, in this
function from one of the Delve regression tests:

L13:  func foo((eface interface{}) {
L14:	if eface != nil {
L15:		n++
L16:	}
L17   }

we wind up with a prolog containing two spill instructions, the first
with line 14, the second with line 13.  The end result for the user
is that if you set a breakpoint in foo and run to it, then do "step",
execution will initially stop at L14, then jump "backwards" to L13.

The root of the problem in this case is that an ArgIntReg pseudo-op is
introduced during expand calls, then promoted (due to lowering) to a
first-class statement (IsStmt flag set), which in turn causes
downstream handling to propagate its position to the first of the register
spills in the prolog.

To help improve things, this patch changes the rewriter to avoid
moving an "IsStmt" flag from a deleted/replaced instruction to an
Arg{Int,Float}Reg value, and adds Arg{Int,Float}Reg to the list of
opcodes not suitable for selection as statement boundaries, and
suppresses generation of additional register spills in defframe() when
optimization is disabled (since in that case things will get spilled
in any case).

This is not a comprehensive/complete fix; there are still cases where
we get less-than-ideal source position markers (ex: issue 45680).

Updates #40724.

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

src/cmd/compile/internal/amd64/ssa.go

@@ -1370,5 +1370,6 @@ func spillArgReg(pp *objw.Progs, p *obj.Prog, f *ssa.Func, t *types.Type, reg in
 	p = pp.Append(p, storeByType(t), obj.TYPE_REG, reg, 0, obj.TYPE_MEM, 0, n.FrameOffset()+off)
 	p.To.Name = obj.NAME_PARAM
 	p.To.Sym = n.Linksym()
+	p.Pos = p.Pos.WithNotStmt()
 	return p
 }

src/cmd/compile/internal/ssa/numberlines.go

@@ -16,7 +16,8 @@ func isPoorStatementOp(op Op) bool {
 	// so that a debugger-user sees the stop before the panic, and can examine the value.
 	case OpAddr, OpLocalAddr, OpOffPtr, OpStructSelect, OpPhi, OpITab, OpIData,
 		OpIMake, OpStringMake, OpSliceMake, OpStructMake0, OpStructMake1, OpStructMake2, OpStructMake3, OpStructMake4,
-		OpConstBool, OpConst8, OpConst16, OpConst32, OpConst64, OpConst32F, OpConst64F, OpSB, OpSP:
+		OpConstBool, OpConst8, OpConst16, OpConst32, OpConst64, OpConst32F, OpConst64F, OpSB, OpSP,
+		OpArgIntReg, OpArgFloatReg:
 		return true
 	}
 	return false
@@ -61,7 +62,7 @@ func nextGoodStatementIndex(v *Value, i int, b *Block) int {
 // statement boundary.
 func notStmtBoundary(op Op) bool {
 	switch op {
-	case OpCopy, OpPhi, OpVarKill, OpVarDef, OpVarLive, OpUnknown, OpFwdRef, OpArg:
+	case OpCopy, OpPhi, OpVarKill, OpVarDef, OpVarLive, OpUnknown, OpFwdRef, OpArg, OpArgIntReg, OpArgFloatReg:
 		return true
 	}
 	return false

src/cmd/compile/internal/ssa/rewrite.go

@@ -159,7 +159,7 @@ func applyRewrite(f *Func, rb blockRewriter, rv valueRewriter, deadcode deadValu
 				f.freeValue(v)
 				continue
 			}
-			if v.Pos.IsStmt() != src.PosNotStmt && pendingLines.get(vl) == int32(b.ID) {
+			if v.Pos.IsStmt() != src.PosNotStmt && !notStmtBoundary(v.Op) && pendingLines.get(vl) == int32(b.ID) {
 				pendingLines.remove(vl)
 				v.Pos = v.Pos.WithIsStmt()
 			}

src/cmd/compile/internal/ssagen/ssa.go

@@ -7071,8 +7071,10 @@ func defframe(s *State, e *ssafn, f *ssa.Func) {
 	// slot. This can only happen with aggregate-typed arguments that are SSA-able
 	// and not address-taken (for non-SSA-able or address-taken arguments we always
 	// spill upfront).
+	// Note: spilling is unnecessary in the -N/no-optimize case, since all values
+	// will be considered non-SSAable and spilled up front.
 	// TODO(register args) Make liveness more fine-grained to that partial spilling is okay.
-	if f.OwnAux.ABIInfo().InRegistersUsed() != 0 {
+	if f.OwnAux.ABIInfo().InRegistersUsed() != 0 && base.Flag.N == 0 {
 		// First, see if it is already spilled before it may be live. Look for a spill
 		// in the entry block up to the first safepoint.
 		type nameOff struct {