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

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>

3 files changed, 117 insertions, 9 deletions

diff --git a/src/cmd/compile/internal/gc/testdata/loadstore.go b/src/cmd/compile/internal/gc/testdata/loadstore.go
index 4d67864a6d..dcb61d4b7e 100644
--- 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 caea0506fa..8aaa505c7b 100644
--- 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 af8fd1d456..81469a4b22 100644
--- 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
 	}