Skip to content

Commit

Permalink
[release-branch.go1.20] cmd/compile: use correct type for byteswaps o…
Browse files Browse the repository at this point in the history 
…n multi-byte stores

Use the type of the store for the byteswap, not the type of the
store's value argument.

Normally when we're storing a 16-bit value, the value being stored is
also typed as 16 bits. But sometimes it is typed as something smaller,
usually because it is the result of an upcast from a smaller value,
and that upcast needs no instructions.

If the type of the store's arg is thinner than the type being stored,
and the byteswap'd value uses that thinner type, and the byteswap'd
value needs to be spilled & restored, that spill/restore happens using
the thinner type, which causes us to lose some of the top bits of the
value.

Fixes #59374

Change-Id: If6ce1e8a76f18bf8e9d79871b6caa438bc3cce4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/481395
Reviewed-by: David Chase <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Run-TryBot: Keith Randall <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
(cherry picked from commit b3bc862)
Reviewed-on: https://go-review.googlesource.com/c/go/+/483176
Auto-Submit: Dmitri Shuralyov <[email protected]>
  • Loading branch information
@randall77 @gopherbot
randall77 authored and gopherbot committed Apr 24, 2023
1 parent ecf7e00 commit 0684cec
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 72 deletions.
12 changes: 6 additions & 6 deletions src/cmd/compile/internal/ssa/_gen/AMD64.rules
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1757,13 +1757,13 @@
x0:(MOVBstore [i-1] {s} p (SHRWconst [8] w) mem))
&& x0.Uses == 1
&& clobber(x0)
=> (MOVWstore [i-1] {s} p (ROLWconst <w.Type> [8] w) mem)
=> (MOVWstore [i-1] {s} p (ROLWconst <typ.UInt16> [8] w) mem)
(MOVBstore [i] {s} p1 w
x0:(MOVBstore [i] {s} p0 (SHRWconst [8] w) mem))
&& x0.Uses == 1
&& sequentialAddresses(p0, p1, 1)
&& clobber(x0)
=> (MOVWstore [i] {s} p0 (ROLWconst <w.Type> [8] w) mem)
=> (MOVWstore [i] {s} p0 (ROLWconst <typ.UInt16> [8] w) mem)

