Make BSWAP16 nodes normalize upper 16 bits #67903
Conversation
Currently the JIT's constant folding (gtFoldExprConst and VNs EvalOpSpecialized) assumes that BSWAP16 zero extends into the upper 16 bits. This was not the case, and in fact the behavior of BSWAP16 depended on platform. Normally this would not be a problem since we always insert normalizing casts when creating BSWAP16 nodes, however VN was smart enough to remove this cast in some cases (see the test). Change the semantics of BSWAP16 nodes to zero extend into the upper 16 bits to match constant folding, and add a small peephole to avoid inserting this normalization in the common case where it is not necessary. Fixes dotnet#67723 Subsumes dotnet#67726
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsCurrently the JIT's constant folding (gtFoldExprConst and VNs Normally this would not be a problem since we always insert normalizing Change the semantics of BSWAP16 nodes to zero extend into the upper 16
|
|
Small diffs. These are when we previously removed the cast because we are storing the result directly to an indir. That forces the zero-normalization to appear as the peephole does not recognize this: G_M551_IG05: ; gcrefRegs=00000000 {}, byrefRegs=00000004 {rdx}, byref
ror cx, 8
+ movzx rcx, cx
mov word ptr [rdx], cx
- ;; size=7 bbWeight=1 PerfScore 1.50
+ ;; size=10 bbWeight=1 PerfScore 1.75
G_M551_IG06: ; , epilog, nogc, extend
retSince #66965 improves this and the diffs seem small I don't think it warrants expanding the peephole in this PR to recognize it (let me know if you disagree). cc @dotnet/jit-contrib PTAL @EgorBo |
|
ping @EgorBo |
|
@jakobbotsch Add the test from #67726? |
Whoops, yes, thanks for pointing that out! |
Currently the JIT's constant folding (gtFoldExprConst and VNs EvalOpSpecialized) assumes that BSWAP16 zero extends into the upper 16 bits. This was not the case, and in fact the behavior of BSWAP16 depended on platform. Normally this would not be a problem since we always insert normalizing casts when creating BSWAP16 nodes, however VN was smart enough to remove this cast in some cases (see the test). Change the semantics of BSWAP16 nodes to zero extend into the upper 16 bits to match constant folding, and add a small peephole to avoid inserting this normalization in the common case where it is not necessary. Fixes dotnet#67723 Subsumes dotnet#67726
ghost
locked as resolved and limited conversation to collaborators
Currently the JIT's constant folding (gtFoldExprConst and VNs
EvalOpSpecialized) assumes that BSWAP16 zero extends into the upper 16
bits. This was not the case, and in fact the behavior of BSWAP16
depended on platform.
Normally this would not be a problem since we always insert normalizing
casts when creating BSWAP16 nodes, however VN was smart enough to remove
this cast in some cases (see the test).
Change the semantics of BSWAP16 nodes to zero extend into the upper 16
bits to match constant folding, and add a small peephole to avoid
inserting this normalization in the common case where it is not
necessary.
Fixes #67723
Subsumes #67726