0
1
mirror of https://github.com/golang/go synced 2025-02-19 03:19:07 +00:00

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 .

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>
This commit is contained in:
Cherry Zhang
2021-03-05 15:12:37 -05:00
parent d85083911d
commit a829114b21
5 changed files with 50 additions and 20 deletions
src/cmd/compile/internal/ssa
test/fixedbugs

@ -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)

@ -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.

@ -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
}

@ -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
}

@ -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")
}
}