cmd/compile: match Aux and AuxInt explicitly in store combining rule

CL 280456 introduced a new store combining rule. On the LHS some
of the Aux and AuxInt of the stores are not specified, therefore
ignored during the matching. The rule is only correct if they
match. This CL adds explict match.

TODO: maybe we want the rule matcher require Aux/AuxInt to be
always specified on the LHS (using _ to explicitly ignore)? Or
maybe we want it to match the zero value if not specified? The
current approach is error-prone.

Fixes #44823.

Change-Id: Ic12b4a0de63117f2f070039737f0c905f28561bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/299289
Trust: Cherry Zhang <cherryyz@google.com>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>

diff --git a/src/cmd/compile/internal/ssa/gen/AMD64.rules b/src/cmd/compile/internal/ssa/gen/AMD64.rules
index bab9cee88c3e9775668a630e96ea3ed98aa2c454..7b03034bb7bee27e151a7b63c684a6d34bfaccb3 100644 (file)
--- a/src/cmd/compile/internal/ssa/gen/AMD64.rules
+++ b/src/cmd/compile/internal/ssa/gen/AMD64.rules
@@ -1970,15 +1970,15 @@
   && clobber(x)
   => (MOVQstore [i] {s} p0 w0 mem)
 
-(MOVBstore [7] p1 (SHRQconst [56] w)
-  x1:(MOVWstore [5] p1 (SHRQconst [40] w)
-  x2:(MOVLstore [1] p1 (SHRQconst [8] w)
-  x3:(MOVBstore p1 w mem))))
+(MOVBstore [7] {s} p1 (SHRQconst [56] w)
+  x1:(MOVWstore [5] {s} p1 (SHRQconst [40] w)
+  x2:(MOVLstore [1] {s} p1 (SHRQconst [8] w)
+  x3:(MOVBstore [0] {s} p1 w mem))))
   && x1.Uses == 1
   && x2.Uses == 1
   && x3.Uses == 1
   && clobber(x1, x2, x3)
