Skip to content

G
go
Commit 1e081910 authored by Cherry Zhang's avatar Cherry Zhang
Browse files
Options

cmd/compile: fix subword store/load elision for MIPS 

Apply the fix in CL 44355 to MIPS.

ARM64 has these rules but commented out for performance reason.
Fix the commented rules, in case they are enabled in the future.

Enhance the test so it triggers the failure on ARM and MIPS without
the fix.

Updates #20530.

Change-Id: I82d77448e3939a545fe519d0a29a164f8fa5417c
Reviewed-on: https://go-review.googlesource.com/44430
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarDavid Chase <drchase@google.com>
parent 1948b7f8
Show whitespace changes
Inline Side-by-side
Showing with 45 additions and 33 deletions
src/cmd/compile/internal/ssa/gen/ARM64.rules
View file @ 1e081910
...@@ -668,14 +668,14 @@ ...@@ -668,14 +668,14 @@
(MOVWstore [off] {sym} ptr (MOVDconst [0]) mem) -> (MOVWstorezero [off] {sym} ptr mem) (MOVWstore [off] {sym} ptr (MOVDconst [0]) mem) -> (MOVWstorezero [off] {sym} ptr mem)
(MOVDstore [off] {sym} ptr (MOVDconst [0]) mem) -> (MOVDstorezero [off] {sym} ptr mem) (MOVDstore [off] {sym} ptr (MOVDconst [0]) mem) -> (MOVDstorezero [off] {sym} ptr mem)
// replace load from same location as preceding store with copy // replace load from same location as preceding store with zero/sign extension (or copy in case of full width)
// these seem to have bad interaction with other rules, resulting in slower code // these seem to have bad interaction with other rules, resulting in slower code
//(MOVBload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x //(MOVBload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVBreg x)
//(MOVBUload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x //(MOVBUload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVBUreg x)
//(MOVHload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x //(MOVHload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVHreg x)
//(MOVHUload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x //(MOVHUload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVHUreg x)
//(MOVWload [off] {sym} ptr (MOVWstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x //(MOVWload [off] {sym} ptr (MOVWstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVWreg x)
//(MOVWUload [off] {sym} ptr (MOVWstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x //(MOVWUload [off] {sym} ptr (MOVWstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVWUreg x)
//(MOVDload [off] {sym} ptr (MOVDstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x //(MOVDload [off] {sym} ptr (MOVDstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x
//(FMOVSload [off] {sym} ptr (FMOVSstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x //(FMOVSload [off] {sym} ptr (FMOVSstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x
//(FMOVDload [off] {sym} ptr (FMOVDstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x //(FMOVDload [off] {sym} ptr (FMOVDstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x
......
src/cmd/compile/internal/ssa/gen/MIPS.rules
View file @ 1e081910
...@@ -524,11 +524,11 @@ ...@@ -524,11 +524,11 @@
(MOVWstorezero [off1] {sym1} (MOVWaddr [off2] {sym2} ptr) mem) && canMergeSym(sym1,sym2) -> (MOVWstorezero [off1] {sym1} (MOVWaddr [off2] {sym2} ptr) mem) && canMergeSym(sym1,sym2) ->
(MOVWstorezero [off1+off2] {mergeSym(sym1,sym2)} ptr mem) (MOVWstorezero [off1+off2] {mergeSym(sym1,sym2)} ptr mem)
// replace load from same location as preceding store with copy // replace load from same location as preceding store with zero/sign extension (or copy in case of full width)
(MOVBload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && isSigned(x.Type) -> x (MOVBload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVBreg x)
(MOVBUload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && !isSigned(x.Type) -> x (MOVBUload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVBUreg x)
(MOVHload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && isSigned(x.Type) -> x (MOVHload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVHreg x)
(MOVHUload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && !isSigned(x.Type) -> x (MOVHUload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVHUreg x)
(MOVWload [off] {sym} ptr (MOVWstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x (MOVWload [off] {sym} ptr (MOVWstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x
(MOVFload [off] {sym} ptr (MOVFstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x (MOVFload [off] {sym} ptr (MOVFstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x
(MOVDload [off] {sym} ptr (MOVDstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x (MOVDload [off] {sym} ptr (MOVDstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x
......
src/cmd/compile/internal/ssa/rewriteMIPS.go
View file @ 1e081910
...@@ -3392,8 +3392,8 @@ func rewriteValueMIPS_OpMIPSMOVBUload_0(v *Value) bool { ...@@ -3392,8 +3392,8 @@ func rewriteValueMIPS_OpMIPSMOVBUload_0(v *Value) bool {
return true return true
} }
// match: (MOVBUload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) // match: (MOVBUload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _))
// cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && !isSigned(x.Type) // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)
// result: x // result: (MOVBUreg x)
for { for {
off := v.AuxInt off := v.AuxInt
sym := v.Aux sym := v.Aux
...@@ -3408,11 +3408,10 @@ func rewriteValueMIPS_OpMIPSMOVBUload_0(v *Value) bool { ...@@ -3408,11 +3408,10 @@ func rewriteValueMIPS_OpMIPSMOVBUload_0(v *Value) bool {
_ = v_1.Args[2] _ = v_1.Args[2]
ptr2 := v_1.Args[0] ptr2 := v_1.Args[0]
x := v_1.Args[1] x := v_1.Args[1]
if !(sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && !isSigned(x.Type)) { if !(sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)) {
break break
} }
v.reset(OpCopy) v.reset(OpMIPSMOVBUreg)
v.Type = x.Type
v.AddArg(x) v.AddArg(x)
return true return true
} }
...@@ -3554,8 +3553,8 @@ func rewriteValueMIPS_OpMIPSMOVBload_0(v *Value) bool { ...@@ -3554,8 +3553,8 @@ func rewriteValueMIPS_OpMIPSMOVBload_0(v *Value) bool {
return true return true
} }
// match: (MOVBload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) // match: (MOVBload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _))
// cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && isSigned(x.Type) // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)
// result: x // result: (MOVBreg x)
for { for {
off := v.AuxInt off := v.AuxInt
sym := v.Aux sym := v.Aux
...@@ -3570,11 +3569,10 @@ func rewriteValueMIPS_OpMIPSMOVBload_0(v *Value) bool { ...@@ -3570,11 +3569,10 @@ func rewriteValueMIPS_OpMIPSMOVBload_0(v *Value) bool {
_ = v_1.Args[2] _ = v_1.Args[2]
ptr2 := v_1.Args[0] ptr2 := v_1.Args[0]
x := v_1.Args[1] x := v_1.Args[1]
if !(sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && isSigned(x.Type)) { if !(sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)) {
break break
} }
v.reset(OpCopy) v.reset(OpMIPSMOVBreg)
v.Type = x.Type
v.AddArg(x) v.AddArg(x)
return true return true
} }
...@@ -4226,8 +4224,8 @@ func rewriteValueMIPS_OpMIPSMOVHUload_0(v *Value) bool { ...@@ -4226,8 +4224,8 @@ func rewriteValueMIPS_OpMIPSMOVHUload_0(v *Value) bool {
return true return true
} }
// match: (MOVHUload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) // match: (MOVHUload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _))
// cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && !isSigned(x.Type) // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)
// result: x // result: (MOVHUreg x)
for { for {
off := v.AuxInt off := v.AuxInt
sym := v.Aux sym := v.Aux
...@@ -4242,11 +4240,10 @@ func rewriteValueMIPS_OpMIPSMOVHUload_0(v *Value) bool { ...@@ -4242,11 +4240,10 @@ func rewriteValueMIPS_OpMIPSMOVHUload_0(v *Value) bool {
_ = v_1.Args[2] _ = v_1.Args[2]
ptr2 := v_1.Args[0] ptr2 := v_1.Args[0]
x := v_1.Args[1] x := v_1.Args[1]
if !(sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && !isSigned(x.Type)) { if !(sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)) {
break break
} }
v.reset(OpCopy) v.reset(OpMIPSMOVHUreg)
v.Type = x.Type
v.AddArg(x) v.AddArg(x)
return true return true
} }
...@@ -4413,8 +4410,8 @@ func rewriteValueMIPS_OpMIPSMOVHload_0(v *Value) bool { ...@@ -4413,8 +4410,8 @@ func rewriteValueMIPS_OpMIPSMOVHload_0(v *Value) bool {
return true return true
} }
// match: (MOVHload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) // match: (MOVHload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _))
// cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && isSigned(x.Type) // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)
// result: x // result: (MOVHreg x)
for { for {
off := v.AuxInt off := v.AuxInt
sym := v.Aux sym := v.Aux
...@@ -4429,11 +4426,10 @@ func rewriteValueMIPS_OpMIPSMOVHload_0(v *Value) bool { ...@@ -4429,11 +4426,10 @@ func rewriteValueMIPS_OpMIPSMOVHload_0(v *Value) bool {
_ = v_1.Args[2] _ = v_1.Args[2]
ptr2 := v_1.Args[0] ptr2 := v_1.Args[0]
x := v_1.Args[1] x := v_1.Args[1]
if !(sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && isSigned(x.Type)) { if !(sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)) {
break break
} }
v.reset(OpCopy) v.reset(OpMIPSMOVHreg)
v.Type = x.Type
v.AddArg(x) v.AddArg(x)
return true return true
} }
......
test/fixedbugs/issue20530.go
View file @ 1e081910
...@@ -8,7 +8,8 @@ package main ...@@ -8,7 +8,8 @@ package main
var a uint8 var a uint8
func main() { //go:noinline
func f() {
b := int8(func() int32 { return -1 }()) b := int8(func() int32 { return -1 }())
a = uint8(b) a = uint8(b)
if int32(a) != 255 { if int32(a) != 255 {
...@@ -16,3 +17,18 @@ func main() { ...@@ -16,3 +17,18 @@ func main() {
println("got", a, "expected 255") println("got", a, "expected 255")
} }
} }
//go:noinline
func g() {
b := int8(func() uint32 { return 0xffffffff }())
a = uint8(b)
if int32(a) != 255 {
// Failing case prints 'got 255 expected 255'
println("got", a, "expected 255")
}
}
func main() {
f()
g()
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7