[release-branch.go1.8] cmd/compile: zero extend when replacing load-hit-store on...

Keith pointed out that these rules should zero extend during the review
of CL 36845. In practice the generic rules are responsible for eliminating
most load-hit-stores and they do not have this problem. When the s390x
rules are triggered any cast following the elided load-hit-store is
kept because of the sequence the rules are applied in (i.e. the load is
removed before the zero extension gets a chance to be merged into the load).
It is therefore not clear that this issue results in any functional bugs.

This CL includes a test, but it only tests the generic rules currently.

Change-Id: Idbc43c782097a3fb159be293ec3138c5b36858ad
Reviewed-on: https://go-review.googlesource.com/37154
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-on: https://go-review.googlesource.com/70831
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>

diff --git a/src/cmd/compile/internal/gc/testdata/loadstore.go b/src/cmd/compile/internal/gc/testdata/loadstore.go
index 4d67864a6d5baf77ec30d5a392d8261ee0fe5a3e..dcb61d4b7eb66941cddbe033b7ac1b701ef1528f 100644 (file)
--- a/src/cmd/compile/internal/gc/testdata/loadstore.go
+++ b/src/cmd/compile/internal/gc/testdata/loadstore.go
@@ -102,12 +102,120 @@ func testDeadStorePanic() {
        }
 }
 
