[release-branch.go1.8] cmd/compile/internal/ssa: don't schedule values after select

Scheduling values after calls to selectrecv,
will cause them to be executed multiple times, due to runtime.selectgo
jumping to the next instruction in the selectrecv basic block.
Prevent this by scheduling calls to selectrecv as late as possible

Fixes #19201

Change-Id: I6415792e2c465dc6d9bd6583ba1e54b107bcf5cc
Reviewed-on: https://go-review.googlesource.com/38587
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

diff --git a/src/cmd/compile/internal/ssa/schedule.go b/src/cmd/compile/internal/ssa/schedule.go
index f2a89d82d833429bddc596bcb6a0bbcde982072d..a455a9a399876362db9e6b9792ad5f370b5a3626 100644 (file)
--- a/src/cmd/compile/internal/ssa/schedule.go
+++ b/src/cmd/compile/internal/ssa/schedule.go
@@ -14,6 +14,7 @@ const (
        ScoreMemory
        ScoreDefault
        ScoreFlags
+       ScoreSelectCall
        ScoreControl // towards bottom of block
 )
 
@@ -110,10 +111,25 @@ func schedule(f *Func) {
                                // We want all the vardefs next.
                                score[v.ID] = ScoreVarDef
                        case v.Type.IsMemory():
-                               // Schedule stores as early as possible. This tends to
-                               // reduce register pressure. It also helps make sure
-                               // VARDEF ops are scheduled before the corresponding LEA.
-                               score[v.ID] = ScoreMemory
+                               // Don't schedule independent operations after call to those functions.
+                               // runtime.selectgo will jump to next instruction after this call,
+                               // causing extra execution of those operations. Prevent it, by setting
+                               // priority to high value.
+                               if (v.Op == OpAMD64CALLstatic || v.Op == OpPPC64CALLstatic ||
+                                       v.Op == OpARMCALLstatic || v.Op == OpARM64CALLstatic ||
+                                       v.Op == Op386CALLstatic || v.Op == OpMIPS64CALLstatic ||
+                                       v.Op == OpS390XCALLstatic || v.Op == OpMIPSCALLstatic) &&
+                                       (isSameSym(v.Aux, "runtime.selectrecv") ||
+                                               isSameSym(v.Aux, "runtime.selectrecv2") ||
+                                               isSameSym(v.Aux, "runtime.selectsend") ||
+                                               isSameSym(v.Aux, "runtime.selectdefault")) {
+                                       score[v.ID] = ScoreSelectCall
+                               } else {
+                                       // Schedule stores as early as possible. This tends to
+                                       // reduce register pressure. It also helps make sure
+                                       // VARDEF ops are scheduled before the corresponding LEA.
+                                       score[v.ID] = ScoreMemory
+                               }
                        case v.Op == OpSelect0 || v.Op == OpSelect1:
                                // Schedule the pseudo-op of reading part of a tuple
                                // immediately after the tuple-generating op, since

diff --git a/test/fixedbugs/issue19201.go b/test/fixedbugs/issue19201.go
new file mode 100644 (file)
index 0000000..e370d55
--- /dev/null
+++ b/test/fixedbugs/issue19201.go
@@ -0,0 +1,52 @@
+// run
+
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+       "encoding/binary"
+)
+
+var (
+       ch1 = make(chan int)
+       ch2 = make(chan int)
+
+       bin  = []byte("a\000\000\001")
+       want = binary.BigEndian.Uint32(bin)
+
+       c consumer = noopConsumer{}
+)
+
+type msg struct {
+       code uint32
+}
+
+type consumer interface {
+       consume(msg)
+}
+
+type noopConsumer struct{}
+
+func (noopConsumer) consume(msg) {}
+
+func init() {
+       close(ch1)
+}
+
+func main() {
+       var m msg
+       m.code = binary.BigEndian.Uint32(bin)
+
+       select {
+       case <-ch1:
+               c.consume(m)
+               if m.code != want {
+                       // can not use m.code here, or it will work
+                       panic("BigEndian read failed")
+               }
+       case <-ch2:
+       }
+}