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

diff --git a/src/cmd/compile/internal/gc/lex.go b/src/cmd/compile/internal/gc/lex.go
index fb30d58527061055f4feb839d47974afa12a2e3d..9a1e70f43de1c9d6ed146be3f08d973645db5801 100644 (file)
--- 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 e602d8f5b39055d5a10952ba10246cbcdcc9f7d9..04fd82bfb5fee3c96be8631350cb96699b9e1c3b 100644 (file)
--- 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 052d12dd33858f1c7e4ce5d5f5f5784b56fc9a98..36ab6a3680fbc6e7f7bca049fd562d2b76ef1dc2 100644 (file)
--- 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
                        }
                }