[release-branch.go1.8] 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: David Chase <drchase@google.com>

diff --git a/src/cmd/compile/internal/ssa/gen/ARM64.rules b/src/cmd/compile/internal/ssa/gen/ARM64.rules
index bc58a1f5f5fcdc5243a87fc73a847e49eda37f7f..ba7b00a6410ae2cbe3daf834cd23fa1445934400 100644 (file)
--- a/src/cmd/compile/internal/ssa/gen/ARM64.rules
+++ b/src/cmd/compile/internal/ssa/gen/ARM64.rules
@@ -679,14 +679,14 @@
 (MOVWstore [off] {sym} ptr (MOVDconst [0]) mem) -> (MOVWstorezero [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
-//(MOVBload [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) -> x
-//(MOVHload [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) -> x
-//(MOVWload [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) -> 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) -> (MOVBUreg 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) -> (MOVHUreg 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) -> (MOVWUreg 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
 //(FMOVDload [off] {sym} ptr (FMOVDstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> x

diff --git a/src/cmd/compile/internal/ssa/gen/MIPS.rules b/src/cmd/compile/internal/ssa/gen/MIPS.rules
index ad7a95497308b7c2fb74166c3e12619069102355..c231cd833f81648643cb20324319c4eb5073f87c 100644 (file)
--- a/src/cmd/compile/internal/ssa/gen/MIPS.rules
+++ b/src/cmd/compile/internal/ssa/gen/MIPS.rules
@@ -522,11 +522,11 @@
 (MOVWstorezero [off1] {sym1} (MOVWaddr [off2] {sym2} ptr) mem) && canMergeSym(sym1,sym2) ->
        (MOVWstorezero [off1+off2] {mergeSym(sym1,sym2)} ptr mem)
 
-// replace load from same location as preceding store with copy
-(MOVBload [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) && !isSigned(x.Type) -> x
-(MOVHload [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) && !isSigned(x.Type) -> x
+// 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) -> (MOVBreg 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) -> (MOVHreg 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
 (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

diff --git a/src/cmd/compile/internal/ssa/rewriteMIPS.go b/src/cmd/compile/internal/ssa/rewriteMIPS.go
index 76eac5b1cf9cccec777ef5afca3b7b2c2fad2c9c..2590611ab2b5fc3d2cb2d0df3e3827d8cd99f30e 100644 (file)
--- a/src/cmd/compile/internal/ssa/rewriteMIPS.go
+++ b/src/cmd/compile/internal/ssa/rewriteMIPS.go
@@ -3332,8 +3332,8 @@ func rewriteValueMIPS_OpMIPSMOVBUload(v *Value, config *Config) bool {
                return true
        }
        // match: (MOVBUload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _))
-       // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && !isSigned(x.Type)
-       // result: x
+       // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)
+       // result: (MOVBUreg x)
        for {
                off := v.AuxInt
                sym := v.Aux
@@ -3346,11 +3346,10 @@ func rewriteValueMIPS_OpMIPSMOVBUload(v *Value, config *Config) bool {
                sym2 := v_1.Aux
                ptr2 := v_1.Args[0]
                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
                }
-               v.reset(OpCopy)
-               v.Type = x.Type
+               v.reset(OpMIPSMOVBUreg)
                v.AddArg(x)
                return true
        }
@@ -3490,8 +3489,8 @@ func rewriteValueMIPS_OpMIPSMOVBload(v *Value, config *Config) bool {
                return true
        }
        // match: (MOVBload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _))
-       // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && isSigned(x.Type)
-       // result: x
+       // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)
+       // result: (MOVBreg x)
        for {
                off := v.AuxInt
                sym := v.Aux
@@ -3504,11 +3503,10 @@ func rewriteValueMIPS_OpMIPSMOVBload(v *Value, config *Config) bool {
                sym2 := v_1.Aux
                ptr2 := v_1.Args[0]
                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
                }
-               v.reset(OpCopy)
-               v.Type = x.Type
+               v.reset(OpMIPSMOVBreg)
                v.AddArg(x)
                return true
        }
@@ -4148,8 +4146,8 @@ func rewriteValueMIPS_OpMIPSMOVHUload(v *Value, config *Config) bool {
                return true
        }
        // match: (MOVHUload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _))
-       // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && !isSigned(x.Type)
-       // result: x
+       // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)
+       // result: (MOVHUreg x)
        for {
                off := v.AuxInt
                sym := v.Aux
@@ -4162,11 +4160,10 @@ func rewriteValueMIPS_OpMIPSMOVHUload(v *Value, config *Config) bool {
                sym2 := v_1.Aux
                ptr2 := v_1.Args[0]
                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
                }
-               v.reset(OpCopy)
-               v.Type = x.Type
+               v.reset(OpMIPSMOVHUreg)
                v.AddArg(x)
                return true
        }
@@ -4330,8 +4327,8 @@ func rewriteValueMIPS_OpMIPSMOVHload(v *Value, config *Config) bool {
                return true
        }
        // match: (MOVHload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _))
-       // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) && isSigned(x.Type)
-       // result: x
+       // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)
+       // result: (MOVHreg x)
        for {
                off := v.AuxInt
                sym := v.Aux
@@ -4344,11 +4341,10 @@ func rewriteValueMIPS_OpMIPSMOVHload(v *Value, config *Config) bool {
                sym2 := v_1.Aux
                ptr2 := v_1.Args[0]
                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
                }
-               v.reset(OpCopy)
-               v.Type = x.Type
+               v.reset(OpMIPSMOVHreg)
                v.AddArg(x)
                return true
        }

diff --git a/test/fixedbugs/issue20530.go b/test/fixedbugs/issue20530.go
index 7027b1074540fe9bd26c2ab3af63715741be20a5..51f0bd8e39d1632f0973b9bb2026b35a977c0d8f 100644 (file)
--- a/test/fixedbugs/issue20530.go
+++ b/test/fixedbugs/issue20530.go
@@ -8,7 +8,8 @@ package main
 
 var a uint8
 
-func main() {
+//go:noinline
+func f() {
        b := int8(func() int32 { return -1 }())
        a = uint8(b)
        if int32(a) != 255 {
@@ -16,3 +17,18 @@ func main() {
                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()
+}