// Combine stores + shifts into bswap and larger (unaligned) stores
(MOVBstore [i] {s} p w
Expand All @@ -1774,7 +1774,7 @@
&& x1.Uses == 1
&& x2.Uses == 1
&& clobber(x0, x1, x2)
=> (MOVLstore [i-3] {s} p (BSWAPL <w.Type> w) mem)
=> (MOVLstore [i-3] {s} p (BSWAPL <typ.UInt32> w) mem)
(MOVBstore [i] {s} p3 w
x2:(MOVBstore [i] {s} p2 (SHRLconst [8] w)
x1:(MOVBstore [i] {s} p1 (SHRLconst [16] w)
Expand All @@ -1786,7 +1786,7 @@
&& sequentialAddresses(p1, p2, 1)
&& sequentialAddresses(p2, p3, 1)
&& clobber(x0, x1, x2)
=> (MOVLstore [i] {s} p0 (BSWAPL <w.Type> w) mem)
=> (MOVLstore [i] {s} p0 (BSWAPL <typ.UInt32> w) mem)

(MOVBstore [i] {s} p w
x6:(MOVBstore [i-1] {s} p (SHRQconst [8] w)
Expand All @@ -1804,7 +1804,7 @@
&& x5.Uses == 1
&& x6.Uses == 1
&& clobber(x0, x1, x2, x3, x4, x5, x6)
=> (MOVQstore [i-7] {s} p (BSWAPQ <w.Type> w) mem)
=> (MOVQstore [i-7] {s} p (BSWAPQ <typ.UInt64> w) mem)
(MOVBstore [i] {s} p7 w
x6:(MOVBstore [i] {s} p6 (SHRQconst [8] w)
x5:(MOVBstore [i] {s} p5 (SHRQconst [16] w)
Expand All @@ -1828,7 +1828,7 @@
&& sequentialAddresses(p5, p6, 1)
&& sequentialAddresses(p6, p7, 1)
&& clobber(x0, x1, x2, x3, x4, x5, x6)
=> (MOVQstore [i] {s} p0 (BSWAPQ <w.Type> w) mem)
=> (MOVQstore [i] {s} p0 (BSWAPQ <typ.UInt64> w) mem)

// Combine constant stores into larger (unaligned) stores.
(MOVBstoreconst [c] {s} p1 x:(MOVBstoreconst [a] {s} p0 mem))
Expand Down
36 changes: 18 additions & 18 deletions src/cmd/compile/internal/ssa/_gen/ARM64.rules
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2821,7 +2821,7 @@
&& x5.Uses == 1
&& x6.Uses == 1
&& clobber(x0, x1, x2, x3, x4, x5, x6)
=> (MOVDstore [i-7] {s} ptr (REV <w.Type> w) mem)
=> (MOVDstore [i-7] {s} ptr (REV <typ.UInt64> w) mem)
(MOVBstore [7] {s} p w
x0:(MOVBstore [6] {s} p (SRLconst [8] w)
x1:(MOVBstore [5] {s} p (SRLconst [16] w)
Expand All @@ -2841,7 +2841,7 @@
&& (isSamePtr(ptr0, ptr1) && isSamePtr(idx0, idx1) || isSamePtr(ptr0, idx1) && isSamePtr(idx0, ptr1))
&& isSamePtr(p1, p)
&& clobber(x0, x1, x2, x3, x4, x5, x6)
=> (MOVDstoreidx ptr0 idx0 (REV <w.Type> w) mem)
=> (MOVDstoreidx ptr0 idx0 (REV <typ.UInt64> w) mem)
(MOVBstore [i] {s} ptr w
x0:(MOVBstore [i-1] {s} ptr (UBFX [armBFAuxInt(8, 24)] w)
x1:(MOVBstore [i-2] {s} ptr (UBFX [armBFAuxInt(16, 16)] w)
Expand All @@ -2850,7 +2850,7 @@
&& x1.Uses == 1
&& x2.Uses == 1
&& clobber(x0, x1, x2)
=> (MOVWstore [i-3] {s} ptr (REVW <w.Type> w) mem)
=> (MOVWstore [i-3] {s} ptr (REVW <typ.UInt32> w) mem)
(MOVBstore [3] {s} p w
x0:(MOVBstore [2] {s} p (UBFX [armBFAuxInt(8, 24)] w)
x1:(MOVBstore [1] {s} p1:(ADD ptr1 idx1) (UBFX [armBFAuxInt(16, 16)] w)
Expand All @@ -2862,7 +2862,7 @@
&& (isSamePtr(ptr0, ptr1) && isSamePtr(idx0, idx1) || isSamePtr(ptr0, idx1) && isSamePtr(idx0, ptr1))
&& isSamePtr(p1, p)
&& clobber(x0, x1, x2)
=> (MOVWstoreidx ptr0 idx0 (REVW <w.Type> w) mem)
=> (MOVWstoreidx ptr0 idx0 (REVW <typ.UInt32> w) mem)
(MOVBstoreidx ptr (ADDconst [3] idx) w
x0:(MOVBstoreidx ptr (ADDconst [2] idx) (UBFX [armBFAuxInt(8, 24)] w)
x1:(MOVBstoreidx ptr (ADDconst [1] idx) (UBFX [armBFAuxInt(16, 16)] w)
Expand All @@ -2871,7 +2871,7 @@
&& x1.Uses == 1
&& x2.Uses == 1
&& clobber(x0, x1, x2)
=> (MOVWstoreidx ptr idx (REVW <w.Type> w) mem)
=> (MOVWstoreidx ptr idx (REVW <typ.UInt32> w) mem)
(MOVBstoreidx ptr idx w
x0:(MOVBstoreidx ptr (ADDconst [1] idx) (UBFX [armBFAuxInt(8, 24)] w)
x1:(MOVBstoreidx ptr (ADDconst [2] idx) (UBFX [armBFAuxInt(16, 16)] w)
Expand All @@ -2889,7 +2889,7 @@
&& x1.Uses == 1
&& x2.Uses == 1
&& clobber(x0, x1, x2)
=> (MOVWstore [i-3] {s} ptr (REVW <w.Type> w) mem)
=> (MOVWstore [i-3] {s} ptr (REVW <typ.UInt32> w) mem)
(MOVBstore [3] {s} p w
x0:(MOVBstore [2] {s} p (SRLconst [8] (MOVDreg w))
x1:(MOVBstore [1] {s} p1:(ADD ptr1 idx1) (SRLconst [16] (MOVDreg w))
Expand All @@ -2901,7 +2901,7 @@
&& (isSamePtr(ptr0, ptr1) && isSamePtr(idx0, idx1) || isSamePtr(ptr0, idx1) && isSamePtr(idx0, ptr1))
&& isSamePtr(p1, p)
&& clobber(x0, x1, x2)
=> (MOVWstoreidx ptr0 idx0 (REVW <w.Type> w) mem)
=> (MOVWstoreidx ptr0 idx0 (REVW <typ.UInt32> w) mem)
(MOVBstore [i] {s} ptr w
x0:(MOVBstore [i-1] {s} ptr (SRLconst [8] w)
x1:(MOVBstore [i-2] {s} ptr (SRLconst [16] w)
Expand All @@ -2910,7 +2910,7 @@
&& x1.Uses == 1
&& x2.Uses == 1
&& clobber(x0, x1, x2)
=> (MOVWstore [i-3] {s} ptr (REVW <w.Type> w) mem)
=> (MOVWstore [i-3] {s} ptr (REVW <typ.UInt32> w) mem)
(MOVBstore [3] {s} p w
x0:(MOVBstore [2] {s} p (SRLconst [8] w)
x1:(MOVBstore [1] {s} p1:(ADD ptr1 idx1) (SRLconst [16] w)
Expand All @@ -2922,55 +2922,55 @@
&& (isSamePtr(ptr0, ptr1) && isSamePtr(idx0, idx1) || isSamePtr(ptr0, idx1) && isSamePtr(idx0, ptr1))
&& isSamePtr(p1, p)
&& clobber(x0, x1, x2)
=> (MOVWstoreidx ptr0 idx0 (REVW <w.Type> w) mem)
=> (MOVWstoreidx ptr0 idx0 (REVW <typ.UInt32> w) mem)
(MOVBstore [i] {s} ptr w x:(MOVBstore [i-1] {s} ptr (SRLconst [8] w) mem))
&& x.Uses == 1
&& clobber(x)
=> (MOVHstore [i-1] {s} ptr (REV16W <w.Type> w) mem)
=> (MOVHstore [i-1] {s} ptr (REV16W <typ.UInt16> w) mem)
(MOVBstore [1] {s} (ADD ptr1 idx1) w x:(MOVBstoreidx ptr0 idx0 (SRLconst [8] w) mem))
&& x.Uses == 1
&& s == nil
&& (isSamePtr(ptr0, ptr1) && isSamePtr(idx0, idx1) || isSamePtr(ptr0, idx1) && isSamePtr(idx0, ptr1))
&& clobber(x)
=> (MOVHstoreidx ptr0 idx0 (REV16W <w.Type> w) mem)
=> (MOVHstoreidx ptr0 idx0 (REV16W <typ.UInt16> w) mem)
(MOVBstore [i] {s} ptr w x:(MOVBstore [i-1] {s} ptr (UBFX [armBFAuxInt(8, 8)] w) mem))
&& x.Uses == 1
&& clobber(x)
=> (MOVHstore [i-1] {s} ptr (REV16W <w.Type> w) mem)
=> (MOVHstore [i-1] {s} ptr (REV16W <typ.UInt16> w) mem)
(MOVBstore [1] {s} (ADD ptr1 idx1) w x:(MOVBstoreidx ptr0 idx0 (UBFX [armBFAuxInt(8, 8)] w) mem))
&& x.Uses == 1
&& s == nil
&& (isSamePtr(ptr0, ptr1) && isSamePtr(idx0, idx1) || isSamePtr(ptr0, idx1) && isSamePtr(idx0, ptr1))
&& clobber(x)
=> (MOVHstoreidx ptr0 idx0 (REV16W <w.Type> w) mem)
=> (MOVHstoreidx ptr0 idx0 (REV16W <typ.UInt16> w) mem)
(MOVBstoreidx ptr (ADDconst [1] idx) w x:(MOVBstoreidx ptr idx (UBFX [armBFAuxInt(8, 8)] w) mem))
&& x.Uses == 1
&& clobber(x)
=> (MOVHstoreidx ptr idx (REV16W <w.Type> w) mem)
=> (MOVHstoreidx ptr idx (REV16W <typ.UInt16> w) mem)
(MOVBstoreidx ptr idx w x:(MOVBstoreidx ptr (ADDconst [1] idx) (UBFX [armBFAuxInt(8, 8)] w) mem))
&& x.Uses == 1
&& clobber(x)
=> (MOVHstoreidx ptr idx w mem)
(MOVBstore [i] {s} ptr w x:(MOVBstore [i-1] {s} ptr (SRLconst [8] (MOVDreg w)) mem))
&& x.Uses == 1
&& clobber(x)
=> (MOVHstore [i-1] {s} ptr (REV16W <w.Type> w) mem)
=> (MOVHstore [i-1] {s} ptr (REV16W <typ.UInt16> w) mem)
(MOVBstore [1] {s} (ADD ptr1 idx1) w x:(MOVBstoreidx ptr0 idx0 (SRLconst [8] (MOVDreg w)) mem))
&& x.Uses == 1
&& s == nil
&& (isSamePtr(ptr0, ptr1) && isSamePtr(idx0, idx1) || isSamePtr(ptr0, idx1) && isSamePtr(idx0, ptr1))
&& clobber(x)
=> (MOVHstoreidx ptr0 idx0 (REV16W <w.Type> w) mem)
=> (MOVHstoreidx ptr0 idx0 (REV16W <typ.UInt16> w) mem)
(MOVBstore [i] {s} ptr w x:(MOVBstore [i-1] {s} ptr (UBFX [armBFAuxInt(8, 24)] w) mem))
&& x.Uses == 1
&& clobber(x)
=> (MOVHstore [i-1] {s} ptr (REV16W <w.Type> w) mem)
=> (MOVHstore [i-1] {s} ptr (REV16W <typ.UInt16> w) mem)
(MOVBstore [1] {s} (ADD ptr1 idx1) w x:(MOVBstoreidx ptr0 idx0 (UBFX [armBFAuxInt(8, 24)] w) mem))
&& x.Uses == 1
&& s == nil
&& (isSamePtr(ptr0, ptr1) && isSamePtr(idx0, idx1) || isSamePtr(ptr0, idx1) && isSamePtr(idx0, ptr1))
&& clobber(x)
=> (MOVHstoreidx ptr0 idx0 (REV16W <w.Type> w) mem)
=> (MOVHstoreidx ptr0 idx0 (REV16W <typ.UInt16> w) mem)

// FP simplification
(FNEGS (FMULS x y)) => (FNMULS x y)
Expand Down
24 changes: 12 additions & 12 deletions src/cmd/compile/internal/ssa/rewriteAMD64.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 0684cec

Please sign in to comment.