From b9a365681f57c8705bccbd6d3744afb9dc42eebb Mon Sep 17 00:00:00 2001 From: David Chase Date: Fri, 2 Mar 2018 20:33:15 -0500 Subject: [PATCH] cmd/compile: adjust is-statement on Pos's to improve debugging Stores to auto tmp variables can be hoisted to places where the line numbers make debugging look "jumpy". Turning those instructions into ones with is_stmt = 0 in the DWARF (accomplished by marking ssa nodes with NotStmt) makes debugging look better while still attributing the instructions with the correct line number. The same is true for certain register allocator spills and reloads. Change-Id: I97a394eb522d4911cc40b4bf5bf76d3d7221f6c0 Reviewed-on: https://go-review.googlesource.com/98415 Run-TryBot: David Chase Reviewed-by: Cherry Zhang Reviewed-by: Keith Randall --- src/cmd/compile/internal/gc/ssa.go | 36 ++++++++++++++----- src/cmd/compile/internal/ssa/regalloc.go | 10 +++--- .../internal/ssa/testdata/hist.gdb-opt.nexts | 24 ------------- .../ssa/testdata/scopes.gdb-opt.nexts | 2 -- 4 files changed, 34 insertions(+), 38 deletions(-) diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go index 909df12e5e..704278cd1e 100644 --- a/src/cmd/compile/internal/gc/ssa.go +++ b/src/cmd/compile/internal/gc/ssa.go @@ -419,6 +419,16 @@ func (s *state) newValue1A(op ssa.Op, t *types.Type, aux interface{}, arg *ssa.V return s.curBlock.NewValue1A(s.peekPos(), op, t, aux, arg) } +// newValue1Apos adds a new value with one argument and an aux value to the current block. +// isStmt determines whether the created values may be a statement or not +// (i.e., false means never, yes means maybe). +func (s *state) newValue1Apos(op ssa.Op, t *types.Type, aux interface{}, arg *ssa.Value, isStmt bool) *ssa.Value { + if isStmt { + return s.curBlock.NewValue1A(s.peekPos(), op, t, aux, arg) + } + return s.curBlock.NewValue1A(s.peekPos().WithNotStmt(), op, t, aux, arg) +} + // newValue1I adds a new value with one argument and an auxint value to the current block. func (s *state) newValue1I(op ssa.Op, t *types.Type, aux int64, arg *ssa.Value) *ssa.Value { return s.curBlock.NewValue1I(s.peekPos(), op, t, aux, arg) @@ -449,6 +459,16 @@ func (s *state) newValue3A(op ssa.Op, t *types.Type, aux interface{}, arg0, arg1 return s.curBlock.NewValue3A(s.peekPos(), op, t, aux, arg0, arg1, arg2) } +// newValue3Apos adds a new value with three arguments and an aux value to the current block. +// isStmt determines whether the created values may be a statement or not +// (i.e., false means never, yes means maybe). +func (s *state) newValue3Apos(op ssa.Op, t *types.Type, aux interface{}, arg0, arg1, arg2 *ssa.Value, isStmt bool) *ssa.Value { + if isStmt { + return s.curBlock.NewValue3A(s.peekPos(), op, t, aux, arg0, arg1, arg2) + } + return s.curBlock.NewValue3A(s.peekPos().WithNotStmt(), op, t, aux, arg0, arg1, arg2) +} + // newValue4 adds a new value with four arguments to the current block. func (s *state) newValue4(op ssa.Op, t *types.Type, arg0, arg1, arg2, arg3 *ssa.Value) *ssa.Value { return s.curBlock.NewValue4(s.peekPos(), op, t, arg0, arg1, arg2, arg3) @@ -997,7 +1017,7 @@ func (s *state) stmt(n *Node) { // varkill in the store chain is enough to keep it correctly ordered // with respect to call ops. if !s.canSSA(n.Left) { - s.vars[&memVar] = s.newValue1A(ssa.OpVarKill, types.TypeMem, n.Left, s.mem()) + s.vars[&memVar] = s.newValue1Apos(ssa.OpVarKill, types.TypeMem, n.Left, s.mem(), false) } case OVARLIVE: @@ -2338,7 +2358,7 @@ func (s *state) append(n *Node, inplace bool) *ssa.Value { for i, arg := range args { addr := s.newValue2(ssa.OpPtrIndex, pt, p2, s.constInt(types.Types[TINT], int64(i))) if arg.store { - s.storeType(et, addr, arg.v, 0) + s.storeType(et, addr, arg.v, 0, true) } else { s.move(et, addr, arg.v) } @@ -2488,7 +2508,7 @@ func (s *state) assign(left *Node, right *ssa.Value, deref bool, skip skipMask) // Left is not ssa-able. Compute its address. addr := s.addr(left, false) if left.Op == ONAME && left.Class() != PEXTERN && skip == 0 { - s.vars[&memVar] = s.newValue1A(ssa.OpVarDef, types.TypeMem, left, s.mem()) + s.vars[&memVar] = s.newValue1Apos(ssa.OpVarDef, types.TypeMem, left, s.mem(), !left.IsAutoTmp()) } if isReflectHeaderDataField(left) { // Package unsafe's documentation says storing pointers into @@ -2508,7 +2528,7 @@ func (s *state) assign(left *Node, right *ssa.Value, deref bool, skip skipMask) return } // Treat as a store. - s.storeType(t, addr, right, skip) + s.storeType(t, addr, right, skip, !left.IsAutoTmp()) } // zeroVal returns the zero value for type t. @@ -3491,7 +3511,7 @@ func (s *state) addr(n *Node, bounded bool) *ssa.Value { s.Fatalf("addr of undeclared ONAME %v. declared: %v", n, s.decladdrs) return nil case PAUTO: - return s.newValue1A(ssa.OpAddr, t, n, s.sp) + return s.newValue1Apos(ssa.OpAddr, t, n, s.sp, !n.IsAutoTmp()) case PPARAMOUT: // Same as PAUTO -- cannot generate LEA early. // ensure that we reuse symbols for out parameters so // that cse works on their addresses @@ -3776,11 +3796,11 @@ func (s *state) rtcall(fn *obj.LSym, returns bool, results []*types.Type, args . return res } -// do *left = right for type t. -func (s *state) storeType(t *types.Type, left, right *ssa.Value, skip skipMask) { +/// do *left = right for type t. +func (s *state) storeType(t *types.Type, left, right *ssa.Value, skip skipMask, leftIsStmt bool) { if skip == 0 && (!types.Haspointers(t) || ssa.IsStackAddr(left)) { // Known to not have write barrier. Store the whole type. - s.store(t, left, right) + s.vars[&memVar] = s.newValue3Apos(ssa.OpStore, types.TypeMem, t, left, right, s.mem(), leftIsStmt) return } diff --git a/src/cmd/compile/internal/ssa/regalloc.go b/src/cmd/compile/internal/ssa/regalloc.go index 0839bba86d..3fe170ac55 100644 --- a/src/cmd/compile/internal/ssa/regalloc.go +++ b/src/cmd/compile/internal/ssa/regalloc.go @@ -444,7 +444,7 @@ func (s *regAllocState) makeSpill(v *Value, b *Block) *Value { // *Value which is either v or a copy of v allocated to the chosen register. func (s *regAllocState) allocValToReg(v *Value, mask regMask, nospill bool, pos src.XPos) *Value { vi := &s.values[v.ID] - + pos = pos.WithNotStmt() // Check if v is already in a requested register. if mask&vi.regs != 0 { r := pickReg(mask & vi.regs) @@ -1893,18 +1893,20 @@ func (e *edgeState) process() { fmt.Printf("breaking cycle with v%d in %s:%s\n", vid, loc, c) } e.erase(r) + pos := d.pos.WithNotStmt() if _, isReg := loc.(*Register); isReg { - c = e.p.NewValue1(d.pos, OpCopy, c.Type, c) + c = e.p.NewValue1(pos, OpCopy, c.Type, c) } else { - c = e.p.NewValue1(d.pos, OpLoadReg, c.Type, c) + c = e.p.NewValue1(pos, OpLoadReg, c.Type, c) } - e.set(r, vid, c, false, d.pos) + e.set(r, vid, c, false, pos) } } // processDest generates code to put value vid into location loc. Returns true // if progress was made. func (e *edgeState) processDest(loc Location, vid ID, splice **Value, pos src.XPos) bool { + pos = pos.WithNotStmt() occupant := e.contents[loc] if occupant.vid == vid { // Value is already in the correct place. diff --git a/src/cmd/compile/internal/ssa/testdata/hist.gdb-opt.nexts b/src/cmd/compile/internal/ssa/testdata/hist.gdb-opt.nexts index 79c65a9288..69091d12ed 100644 --- a/src/cmd/compile/internal/ssa/testdata/hist.gdb-opt.nexts +++ b/src/cmd/compile/internal/ssa/testdata/hist.gdb-opt.nexts @@ -116,54 +116,30 @@ scanner = (struct bufio.Scanner *) a = 0 n = 0 t = 0 -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) 91: n += a -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) -90: t += i * a -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) 90: t += i * a 92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) -86: for i, a := range hist { -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) 87: if a == 0 { //gdb-opt=(a,n,t) a = 3 n = 3 t = 3 -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) 91: n += a -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) -90: t += i * a -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) 90: t += i * a 92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) -86: for i, a := range hist { -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) 87: if a == 0 { //gdb-opt=(a,n,t) a = 0 n = 6 t = 9 -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) 91: n += a -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) -90: t += i * a -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) 90: t += i * a 92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) -86: for i, a := range hist { -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) 87: if a == 0 { //gdb-opt=(a,n,t) a = 1 n = 8 t = 17 -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) 91: n += a -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) -90: t += i * a -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) 90: t += i * a 92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) -86: for i, a := range hist { -92: fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t) 87: if a == 0 { //gdb-opt=(a,n,t) a = 0 n = 9 diff --git a/src/cmd/compile/internal/ssa/testdata/scopes.gdb-opt.nexts b/src/cmd/compile/internal/ssa/testdata/scopes.gdb-opt.nexts index 290f02fd92..0a09989f88 100644 --- a/src/cmd/compile/internal/ssa/testdata/scopes.gdb-opt.nexts +++ b/src/cmd/compile/internal/ssa/testdata/scopes.gdb-opt.nexts @@ -4,7 +4,6 @@ 20: y := id(0) 21: fmt.Println(x) 0: -22: for i := x; i < 3; i++ { 23: x := i * i 24: y += id(x) //gdb-dbg=(x,y)//gdb-opt=(x,y) x = 0 @@ -32,7 +31,6 @@ y = 1 x = y = 1 22: for i := x; i < 3; i++ { -27: fmt.Println(x, y) 26: y = x + y //gdb-dbg=(x,y)//gdb-opt=(x,y) x = 0 y = 1 -- 2.48.1