+//go:noinline
+func loadHitStore8(x int8, p *int8) int32 {
+       x *= x           // try to trash high bits (arch-dependent)
+       *p = x           // store
+       return int32(*p) // load and cast
+}
+
+//go:noinline
+func loadHitStoreU8(x uint8, p *uint8) uint32 {
+       x *= x            // try to trash high bits (arch-dependent)
+       *p = x            // store
+       return uint32(*p) // load and cast
+}
+
+//go:noinline
+func loadHitStore16(x int16, p *int16) int32 {
+       x *= x           // try to trash high bits (arch-dependent)
+       *p = x           // store
+       return int32(*p) // load and cast
+}
+
+//go:noinline
+func loadHitStoreU16(x uint16, p *uint16) uint32 {
+       x *= x            // try to trash high bits (arch-dependent)
+       *p = x            // store
+       return uint32(*p) // load and cast
+}
+
+//go:noinline
+func loadHitStore32(x int32, p *int32) int64 {
+       x *= x           // try to trash high bits (arch-dependent)
+       *p = x           // store
+       return int64(*p) // load and cast
+}
+
+//go:noinline
+func loadHitStoreU32(x uint32, p *uint32) uint64 {
+       x *= x            // try to trash high bits (arch-dependent)
+       *p = x            // store
+       return uint64(*p) // load and cast
+}
+
+func testLoadHitStore() {
+       // Test that sign/zero extensions are kept when a load-hit-store
+       // is replaced by a register-register move.
+       {
+               var in int8 = (1 << 6) + 1
+               var p int8
+               got := loadHitStore8(in, &p)
+               want := int32(in * in)
+               if got != want {
+                       fmt.Println("testLoadHitStore (int8) failed. want =", want, ", got =", got)
+                       failed = true
+               }
+       }
+       {
+               var in uint8 = (1 << 6) + 1
+               var p uint8
+               got := loadHitStoreU8(in, &p)
+               want := uint32(in * in)
+               if got != want {
+                       fmt.Println("testLoadHitStore (uint8) failed. want =", want, ", got =", got)
+                       failed = true
+               }
+       }
+       {
+               var in int16 = (1 << 10) + 1
+               var p int16
+               got := loadHitStore16(in, &p)
+               want := int32(in * in)
+               if got != want {
+                       fmt.Println("testLoadHitStore (int16) failed. want =", want, ", got =", got)
+                       failed = true
+               }
+       }
+       {
+               var in uint16 = (1 << 10) + 1
+               var p uint16
+               got := loadHitStoreU16(in, &p)
+               want := uint32(in * in)
+               if got != want {
+                       fmt.Println("testLoadHitStore (uint16) failed. want =", want, ", got =", got)
+                       failed = true
+               }
+       }
+       {
+               var in int32 = (1 << 30) + 1
+               var p int32
+               got := loadHitStore32(in, &p)
+               want := int64(in * in)
+               if got != want {
+                       fmt.Println("testLoadHitStore (int32) failed. want =", want, ", got =", got)
+                       failed = true
+               }
+       }
+       {
+               var in uint32 = (1 << 30) + 1
+               var p uint32
+               got := loadHitStoreU32(in, &p)
+               want := uint64(in * in)
+               if got != want {
+                       fmt.Println("testLoadHitStore (uint32) failed. want =", want, ", got =", got)
+                       failed = true
+               }
+       }
+}
+
 func main() {
 
        testLoadStoreOrder()
        testStoreSize()
        testExtStore()
        testDeadStorePanic()
+       testLoadHitStore()
 
        if failed {
                panic("failed")

diff --git a/src/cmd/compile/internal/ssa/gen/S390X.rules b/src/cmd/compile/internal/ssa/gen/S390X.rules
index caea0506fa979b83e90c41bd4d495c920445ed55..8aaa505c7b75333d673633d83fcb6207fecacda0 100644 (file)
--- a/src/cmd/compile/internal/ssa/gen/S390X.rules
+++ b/src/cmd/compile/internal/ssa/gen/S390X.rules
@@ -656,9 +656,9 @@
 (MOVWZreg x:(MOVWZloadidx [off] {sym} ptr idx mem)) && x.Uses == 1 && clobber(x) -> @x.Block (MOVWZloadidx <v.Type> [off] {sym} ptr idx mem)
 
 // replace load from same location as preceding store with copy
-(MOVBZload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVDreg x)
-(MOVHZload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVDreg x)
-(MOVWZload [off] {sym} ptr (MOVWstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVDreg x)
+(MOVBZload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVBZreg x)
+(MOVHZload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVHZreg x)
+(MOVWZload [off] {sym} ptr (MOVWstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVWZreg x)
 (MOVDload [off] {sym} ptr (MOVDstore [off2] {sym2} ptr2 x _)) && sym == sym2 && off == off2 && isSamePtr(ptr, ptr2) -> (MOVDreg x)
 
 // Don't extend before storing

diff --git a/src/cmd/compile/internal/ssa/rewriteS390X.go b/src/cmd/compile/internal/ssa/rewriteS390X.go
index af8fd1d45613aa0a1976c467420588f839edaae8..81469a4b2271c16cf062c2aa295e435268a4d319 100644 (file)
--- a/src/cmd/compile/internal/ssa/rewriteS390X.go
+++ b/src/cmd/compile/internal/ssa/rewriteS390X.go
@@ -7851,7 +7851,7 @@ func rewriteValueS390X_OpS390XMOVBZload(v *Value, config *Config) bool {
        _ = b
        // match: (MOVBZload [off] {sym} ptr (MOVBstore [off2] {sym2} ptr2 x _))
        // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)
-       // result: (MOVDreg x)
+       // result: (MOVBZreg x)
        for {
                off := v.AuxInt
                sym := v.Aux
@@ -7867,7 +7867,7 @@ func rewriteValueS390X_OpS390XMOVBZload(v *Value, config *Config) bool {
                if !(sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)) {
                        break
                }
-               v.reset(OpS390XMOVDreg)
+               v.reset(OpS390XMOVBZreg)
                v.AddArg(x)
                return true
        }
@@ -11021,7 +11021,7 @@ func rewriteValueS390X_OpS390XMOVHZload(v *Value, config *Config) bool {
        _ = b
        // match: (MOVHZload [off] {sym} ptr (MOVHstore [off2] {sym2} ptr2 x _))
        // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)
-       // result: (MOVDreg x)
+       // result: (MOVHZreg x)
        for {
                off := v.AuxInt
                sym := v.Aux
@@ -11037,7 +11037,7 @@ func rewriteValueS390X_OpS390XMOVHZload(v *Value, config *Config) bool {
                if !(sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)) {
                        break
                }
-               v.reset(OpS390XMOVDreg)
+               v.reset(OpS390XMOVHZreg)
                v.AddArg(x)
                return true
        }
@@ -12406,7 +12406,7 @@ func rewriteValueS390X_OpS390XMOVWZload(v *Value, config *Config) bool {
        _ = b
        // match: (MOVWZload [off] {sym} ptr (MOVWstore [off2] {sym2} ptr2 x _))
        // cond: sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)
-       // result: (MOVDreg x)
+       // result: (MOVWZreg x)
        for {
                off := v.AuxInt
                sym := v.Aux
@@ -12422,7 +12422,7 @@ func rewriteValueS390X_OpS390XMOVWZload(v *Value, config *Config) bool {
                if !(sym == sym2 && off == off2 && isSamePtr(ptr, ptr2)) {
                        break
                }
-               v.reset(OpS390XMOVDreg)
+               v.reset(OpS390XMOVWZreg)
                v.AddArg(x)
                return true
        }