From 58cfa40419f2ef1c58f58015eb8421a5a1b94129 Mon Sep 17 00:00:00 2001
From: David Chase <drchase@google.com>
Date: Mon, 8 Feb 2016 12:07:39 -0500
Subject: [PATCH] [dev.ssa] cmd/compile: fix for bug in cse speed improvements

Problem was caused by use of Args[].Aux differences
in early partitioning.  This artificially separated
two equivalent expressions because sort ignores the
Aux field, hence things can end with equal things
separated by unequal things and thus the equal things
are split into more than one partition.  For example:
SliceLen(a), SliceLen(b), SliceLen(a).

Fix: don't use Args[].Aux in initial partitioning.

Left in a debugging flag and some debugging Fprintf's;
not sure if that is house style or not.  We'll probably
want to be more systematic in our naming conventions,
e.g. ssa.cse, ssa.scc, etc.

Change-Id: Ib1412539cc30d91ea542c0ac7b2f9b504108ca7f
Reviewed-on: https://go-review.googlesource.com/19316
Reviewed-by: Keith Randall <khr@golang.org>
---
 src/cmd/compile/internal/gc/lex.go      |  2 ++
 src/cmd/compile/internal/ssa/compile.go |  2 ++
 src/cmd/compile/internal/ssa/cse.go     | 35 ++++++++++++++++++++-----
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/src/cmd/compile/internal/gc/lex.go b/src/cmd/compile/internal/gc/lex.go
index fb30d585270..9a1e70f43de 100644
--- a/src/cmd/compile/internal/gc/lex.go
+++ b/src/cmd/compile/internal/gc/lex.go
@@ -8,6 +8,7 @@ package gc
 
 import (
 	"bytes"
+	"cmd/compile/internal/ssa"
 	"cmd/internal/obj"
 	"flag"
 	"fmt"
@@ -54,6 +55,7 @@ var debugtab = []struct {
 	{"typeassert", &Debug_typeassert}, // print information about type assertion inlining
 	{"wb", &Debug_wb},                 // print information about write barriers
 	{"export", &Debug_export},         // print export data
+	{"ssa", &ssa.Debug},               // ssa debugging flag
 }
 
 const (
diff --git a/src/cmd/compile/internal/ssa/compile.go b/src/cmd/compile/internal/ssa/compile.go
index e602d8f5b39..04fd82bfb5f 100644
--- a/src/cmd/compile/internal/ssa/compile.go
+++ b/src/cmd/compile/internal/ssa/compile.go
@@ -11,6 +11,8 @@ import (
 	"time"
 )
 
+var Debug int
+
 // Compile is the main entry point for this package.
 // Compile modifies f so that on return:
 //   · all Values in f map to 0 or 1 assembly instructions of the target architecture
diff --git a/src/cmd/compile/internal/ssa/cse.go b/src/cmd/compile/internal/ssa/cse.go
index 052d12dd338..36ab6a3680f 100644
--- a/src/cmd/compile/internal/ssa/cse.go
+++ b/src/cmd/compile/internal/ssa/cse.go
@@ -4,7 +4,10 @@
 
 package ssa
 
-import "sort"
+import (
+	"fmt"
+	"sort"
+)
 
 // cse does common-subexpression elimination on the Function.
 // Values are just relinked, nothing is deleted.  A subsequent deadcode
@@ -77,6 +80,13 @@ func cse(f *Func) {
 		for _, v := range e {
 			valueEqClass[v.ID] = ID(i)
 		}
+		if Debug > 2 && len(e) > 1 {
+			fmt.Printf("CSE.partition #%d:", i)
+			for _, v := range e {
+				fmt.Printf(" %s", v.String())
+			}
+			fmt.Printf("\n")
+		}
 	}
 
 	// Find an equivalence class where some members of the class have
@@ -196,7 +206,8 @@ type eqclass []*Value
 //  - aux
 //  - nargs
 //  - block # if a phi op
-//  - first two arg's opcodes
+//  - first two arg's opcodes and auxint
+//  - NOT first two arg's aux; that can break CSE.
 // partitionValues returns a list of equivalence classes, each
 // being a sorted by ID list of *Values.  The eqclass slices are
 // backed by the same storage as the input slice.
@@ -212,18 +223,30 @@ func partitionValues(a []*Value) []eqclass {
 		j := 1
 		for ; j < len(a); j++ {
 			w := a[j]
-			if v.Op != w.Op ||
+			rootsDiffer := v.Op != w.Op ||
 				v.AuxInt != w.AuxInt ||
 				len(v.Args) != len(w.Args) ||
 				v.Op == OpPhi && v.Block != w.Block ||
-				v.Aux != w.Aux ||
+				v.Aux != w.Aux
+			if rootsDiffer ||
 				len(v.Args) >= 1 && (v.Args[0].Op != w.Args[0].Op ||
-					v.Args[0].Aux != w.Args[0].Aux ||
 					v.Args[0].AuxInt != w.Args[0].AuxInt) ||
 				len(v.Args) >= 2 && (v.Args[1].Op != w.Args[1].Op ||
-					v.Args[1].Aux != w.Args[1].Aux ||
 					v.Args[1].AuxInt != w.Args[1].AuxInt) ||
 				typNames[v.Type] != typNames[w.Type] {
+				if Debug > 3 {
+					fmt.Printf("CSE.partitionValues separates %s from %s, AuxInt=%v, Aux=%v, typNames=%v",
+						v.LongString(), w.LongString(), v.AuxInt != w.AuxInt, v.Aux != w.Aux, typNames[v.Type] != typNames[w.Type])
+					if !rootsDiffer {
+						if len(v.Args) >= 1 {
+							fmt.Printf(", a0Op=%v, a0AuxInt=%v", v.Args[0].Op != w.Args[0].Op, v.Args[0].AuxInt != w.Args[0].AuxInt)
+							if len(v.Args) >= 2 {
+								fmt.Printf(", a1Op=%v, a1AuxInt=%v", v.Args[1].Op != w.Args[1].Op, v.Args[1].AuxInt != w.Args[1].AuxInt)
+							}
+						}
+					}
+					fmt.Printf("\n")
+				}
 				break
 			}
 		}