-  => (MOVQstore p1 w mem)
+  => (MOVQstore {s} p1 w mem)
 
 (MOVBstore [i] {s} p
   x1:(MOVBload [j] {s2} p2 mem)

diff --git a/src/cmd/compile/internal/ssa/gen/S390X.rules b/src/cmd/compile/internal/ssa/gen/S390X.rules
index e4a1cd69812eb6419ee6ceb7e9e6c2d1474a9617..1f75f78a710b60556ba8d06c02f21902bfdd77d5 100644 (file)
--- a/src/cmd/compile/internal/ssa/gen/S390X.rules
+++ b/src/cmd/compile/internal/ssa/gen/S390X.rules
@@ -1422,15 +1422,15 @@
   && clobber(x)
   => (MOVDBRstore [i-4] {s} p w0 mem)
 
-(MOVBstore [7] p1 (SRDconst w)
-  x1:(MOVHBRstore [5] p1 (SRDconst w)
-  x2:(MOVWBRstore [1] p1 (SRDconst w)
-  x3:(MOVBstore p1 w mem))))
+(MOVBstore [7] {s} p1 (SRDconst w)
+  x1:(MOVHBRstore [5] {s} p1 (SRDconst w)
+  x2:(MOVWBRstore [1] {s} p1 (SRDconst w)
+  x3:(MOVBstore [0] {s} p1 w mem))))
   && x1.Uses == 1
   && x2.Uses == 1
   && x3.Uses == 1
   && clobber(x1, x2, x3)
-  => (MOVDBRstore p1 w mem)
+  => (MOVDBRstore {s} p1 w mem)
 
 // Combining byte loads into larger (unaligned) loads.
 

diff --git a/src/cmd/compile/internal/ssa/rewriteAMD64.go b/src/cmd/compile/internal/ssa/rewriteAMD64.go
index 52d0fd095df2cdc5a70275fcfbde7c6f463d67d8..8da3b28b5c2bea55a6848702b7caa8dfa753a4a8 100644 (file)
--- a/src/cmd/compile/internal/ssa/rewriteAMD64.go
+++ b/src/cmd/compile/internal/ssa/rewriteAMD64.go
@@ -11415,20 +11415,21 @@ func rewriteValueAMD64_OpAMD64MOVBstore(v *Value) bool {
                v.AddArg3(p0, w0, mem)
                return true
        }
-       // match: (MOVBstore [7] p1 (SHRQconst [56] w) x1:(MOVWstore [5] p1 (SHRQconst [40] w) x2:(MOVLstore [1] p1 (SHRQconst [8] w) x3:(MOVBstore p1 w mem))))
+       // match: (MOVBstore [7] {s} p1 (SHRQconst [56] w) x1:(MOVWstore [5] {s} p1 (SHRQconst [40] w) x2:(MOVLstore [1] {s} p1 (SHRQconst [8] w) x3:(MOVBstore [0] {s} p1 w mem))))
        // cond: x1.Uses == 1 && x2.Uses == 1 && x3.Uses == 1 && clobber(x1, x2, x3)
-       // result: (MOVQstore p1 w mem)
+       // result: (MOVQstore {s} p1 w mem)
        for {
                if auxIntToInt32(v.AuxInt) != 7 {
                        break
                }
+               s := auxToSym(v.Aux)
                p1 := v_0
                if v_1.Op != OpAMD64SHRQconst || auxIntToInt8(v_1.AuxInt) != 56 {
                        break
                }
                w := v_1.Args[0]
                x1 := v_2
-               if x1.Op != OpAMD64MOVWstore || auxIntToInt32(x1.AuxInt) != 5 {
+               if x1.Op != OpAMD64MOVWstore || auxIntToInt32(x1.AuxInt) != 5 || auxToSym(x1.Aux) != s {
                        break
                }
                _ = x1.Args[2]
@@ -11440,7 +11441,7 @@ func rewriteValueAMD64_OpAMD64MOVBstore(v *Value) bool {
                        break
                }
                x2 := x1.Args[2]
-               if x2.Op != OpAMD64MOVLstore || auxIntToInt32(x2.AuxInt) != 1 {
+               if x2.Op != OpAMD64MOVLstore || auxIntToInt32(x2.AuxInt) != 1 || auxToSym(x2.Aux) != s {
                        break
                }
                _ = x2.Args[2]
@@ -11452,7 +11453,7 @@ func rewriteValueAMD64_OpAMD64MOVBstore(v *Value) bool {
                        break
                }
                x3 := x2.Args[2]
-               if x3.Op != OpAMD64MOVBstore {
+               if x3.Op != OpAMD64MOVBstore || auxIntToInt32(x3.AuxInt) != 0 || auxToSym(x3.Aux) != s {
                        break
                }
                mem := x3.Args[2]
@@ -11460,6 +11461,7 @@ func rewriteValueAMD64_OpAMD64MOVBstore(v *Value) bool {
                        break
                }
                v.reset(OpAMD64MOVQstore)
+               v.Aux = symToAux(s)
                v.AddArg3(p1, w, mem)
                return true
        }

diff --git a/src/cmd/compile/internal/ssa/rewriteS390X.go b/src/cmd/compile/internal/ssa/rewriteS390X.go
index e0a5ff4cbb2e561f72520b52077461d65fe0a775..85260dace83322ae507495a002d5b7ab7bf27964 100644 (file)
--- a/src/cmd/compile/internal/ssa/rewriteS390X.go
+++ b/src/cmd/compile/internal/ssa/rewriteS390X.go
@@ -8883,20 +8883,21 @@ func rewriteValueS390X_OpS390XMOVBstore(v *Value) bool {
                v.AddArg3(p, w0, mem)
                return true
        }
-       // match: (MOVBstore [7] p1 (SRDconst w) x1:(MOVHBRstore [5] p1 (SRDconst w) x2:(MOVWBRstore [1] p1 (SRDconst w) x3:(MOVBstore p1 w mem))))
+       // match: (MOVBstore [7] {s} p1 (SRDconst w) x1:(MOVHBRstore [5] {s} p1 (SRDconst w) x2:(MOVWBRstore [1] {s} p1 (SRDconst w) x3:(MOVBstore [0] {s} p1 w mem))))
        // cond: x1.Uses == 1 && x2.Uses == 1 && x3.Uses == 1 && clobber(x1, x2, x3)
-       // result: (MOVDBRstore p1 w mem)
+       // result: (MOVDBRstore {s} p1 w mem)
        for {
                if auxIntToInt32(v.AuxInt) != 7 {
                        break
                }
+               s := auxToSym(v.Aux)
                p1 := v_0
                if v_1.Op != OpS390XSRDconst {
                        break
                }
                w := v_1.Args[0]
                x1 := v_2
-               if x1.Op != OpS390XMOVHBRstore || auxIntToInt32(x1.AuxInt) != 5 {
+               if x1.Op != OpS390XMOVHBRstore || auxIntToInt32(x1.AuxInt) != 5 || auxToSym(x1.Aux) != s {
                        break
                }
                _ = x1.Args[2]
@@ -8908,7 +8909,7 @@ func rewriteValueS390X_OpS390XMOVBstore(v *Value) bool {
                        break
                }
                x2 := x1.Args[2]
-               if x2.Op != OpS390XMOVWBRstore || auxIntToInt32(x2.AuxInt) != 1 {
+               if x2.Op != OpS390XMOVWBRstore || auxIntToInt32(x2.AuxInt) != 1 || auxToSym(x2.Aux) != s {
                        break
                }
                _ = x2.Args[2]
@@ -8920,7 +8921,7 @@ func rewriteValueS390X_OpS390XMOVBstore(v *Value) bool {
                        break
                }
                x3 := x2.Args[2]
-               if x3.Op != OpS390XMOVBstore {
+               if x3.Op != OpS390XMOVBstore || auxIntToInt32(x3.AuxInt) != 0 || auxToSym(x3.Aux) != s {
                        break
                }
                mem := x3.Args[2]
@@ -8928,6 +8929,7 @@ func rewriteValueS390X_OpS390XMOVBstore(v *Value) bool {
                        break
                }
                v.reset(OpS390XMOVDBRstore)
+               v.Aux = symToAux(s)
                v.AddArg3(p1, w, mem)
                return true
        }

diff --git a/test/fixedbugs/issue44823.go b/test/fixedbugs/issue44823.go
new file mode 100644 (file)
index 0000000..85811df
--- /dev/null
+++ b/test/fixedbugs/issue44823.go
@@ -0,0 +1,26 @@
+// run
+
+// Copyright 2021 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.
+
+// Issue 44823: miscompilation with store combining.
+
+package main
+
+import "encoding/binary"
+
+//go:noinline
+func Id(a [8]byte) (x [8]byte) {
+       binary.LittleEndian.PutUint64(x[:], binary.LittleEndian.Uint64(a[:]))
+       return
+}
+
+var a = [8]byte{1, 2, 3, 4, 5, 6, 7, 8}
+
+func main() {
+       x := Id(a)
+       if x != a {
+               panic("FAIL")
+       }
+}