Skip to content

Commit

Permalink
cmd/compile: use correct type for byteswaps on multi-byte stores
Browse files Browse the repository at this point in the history 
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 golang#59367

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]>
  • Loading branch information
@randall77
randall77 committed Apr 7, 2023
1 parent 42866e5 commit b3bc862
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 @@ -1758,13 +1758,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 @@ -1775,7 +1775,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 @@ -1787,7 +1787,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 @@ -1805,7 +1805,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 @@ -1829,7 +1829,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 @@ -2688,7 +2688,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 @@ -2708,7 +2708,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 @@ -2717,7 +2717,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 @@ -2729,7 +2729,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 @@ -2738,7 +2738,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 @@ -2756,7 +2756,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 @@ -2768,7 +2768,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 @@ -2777,7 +2777,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 @@ -2789,55 +2789,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 b3bc862

Please sign in to comment.