[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>

2 files changed, 72 insertions, 4 deletions

diff --git a/src/cmd/compile/internal/ssa/schedule.go b/src/cmd/compile/internal/ssa/schedule.go
index f2a89d82d8..a455a9a399 100644
--- 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
index 0000000000..e370d55df1
--- /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:
+	}
+}