]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: adjust Pos setting for "empty" blocks
authorDavid Chase <drchase@google.com>
Sun, 29 Oct 2017 15:53:18 +0000 (11:53 -0400)
committerDavid Chase <drchase@google.com>
Wed, 8 Nov 2017 22:39:49 +0000 (22:39 +0000)
Plain blocks that contain only uninteresting instructions
(that do not have reliable Pos information themselves)
need to have their Pos left unset so that they can
inherit it from their successors.  The "uninteresting"
test was not properly applied and not properly defined.
OpFwdRef does not appear in the ssa.html debugging output,
but at the time of the test these instructions did appear,
and it needs to be part of the test.

Fixes #22365.

Change-Id: I99e5b271acd8f6bcfe0f72395f905c7744ea9a02
Reviewed-on: https://go-review.googlesource.com/74252
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
src/cmd/compile/internal/gc/ssa.go
src/cmd/compile/internal/ssa/block.go
src/cmd/compile/internal/ssa/testdata/hist.dbg-dlv.nexts
src/cmd/compile/internal/ssa/testdata/hist.dbg-gdb.nexts
src/cmd/compile/internal/ssa/testdata/hist.opt-gdb.nexts
src/cmd/compile/internal/ssa/value.go
test/fixedbugs/issue18902.go

