Morph Vector.Create(0) to Vector.Zero
#63821
Conversation
|
This is very close to being ready; only 1 test regressed in perf on x64: - vxorps xmm0, xmm0, xmm0
+ vmovupd xmm0, xmmword ptr [reloc @RWD96]
vmovapd xmmword ptr [rbp-60H], xmm0
- vxorps xmm0, xmm0, xmm0
+ vmovupd xmm0, xmmword ptr [reloc @RWD96]Will investigate. Edit: This was fixed by reverting back the lowering change. Seems we need to have similar optimizations in morph and lowering. |
|
@dotnet/jit-contrib this is ready again. |
…reate-to-get_Zero
| @@ -7651,16 +7651,20 @@ inline bool GenTree::IsSIMDZero() const | |||
| // | |||
| inline bool GenTree::IsFloatPositiveZero() const | |||
| { | |||
| return IsCnsFltOrDbl() && !IsCnsNonZeroFltOrDbl(); | |||
| if (IsCnsFltOrDbl()) | |||
There was a problem hiding this comment.
why is this change? Seems IsCnsNonZeroFltOrDbl already does this?
There was a problem hiding this comment.
It does, but we decided that !IsCnsNonZeroFltOrDbl() is a bit confusing; an explicit implementation seemed to be clearer.
There was a problem hiding this comment.
Perhaps add a comment explaining why you're not using !IsCns ...
| @@ -14349,6 +14432,13 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) | |||
| } | |||
| #endif // defined(FEATURE_HW_INTRINSICS) && defined(TARGET_XARCH) | |||
|
|
|||
| #ifdef FEATURE_HW_INTRINSICS | |||
| if (multiOp->OperIsHWIntrinsic() && !optValnumCSE_phase) | |||
There was a problem hiding this comment.
why is !optValnumCSE_phase check needed?
There was a problem hiding this comment.
It was added because I was deleting nodes; however, I'm not deleting the nodes anymore and instead using ResetHWIntrinsic. I do feel safer having that guard there in-case more optimizations get added in fgOptimizeHWIntrinsic.
Read more in the discussion: #63821 (comment)
|
@jakobbotsch , since you approved this PR: #65028 - and it depends on this one, I'm wondering if you approve of this one as well. |
| @@ -7651,16 +7651,20 @@ inline bool GenTree::IsSIMDZero() const | |||
| // | |||
| inline bool GenTree::IsFloatPositiveZero() const | |||
| { | |||
| return IsCnsFltOrDbl() && !IsCnsNonZeroFltOrDbl(); | |||
| if (IsCnsFltOrDbl()) | |||
There was a problem hiding this comment.
Perhaps add a comment explaining why you're not using !IsCns ...
Sorry I didn't get to it yesterday @TIHan, but looks like Andy unblocked you. |
|
No worries, thank you all for the reviews! |
…reate-to-get_Zero
ghost
locked as resolved and limited conversation to collaborators
Addresses #63490
Currently, this PR is dependent on #62933 due to using
IsFloatPositiveZero.Description
This PR will normalize
Vector.Create(0)calls to simply becomeVector.Zerocalls, for example:will turn into
This happens in the morph phase.
Acceptance Criteria
Vector.CreatecallsSome ARM64 diffs
G_M17803_IG02: ; gcrefRegs=0000 {}, byrefRegs=0000 {}, byref - movi v16.2s, #0x00 - cmeq v16.2s, v0.2s, v16.2s + cmeq v16.2s, v0.2s, #0 mov v0.8b, v16.8b G_M54162_IG02: ; gcrefRegs=0000 {}, byrefRegs=0000 {}, byref - movi v16.4s, #0x00 - cmeq v16.4s, v0.4s, v16.4s + cmeq v16.4s, v0.4s, #0 mov v0.16b, v16.16bSome x64 diffs
(The diff isn't really a regression, a move got shuffled around, but wanted to mention it)