cmd/compile: clean up equality generation
We're using sort.SliceStable, so no need to keep track of indexes as well.
Use a more robust test for whether a node is a call.
Add a test that we're actually reordering comparisons. This test fails
without the alg.go changes in this CL because eqstring uses OCALLFUNC
instead of OCALL for its data comparisons.
Update #8606
Change-Id: Ieeec33434c72e3aa328deb11cc415cfda05632e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/237921
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
diff --git a/src/cmd/compile/internal/gc/alg.go b/src/cmd/compile/internal/gc/alg.go
index e2e2374..2b63700 100644
--- a/src/cmd/compile/internal/gc/alg.go
+++ b/src/cmd/compile/internal/gc/alg.go
@@ -646,17 +646,11 @@
// Build a list of conditions to satisfy.
// The conditions are a list-of-lists. Conditions are reorderable
// within each inner list. The outer lists must be evaluated in order.
- // Even within each inner list, track their order so that we can preserve
- // aspects of that order. (TODO: latter part needed?)
- type nodeIdx struct {
- n *Node
- idx int
- }
- var conds [][]nodeIdx
- conds = append(conds, []nodeIdx{})
+ var conds [][]*Node
+ conds = append(conds, []*Node{})
and := func(n *Node) {
i := len(conds) - 1
- conds[i] = append(conds[i], nodeIdx{n: n, idx: len(conds[i])})
+ conds[i] = append(conds[i], n)
}
// Walk the struct using memequal for runs of AMEM
@@ -674,7 +668,7 @@
if !IsRegularMemory(f.Type) {
if EqCanPanic(f.Type) {
// Enforce ordering by starting a new set of reorderable conditions.
- conds = append(conds, []nodeIdx{})
+ conds = append(conds, []*Node{})
}
p := nodSym(OXDOT, np, f.Sym)
q := nodSym(OXDOT, nq, f.Sym)
@@ -688,7 +682,7 @@
}
if EqCanPanic(f.Type) {
// Also enforce ordering after something that can panic.
- conds = append(conds, []nodeIdx{})
+ conds = append(conds, []*Node{})
}
i++
continue
@@ -713,14 +707,13 @@
// Sort conditions to put runtime calls last.
// Preserve the rest of the ordering.
- var flatConds []nodeIdx
+ var flatConds []*Node
for _, c := range conds {
+ isCall := func(n *Node) bool {
+ return n.Op == OCALL || n.Op == OCALLFUNC
+ }
sort.SliceStable(c, func(i, j int) bool {
- x, y := c[i], c[j]
- if (x.n.Op != OCALL) == (y.n.Op != OCALL) {
- return x.idx < y.idx
- }
- return x.n.Op != OCALL
+ return !isCall(c[i]) && isCall(c[j])
})
flatConds = append(flatConds, c...)
}
@@ -729,9 +722,9 @@
if len(flatConds) == 0 {
cond = nodbool(true)
} else {
- cond = flatConds[0].n
+ cond = flatConds[0]
for _, c := range flatConds[1:] {
- cond = nod(OANDAND, cond, c.n)
+ cond = nod(OANDAND, cond, c)
}
}
diff --git a/test/fixedbugs/issue8606b.go b/test/fixedbugs/issue8606b.go
new file mode 100644
index 0000000..448ea56
--- /dev/null
+++ b/test/fixedbugs/issue8606b.go
@@ -0,0 +1,63 @@
+// run
+
+// Copyright 2020 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.
+
+// This is an optimization check. We want to make sure that we compare
+// string lengths, and other scalar fields, before checking string
+// contents. There's no way to verify this in the language, and
+// codegen tests in test/codegen can't really detect ordering
+// optimizations like this. Instead, we generate invalid strings with
+// bad backing store pointers but nonzero length, so we can check that
+// the backing store never gets compared.
+//
+// We use two different bad strings so that pointer comparisons of
+// backing store pointers fail.
+
+package main
+
+import (
+ "fmt"
+ "reflect"
+ "unsafe"
+)
+
+func bad1() string {
+ s := "foo"
+ (*reflect.StringHeader)(unsafe.Pointer(&s)).Data = 1 // write bad value to data ptr
+ return s
+}
+func bad2() string {
+ s := "foo"
+ (*reflect.StringHeader)(unsafe.Pointer(&s)).Data = 2 // write bad value to data ptr
+ return s
+}
+
+type SI struct {
+ s string
+ i int
+}
+
+type SS struct {
+ s string
+ t string
+}
+
+func main() {
+ for _, test := range []struct {
+ a, b interface{}
+ }{
+ {SI{s: bad1(), i: 1}, SI{s: bad2(), i: 2}},
+ {SS{s: bad1(), t: "a"}, SS{s: bad2(), t: "aa"}},
+ {SS{s: "a", t: bad1()}, SS{s: "b", t: bad2()}},
+ // This one would panic because the length of both strings match, and we check
+ // the body of the bad strings before the body of the good strings.
+ //{SS{s: bad1(), t: "a"}, SS{s: bad2(), t: "b"}},
+ } {
+ if test.a == test.b {
+ panic(fmt.Sprintf("values %#v and %#v should not be equal", test.a, test.b))
+ }
+ }
+
+}