index 339c6be7a49add455d672d721c30b735a4364c9a..7780953e90f506b702e0d38a9b02596dd422eb4d 100644 (file)
@@ -203,7 +203,7 @@ func buildssa(fn *Node, worker int) *ssa.Func {
 
        for _, b := range s.f.Blocks {
                if b.Pos != src.NoXPos {
-                       updateUnsetPredPos(b)
+                       s.updateUnsetPredPos(b)
                }
        }
 
@@ -217,26 +217,35 @@ func buildssa(fn *Node, worker int) *ssa.Func {
        return s.f
 }
 
-func updateUnsetPredPos(b *ssa.Block) {
+// updateUnsetPredPos propagates the earliest-value position information for b
+// towards all of b's predecessors that need a position, and recurs on that
+// predecessor if its position is updated. B should have a non-empty position.
+func (s *state) updateUnsetPredPos(b *ssa.Block) {
+       if b.Pos == src.NoXPos {
+               s.Fatalf("Block %s should have a position", b)
+       }
+       bestPos := src.NoXPos
        for _, e := range b.Preds {
                p := e.Block()
-               if p.Pos == src.NoXPos && p.Kind == ssa.BlockPlain {
-                       pos := b.Pos
-                       // TODO: This ought to be produce a better result, but it causes
-                       // line 46 ("scanner := bufio.NewScanner(reader)")
-                       // to drop out of gdb-dbg and dlv-dbg debug-next traces for hist.go.
+               if !p.LackingPos() {
+                       continue
+               }
+               if bestPos == src.NoXPos {
+                       bestPos = b.Pos
                        for _, v := range b.Values {
-                               if v.Op == ssa.OpVarDef || v.Op == ssa.OpVarKill || v.Op == ssa.OpVarLive || v.Op == ssa.OpCopy && v.Type == types.TypeMem {
+                               if v.LackingPos() {
                                        continue
                                }
                                if v.Pos != src.NoXPos {
-                                       pos = v.Pos
+                                       // Assume values are still in roughly textual order;
+                                       // TODO: could also seek minimum position?
+                                       bestPos = v.Pos
                                        break
                                }
                        }
-                       p.Pos = pos
-                       updateUnsetPredPos(p) // We do not expect long chains of these, thus recursion is okay.
                }
+               p.Pos = bestPos
+               s.updateUnsetPredPos(p) // We do not expect long chains of these, thus recursion is okay.
        }
        return
 }
@@ -372,7 +381,7 @@ func (s *state) endBlock() *ssa.Block {
        s.defvars[b.ID] = s.vars
        s.curBlock = nil
        s.vars = nil
-       if len(b.Values) == 0 && b.Kind == ssa.BlockPlain {
+       if b.LackingPos() {
                // Empty plain blocks get the line of their successor (handled after all blocks created),
                // except for increment blocks in For statements (handled in ssa conversion of OFOR),
                // and for blocks ending in GOTO/BREAK/CONTINUE.
@@ -817,7 +826,9 @@ func (s *state) stmt(n *Node) {
 
        case ORETURN:
                s.stmtList(n.List)
-               s.exit()
+               b := s.exit()
+               b.Pos = s.lastPos
+
        case ORETJMP:
                s.stmtList(n.List)
                b := s.exit()
index 10f07cefba7d33317bcbe08be8133b4fd7ee46e4..273e5f15d779b6be2871435c19d6d7c058b3888d 100644 (file)
@@ -198,6 +198,29 @@ func (b *Block) swapSuccessors() {
        b.Likely *= -1
 }
 
+// LackingPos indicates whether b is a block whose position should be inherited
+// from its successors.  This is true if all the values within it have unreliable positions
+// and if it is "plain", meaning that there is no control flow that is also very likely
+// to correspond to a well-understood source position.
+func (b *Block) LackingPos() bool {
+       // Non-plain predecessors are If or Defer, which both (1) have two successors,
+       // which might have different line numbers and (2) correspond to statements
+       // in the source code that have positions, so this case ought not occur anyway.
+       if b.Kind != BlockPlain {
+               return false
+       }
+       if b.Pos != src.NoXPos {
+               return false
+       }
+       for _, v := range b.Values {
+               if v.LackingPos() {
+                       continue
+               }
+               return false
+       }
+       return true
+}
+
 func (b *Block) Logf(msg string, args ...interface{})   { b.Func.Logf(msg, args...) }
 func (b *Block) Log() bool                              { return b.Func.Log() }
 func (b *Block) Fatalf(msg string, args ...interface{}) { b.Func.Fatalf(msg, args...) }
index 49a63c7294026a74d199cecfec0c9d4805af42be..f4fe2af161904338ab072808fc667d49d98b5635 100644 (file)
@@ -8,7 +8,7 @@
 63:            hist := make([]int, 7)                                //gdb-opt=(sink,dx/O,dy/O)
 64:            var reader io.Reader = strings.NewReader(cannedInput) //gdb-dbg=(hist/A,cannedInput/A)
 65:            if len(os.Args) > 1 {
-70:                            return
+73:            scanner := bufio.NewScanner(reader)
 74:            for scanner.Scan() { //gdb-opt=(scanner/A)
 75:                    s := scanner.Text()
 76:                    i, err := strconv.ParseInt(s, 10, 64)
@@ -96,4 +96,3 @@
 87:                    if a == 0 { //gdb-opt=(a,n,t)
 88:                            continue
 86:            for i, a := range hist {
-95:    }
index 6a62b0533f10e1e2aaccff4ee3ff2623d6892bf9..abd4535ca5e42f67f72f76282e745600d06baf84 100644 (file)
@@ -12,7 +12,7 @@ l.end.y = 4
 hist =  []int = {0, 0, 0, 0, 0, 0, 0}
 cannedInput = "1\n1\n1\n2\n2\n2\n4\n4\n5\n"
 65:            if len(os.Args) > 1 {
-70:                            return
+73:            scanner := bufio.NewScanner(reader)
 74:            for scanner.Scan() { //gdb-opt=(scanner/A)
 75:                    s := scanner.Text()
 76:                    i, err := strconv.ParseInt(s, 10, 64)
@@ -121,4 +121,3 @@ t = 22
 87:                    if a == 0 { //gdb-opt=(a,n,t)
 88:                            continue
 86:            for i, a := range hist {
-95:    }
index 0b5db28c0e6939f4e5a4e026c1e8cc155fbffa3f..f6c6a3c9bec8bafbf5c9bd16e1232c24e7a78609 100644 (file)
@@ -180,4 +180,3 @@ a = 0
 n = 9
 t = 22
 88:                            continue
-95:    }
index 68d9565b2bea378c974abfdd09d2c850b4142193..288ad19bc3cdcfcc6eabe682527be3bc6fbf75dd 100644 (file)
@@ -320,3 +320,14 @@ func (v *Value) MemoryArg() *Value {
        }
        return nil
 }
+
+// LackingPos indicates whether v is a value that is unlikely to have a correct
+// position assigned to it.  Ignoring such values leads to more user-friendly positions
+// assigned to nearby values and the blocks containing them.
+func (v *Value) LackingPos() bool {
+       // The exact definition of LackingPos is somewhat heuristically defined and may change
+       // in the future, for example if some of these operations are generated more carefully
+       // with respect to their source position.
+       return v.Op == OpVarDef || v.Op == OpVarKill || v.Op == OpVarLive || v.Op == OpPhi ||
+               (v.Op == OpFwdRef || v.Op == OpCopy) && v.Type == types.TypeMem
+}
index 9b85503eca8b5937bc7e5a5ac58d16f0ae0e8100..78c92187ee3420c743d93489937bf658680b8689 100644 (file)
@@ -50,7 +50,7 @@ func main() {
        testarch := os.Getenv("TESTARCH")     // Targets other platform in test compilation.
        debug := os.Getenv("TESTDEBUG") != "" // Output the relevant assembly language.
 
-       cmd := exec.Command("go", "build", "-gcflags", "-S", "fixedbugs/issue18902b.go")
+       cmd := exec.Command("go", "tool", "compile", "-S", "fixedbugs/issue18902b.go")
        var buf bytes.Buffer
        cmd.Stdout = &buf
        cmd.Stderr = &buf