cmd/compile: combine phielim and copyelim into a single pass

Change-Id: Id21145b14169d28bac2144a31f6d3d9729f4be1e
GitHub-Last-Rev: 5413f4753e5acb60db6a93cb3409047bddc8df6d
GitHub-Pull-Request: golang/go#63818
Reviewed-on: https://go-review.googlesource.com/c/go/+/538535
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Keith Randall <khr@google.com>

3 files changed, 86 insertions, 98 deletions

diff --git a/src/cmd/compile/internal/ssa/compile.go b/src/cmd/compile/internal/ssa/compile.go
index d125891f88..80ef53d085 100644
--- a/src/cmd/compile/internal/ssa/compile.go
+++ b/src/cmd/compile/internal/ssa/compile.go
@@ -455,10 +455,8 @@ commas. For example:
 
 // list of passes for the compiler
 var passes = [...]pass{
-	// TODO: combine phielim and copyelim into a single pass?
 	{name: "number lines", fn: numberLines, required: true},
-	{name: "early phielim", fn: phielim},
-	{name: "early copyelim", fn: copyelim},
+	{name: "early phielim and copyelim", fn: copyelim},
 	{name: "early deadcode", fn: deadcode}, // remove generated dead code to avoid doing pointless work during opt
 	{name: "short circuit", fn: shortcircuit},
 	{name: "decompose user", fn: decomposeUser, required: true},
@@ -496,8 +494,7 @@ var passes = [...]pass{
 	{name: "tighten tuple selectors", fn: tightenTupleSelectors, required: true},
 	{name: "lowered deadcode", fn: deadcode, required: true},
 	{name: "checkLower", fn: checkLower, required: true},
-	{name: "late phielim", fn: phielim},
-	{name: "late copyelim", fn: copyelim},
+	{name: "late phielim and copyelim", fn: copyelim},
 	{name: "tighten", fn: tighten, required: true}, // move values closer to their uses
 	{name: "late deadcode", fn: deadcode},
 	{name: "critical", fn: critical, required: true}, // remove critical edges
diff --git a/src/cmd/compile/internal/ssa/copyelim.go b/src/cmd/compile/internal/ssa/copyelim.go
index 17471e3b5f..ea888f46f9 100644
--- a/src/cmd/compile/internal/ssa/copyelim.go
+++ b/src/cmd/compile/internal/ssa/copyelim.go
@@ -4,28 +4,13 @@
 
 package ssa
 
+// combine copyelim and phielim into a single pass.
 // copyelim removes all uses of OpCopy values from f.
 // A subsequent deadcode pass is needed to actually remove the copies.
 func copyelim(f *Func) {
-	// Modify all values so no arg (including args
-	// of OpCopy) is a copy.
-	for _, b := range f.Blocks {
-		for _, v := range b.Values {
-
-			// This is an early place in SSA where all values are examined.
-			// Rewrite all 0-sized Go values to remove accessors, dereferences, loads, etc.
-			if t := v.Type; (t.IsStruct() || t.IsArray()) && t.Size() == 0 {
-				if t.IsStruct() {
-					v.reset(OpStructMake0)
-				} else {
-					v.reset(OpArrayMake0)
-				}
-			}
-
-			copyelimValue(v)
-		}
-	}
+	phielim(f)
 
+	// loop of copyelimValue(v) process has been done in phielim() pass.
 	// Update block control values.
 	for _, b := range f.Blocks {
 		for i, v := range b.ControlValues() {
@@ -93,3 +78,84 @@ func copyelimValue(v *Value) {
 		}
 	}
 }
+
+// phielim eliminates redundant phi values from f.
+// A phi is redundant if its arguments are all equal. For
+// purposes of counting, ignore the phi itself. Both of
+// these phis are redundant:
+//
+//	v = phi(x,x,x)
+//	v = phi(x,v,x,v)
+//
+// We repeat this process to also catch situations like:
+//
+//	v = phi(x, phi(x, x), phi(x, v))
+//
+// TODO: Can we also simplify cases like:
+//
+//	v = phi(v, w, x)
+//	w = phi(v, w, x)
+//
+// and would that be useful?
+func phielim(f *Func) {
+	for {
+		change := false
+		for _, b := range f.Blocks {
+			for _, v := range b.Values {
+				// This is an early place in SSA where all values are examined.
+				// Rewrite all 0-sized Go values to remove accessors, dereferences, loads, etc.
+				if t := v.Type; (t.IsStruct() || t.IsArray()) && t.Size() == 0 {
+					if t.IsStruct() {
+						v.reset(OpStructMake0)
+					} else {
+						v.reset(OpArrayMake0)
+					}
+				}
+				// Modify all values so no arg (including args
+				// of OpCopy) is a copy.
+				copyelimValue(v)
+				change = phielimValue(v) || change
+			}
+		}
+		if !change {
+			break
+		}
+	}
+}
+
+// phielimValue tries to convert the phi v to a copy.
+func phielimValue(v *Value) bool {
+	if v.Op != OpPhi {
+		return false
+	}
+
+	// If there are two distinct args of v which
+	// are not v itself, then the phi must remain.
+	// Otherwise, we can replace it with a copy.
+	var w *Value
+	for _, x := range v.Args {
+		if x == v {
+			continue
+		}
+		if x == w {
+			continue
+		}
+		if w != nil {
+			return false
+		}
+		w = x
+	}
+
+	if w == nil {
+		// v references only itself. It must be in
+		// a dead code loop. Don't bother modifying it.
+		return false
+	}
+	v.Op = OpCopy
+	v.SetArgs1(w)
+	f := v.Block.Func
+	if f.pass.debug > 0 {
+		f.Warnl(v.Pos, "eliminated phi")
+	}
+	return true
+}
diff --git a/src/cmd/compile/internal/ssa/phielim.go b/src/cmd/compile/internal/ssa/phielim.go
deleted file mode 100644
index 4fc942375f..0000000000
--- a/src/cmd/compile/internal/ssa/phielim.go
+++ /dev/null
@@ -1,75 +0,0 @@
-// Copyright 2015 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 ssa
-
-// phielim eliminates redundant phi values from f.
-// A phi is redundant if its arguments are all equal. For
-// purposes of counting, ignore the phi itself. Both of
-// these phis are redundant:
-//
-//	v = phi(x,x,x)
-//	v = phi(x,v,x,v)
-//
-// We repeat this process to also catch situations like:
-//
-//	v = phi(x, phi(x, x), phi(x, v))
-//
-// TODO: Can we also simplify cases like:
-//
-//	v = phi(v, w, x)
-//	w = phi(v, w, x)
-//
-// and would that be useful?
-func phielim(f *Func) {
-	for {
-		change := false
-		for _, b := range f.Blocks {
-			for _, v := range b.Values {
-				copyelimValue(v)
-				change = phielimValue(v) || change
-			}
-		}
-		if !change {
-			break
-		}
-	}
-}
-
-// phielimValue tries to convert the phi v to a copy.
-func phielimValue(v *Value) bool {
-	if v.Op != OpPhi {
-		return false
-	}
-
-	// If there are two distinct args of v which
-	// are not v itself, then the phi must remain.
-	// Otherwise, we can replace it with a copy.
-	var w *Value
-	for _, x := range v.Args {
-		if x == v {
-			continue
-		}
-		if x == w {
-			continue
-		}
-		if w != nil {
-			return false
-		}
-		w = x
-	}
-
-	if w == nil {
-		// v references only itself. It must be in
-		// a dead code loop. Don't bother modifying it.
-		return false
-	}
-	v.Op = OpCopy
-	v.SetArgs1(w)
-	f := v.Block.Func
-	if f.pass.debug > 0 {
-		f.Warnl(v.Pos, "eliminated phi")
-	}
-	return true
-}