From 7e9658fea900b286d10c80cec9d53641b37b9f8a Mon Sep 17 00:00:00 2001 From: Michael Munday Date: Thu, 16 Feb 2017 19:53:08 -0500 Subject: [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 TryBot-Result: Gobot Gobot Reviewed-by: Keith Randall Reviewed-on: https://go-review.googlesource.com/70831 Run-TryBot: Russ Cox Reviewed-by: Michael Munday --- src/cmd/compile/internal/gc/testdata/loadstore.go | 108 ++++++++++++++++++++++ src/cmd/compile/internal/ssa/gen/S390X.rules | 6 +- src/cmd/compile/internal/ssa/rewriteS390X.go | 12 +-- 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 [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 } -- cgit v1.2.3-54-g00ecf