[dev.ssa] cmd/compile: enhance SSA filtering, add OpConvert
Modified GOSSA{HASH.PKG} environment variable filters to
make it easier to make/run with all SSA for testing.
Disable attempts at SSA for architectures that are not
amd64 (avoid spurious errors/unimplementeds.)
Removed easy out for unimplemented features.
Add convert op for proper liveness in presence of uintptr
to/from unsafe.Pointer conversions.
Tweaked stack sizes to get a pass on windows;
1024 instead 768, was observed to pass at least once.
Change-Id: Ida3800afcda67d529e3b1cf48ca4a3f0fa48b2c5
Reviewed-on: https://go-review.googlesource.com/16201
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Chase <drchase@google.com>
diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go
index a5010a3..b3ba2fb 100644
--- a/src/cmd/compile/internal/gc/pgen.go
+++ b/src/cmd/compile/internal/gc/pgen.go
@@ -414,7 +414,9 @@
// Build an SSA backend function.
// TODO: get rid of usessa.
- ssafn, usessa = buildssa(Curfn)
+ if Thearch.Thestring == "amd64" {
+ ssafn, usessa = buildssa(Curfn)
+ }
continpc = nil
breakpc = nil
diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go
index 64391b0..8939f14 100644
--- a/src/cmd/compile/internal/gc/ssa.go
+++ b/src/cmd/compile/internal/gc/ssa.go
@@ -24,8 +24,32 @@
// it will never return nil, and the bool can be removed.
func buildssa(fn *Node) (ssafn *ssa.Func, usessa bool) {
name := fn.Func.Nname.Sym.Name
+ gossahash := os.Getenv("GOSSAHASH")
usessa = strings.HasSuffix(name, "_ssa") || strings.Contains(name, "_ssa.") || name == os.Getenv("GOSSAFUNC")
+ // Environment variable control of SSA CG
+ // 1. IF GOSSAFUNC == current function name THEN
+ // compile this function with SSA and log output to ssa.html
+
+ // 2. IF GOSSAHASH == "y" or "Y" THEN
+ // compile this function (and everything else) with SSA
+
+ // 3. IF GOSSAHASH == "" THEN
+ // IF GOSSAPKG == current package name THEN
+ // compile this function (and everything in this package) with SSA
+ // ELSE
+ // use the old back end for this function.
+ // This is for compatibility with existing test harness and should go away.
+
+ // 4. IF GOSSAHASH is a suffix of the binary-rendered SHA1 hash of the function name THEN
+ // compile this function with SSA
+ // ELSE
+ // compile this function with the old back end.
+
+ // Plan is for 3 to be remove, and the 2) dependence on GOSSAHASH changes
+ // from "y"/"Y" to empty -- then SSA is default, and is disabled by setting
+ // GOSSAHASH to a value that is neither 0 nor 1 (e.g., "N" or "X")
+
if usessa {
fmt.Println("generating SSA for", name)
dumplist("buildssa-enter", fn.Func.Enter)
@@ -58,17 +82,6 @@
}
}()
- // If SSA support for the function is incomplete,
- // assume that any panics are due to violated
- // invariants. Swallow them silently.
- defer func() {
- if err := recover(); err != nil {
- if !e.unimplemented {
- panic(err)
- }
- }
- }()
-
// We construct SSA using an algorithm similar to
// Brau, Buchwald, Hack, Leißa, Mallon, and Zwinkau
// http://pp.info.uni-karlsruhe.de/uploads/publikationen/braun13cc.pdf
@@ -167,27 +180,17 @@
// Main call to ssa package to compile function
ssa.Compile(s.f)
- // Calculate stats about what percentage of functions SSA handles.
- if false {
- fmt.Printf("SSA implemented: %t\n", !e.unimplemented)
- }
-
- if e.unimplemented {
- return nil, false
- }
-
- // TODO: enable codegen more broadly once the codegen stabilizes
- // and runtime support is in (gc maps, write barriers, etc.)
- if usessa {
+ if usessa || gossahash == "y" || gossahash == "Y" {
return s.f, true
}
- if localpkg.Name != os.Getenv("GOSSAPKG") {
- return s.f, false
- }
- if os.Getenv("GOSSAHASH") == "" {
+ if gossahash == "" {
+ if localpkg.Name != os.Getenv("GOSSAPKG") {
+ return s.f, false
+ }
// Use everything in the package
return s.f, true
}
+
// Check the hash of the name against a partial input hash.
// We use this feature to do a binary search within a package to
// find a function that is incorrectly compiled.
@@ -195,10 +198,26 @@
for _, b := range sha1.Sum([]byte(name)) {
hstr += fmt.Sprintf("%08b", b)
}
- if strings.HasSuffix(hstr, os.Getenv("GOSSAHASH")) {
+
+ if strings.HasSuffix(hstr, gossahash) {
fmt.Printf("GOSSAHASH triggered %s\n", name)
return s.f, true
}
+
+ // Iteratively try additional hashes to allow tests for multi-point
+ // failure.
+ for i := 0; true; i++ {
+ ev := fmt.Sprintf("GOSSAHASH%d", i)
+ evv := os.Getenv(ev)
+ if evv == "" {
+ break
+ }
+ if strings.HasSuffix(hstr, evv) {
+ fmt.Printf("%s triggered %s\n", ev, name)
+ return s.f, true
+ }
+ }
+
return s.f, false
}
@@ -1353,6 +1372,15 @@
// Assume everything will work out, so set up our return value.
// Anything interesting that happens from here is a fatal.
x := s.expr(n.Left)
+
+ // Special case for not confusing GC and liveness.
+ // We don't want pointers accidentally classified
+ // as not-pointers or vice-versa because of copy
+ // elision.
+ if to.IsPtr() != from.IsPtr() {
+ return s.newValue1(ssa.OpConvert, to, x)
+ }
+
v := s.newValue1(ssa.OpCopy, to, x) // ensure that v has the right type
// CONVNOP closure
@@ -1364,6 +1392,7 @@
if from.Etype == to.Etype {
return v
}
+
// unsafe.Pointer <--> *T
if to.Etype == TUNSAFEPTR && from.IsPtr() || from.Etype == TUNSAFEPTR && to.IsPtr() {
return v
diff --git a/src/cmd/compile/internal/ssa/gen/AMD64.rules b/src/cmd/compile/internal/ssa/gen/AMD64.rules
index dd50dd2..abe1035 100644
--- a/src/cmd/compile/internal/ssa/gen/AMD64.rules
+++ b/src/cmd/compile/internal/ssa/gen/AMD64.rules
@@ -281,6 +281,9 @@
(Store [2] ptr val mem) -> (MOVWstore ptr val mem)
(Store [1] ptr val mem) -> (MOVBstore ptr val mem)
+// We want this to stick out so the to/from ptr conversion is obvious
+(Convert <t> x) -> (LEAQ <t> x)
+
// checks
(IsNonNil p) -> (SETNE (TESTQ p p))
(IsInBounds idx len) -> (SETB (CMPQ idx len))
diff --git a/src/cmd/compile/internal/ssa/gen/genericOps.go b/src/cmd/compile/internal/ssa/gen/genericOps.go
index 5881596..8a8837c 100644
--- a/src/cmd/compile/internal/ssa/gen/genericOps.go
+++ b/src/cmd/compile/internal/ssa/gen/genericOps.go
@@ -237,8 +237,9 @@
{name: "Sqrt"}, // sqrt(arg0), float64 only
// Data movement
- {name: "Phi"}, // select an argument based on which predecessor block we came from
- {name: "Copy"}, // output = arg0
+ {name: "Phi"}, // select an argument based on which predecessor block we came from
+ {name: "Copy"}, // output = arg0
+ {name: "Convert"}, // output = arg0 -- a copy that converts to/from a pointer
// constants. Constant values are stored in the aux field.
// booleans have a bool aux field, strings have a string aux
diff --git a/src/cmd/compile/internal/ssa/opGen.go b/src/cmd/compile/internal/ssa/opGen.go
index d86dce3..4c19180 100644
--- a/src/cmd/compile/internal/ssa/opGen.go
+++ b/src/cmd/compile/internal/ssa/opGen.go
@@ -455,6 +455,7 @@
OpSqrt
OpPhi
OpCopy
+ OpConvert
OpConstBool
OpConstString
OpConstNil
@@ -3867,6 +3868,10 @@
generic: true,
},
{
+ name: "Convert",
+ generic: true,
+ },
+ {
name: "ConstBool",
generic: true,
},
diff --git a/src/cmd/compile/internal/ssa/rewriteAMD64.go b/src/cmd/compile/internal/ssa/rewriteAMD64.go
index 2fd9a08..3fe272c 100644
--- a/src/cmd/compile/internal/ssa/rewriteAMD64.go
+++ b/src/cmd/compile/internal/ssa/rewriteAMD64.go
@@ -1670,6 +1670,24 @@
goto endc395c0a53eeccf597e225a07b53047d1
endc395c0a53eeccf597e225a07b53047d1:
;
+ case OpConvert:
+ // match: (Convert <t> x)
+ // cond:
+ // result: (LEAQ <t> x)
+ {
+ t := v.Type
+ x := v.Args[0]
+ v.Op = OpAMD64LEAQ
+ v.AuxInt = 0
+ v.Aux = nil
+ v.resetArgs()
+ v.Type = t
+ v.AddArg(x)
+ return true
+ }
+ goto end1cac40a6074914d6ae3d4aa039a625ed
+ end1cac40a6074914d6ae3d4aa039a625ed:
+ ;
case OpCvt32Fto32:
// match: (Cvt32Fto32 x)
// cond:
diff --git a/src/cmd/compile/internal/ssa/tighten.go b/src/cmd/compile/internal/ssa/tighten.go
index 1da5071..4fa26d2 100644
--- a/src/cmd/compile/internal/ssa/tighten.go
+++ b/src/cmd/compile/internal/ssa/tighten.go
@@ -54,8 +54,12 @@
for _, b := range f.Blocks {
for i := 0; i < len(b.Values); i++ {
v := b.Values[i]
- if v.Op == OpPhi || v.Op == OpGetClosurePtr {
- // GetClosurePtr must stay in entry block
+ if v.Op == OpPhi || v.Op == OpGetClosurePtr || v.Op == OpConvert {
+ // GetClosurePtr must stay in entry block.
+ // OpConvert must not float over call sites.
+ // TODO do we instead need a dependence edge of some sort for OpConvert?
+ // Would memory do the trick, or do we need something else that relates
+ // to safe point operations?
continue
}
if len(v.Args) > 0 && v.Args[len(v.Args)-1].Type.IsMemory() {
