cmd/compile: transform some Phis into Or8.

func f(a, b bool) bool {
          return a || b
}

is now a single instructions (excluding loading and unloading the arguments):
      v10 = ORB <bool> v11 v12 : AX

Change-Id: Iff63399410cb46909f4318ea1c3f45a029f4aa5e
Reviewed-on: https://go-review.googlesource.com/21872
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>

4 files changed, 74 insertions, 30 deletions

diff --git a/src/cmd/compile/internal/ssa/compile.go b/src/cmd/compile/internal/ssa/compile.go
index a0b5ff71cf..bc9c830ee9 100644
--- a/src/cmd/compile/internal/ssa/compile.go
+++ b/src/cmd/compile/internal/ssa/compile.go
@@ -289,8 +289,9 @@ var passOrder = [...]constraint{
 	{"opt", "nilcheckelim"},
 	// tighten should happen before lowering to avoid splitting naturally paired instructions such as CMP/SET
 	{"tighten", "lower"},
-	// cse, nilcheckelim, prove and loopbce share idom.
+	// cse, phiopt, nilcheckelim, prove and loopbce share idom.
 	{"generic domtree", "generic cse"},
+	{"generic domtree", "phiopt"},
 	{"generic domtree", "nilcheckelim"},
 	{"generic domtree", "prove"},
 	{"generic domtree", "loopbce"},
diff --git a/src/cmd/compile/internal/ssa/phiopt.go b/src/cmd/compile/internal/ssa/phiopt.go
index 2d0a45733a..4efd497bdb 100644
--- a/src/cmd/compile/internal/ssa/phiopt.go
+++ b/src/cmd/compile/internal/ssa/phiopt.go
@@ -45,44 +45,51 @@ func phiopt(f *Func) {
 		}
 		// b0 is the if block giving the boolean value.
 
-		var reverse bool
+		// reverse is the predecessor from which the truth value comes.
+		var reverse int
 		if b0.Succs[0] == pb0 && b0.Succs[1] == pb1 {
-			reverse = false
+			reverse = 0
 		} else if b0.Succs[0] == pb1 && b0.Succs[1] == pb0 {
-			reverse = true
+			reverse = 1
 		} else {
 			b.Fatalf("invalid predecessors\n")
 		}
 
 		for _, v := range b.Values {
-			if v.Op != OpPhi || !v.Type.IsBoolean() || v.Args[0].Op != OpConstBool || v.Args[1].Op != OpConstBool {
+			if v.Op != OpPhi || !v.Type.IsBoolean() {
 				continue
 			}
 
-			ok, isCopy := false, false
-			if v.Args[0].AuxInt == 1 && v.Args[1].AuxInt == 0 {
-				ok, isCopy = true, !reverse
-			} else if v.Args[0].AuxInt == 0 && v.Args[1].AuxInt == 1 {
-				ok, isCopy = true, reverse
-			}
-
-			// (Phi (ConstBool [x]) (ConstBool [x])) is already handled by opt / phielim.
-
-			if ok && isCopy {
-				if f.pass.debug > 0 {
-					f.Config.Warnl(b.Line, "converted OpPhi to OpCopy")
+			// Replaces
+			//   if a { x = true } else { x = false } with x = a
+			// and
+			//   if a { x = false } else { x = true } with x = !a
+			if v.Args[0].Op == OpConstBool && v.Args[1].Op == OpConstBool {
+				if v.Args[reverse].AuxInt != v.Args[1-reverse].AuxInt {
+					ops := [2]Op{OpNot, OpCopy}
+					v.reset(ops[v.Args[reverse].AuxInt])
+					v.AddArg(b0.Control)
+					if f.pass.debug > 0 {
+						f.Config.Warnl(b.Line, "converted OpPhi to %v", v.Op)
+					}
+					continue
 				}
-				v.reset(OpCopy)
-				v.AddArg(b0.Control)
-				continue
 			}
-			if ok && !isCopy {
-				if f.pass.debug > 0 {
-					f.Config.Warnl(b.Line, "converted OpPhi to OpNot")
+
+			// Replaces
+			//   if a { x = true } else { x = value } with x = a || value.
+			// Requires that value dominates x, meaning that regardless of a,
+			// value is always computed. This guarantees that the side effects
+			// of value are not seen if a is false.
+			if v.Args[reverse].Op == OpConstBool && v.Args[reverse].AuxInt == 1 {
+				if tmp := v.Args[1-reverse]; f.sdom.isAncestorEq(tmp.Block, b) {
+					v.reset(OpOr8)
+					v.SetArgs2(b0.Control, tmp)
+					if f.pass.debug > 0 {
+						f.Config.Warnl(b.Line, "converted OpPhi to %v", v.Op)
+					}
+					continue
 				}
-				v.reset(OpNot)
-				v.AddArg(b0.Control)
-				continue
 			}
 		}
 	}
diff --git a/test/phiopt.go b/test/phiopt.go
index 9b9b701124..37caab0b51 100644
--- a/test/phiopt.go
+++ b/test/phiopt.go
@@ -1,8 +1,13 @@
 // +build amd64
 // errorcheck -0 -d=ssa/phiopt/debug=3
 
+// Copyright 2016 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
 
+//go:noinline
 func f0(a bool) bool {
 	x := false
 	if a {
@@ -10,9 +15,10 @@ func f0(a bool) bool {
 	} else {
 		x = false
 	}
-	return x // ERROR "converted OpPhi to OpCopy$"
+	return x // ERROR "converted OpPhi to Copy$"
 }
 
+//go:noinline
 func f1(a bool) bool {
 	x := false
 	if a {
@@ -20,23 +26,49 @@ func f1(a bool) bool {
 	} else {
 		x = true
 	}
-	return x // ERROR "converted OpPhi to OpNot$"
+	return x // ERROR "converted OpPhi to Not$"
 }
 
+//go:noinline
 func f2(a, b int) bool {
 	x := true
 	if a == b {
 		x = false
 	}
-	return x // ERROR "converted OpPhi to OpNot$"
+	return x // ERROR "converted OpPhi to Not$"
 }
 
+//go:noinline
 func f3(a, b int) bool {
 	x := false
 	if a == b {
 		x = true
 	}
-	return x // ERROR "converted OpPhi to OpCopy$"
+	return x // ERROR "converted OpPhi to Copy$"
+}
+
+//go:noinline
+func f4(a, b bool) bool {
+	return a || b // ERROR "converted OpPhi to Or8$"
+}
+
+//go:noinline
+func f5(a int, b bool) bool {
+	x := b
+	if a == 0 {
+		x = true
+	}
+	return x // ERROR "converted OpPhi to Or8$"
+}
+
+//go:noinline
+func f6(a int, b bool) bool {
+	x := b
+	if a == 0 {
+		// f6 has side effects so the OpPhi should not be converted.
+		x = f6(a, b)
+	}
+	return x
 }
 
 func main() {
diff --git a/test/prove.go b/test/prove.go
index a78adf03dc..8bcc9ae614 100644
--- a/test/prove.go
+++ b/test/prove.go
@@ -1,6 +1,10 @@
 // +build amd64
 // errorcheck -0 -d=ssa/prove/debug=3
 
+// Copyright 2016 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 "math"