Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

JIT: Always look for SIMD fields during promotion #84122

Merged
merged 1 commit into from
Jul 5, 2023

Conversation

jakobbotsch
Copy link
Member

Fix #83749

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 30, 2023
@ghost ghost assigned jakobbotsch Mar 30, 2023
@ghost
Copy link

ghost commented Mar 30, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak
See info in area-owners.md if you want to be subscribed.

Issue Details

Fix #83749

Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@lewing lewing added the area-codeflow for labeling automated codeflow label Apr 4, 2023
@ghost
Copy link

ghost commented Jun 1, 2023

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@ghost ghost closed this Jun 1, 2023
@tannergooding
Copy link
Member

@jakobbotsch, was there something blocking or preventing this from being merged?

@jakobbotsch
Copy link
Member Author

@tannergooding There were some failures that I needed to figure out if were related or not.

@jakobbotsch
Copy link
Member Author

The failing case is with JitStressRegs=2:

[11:07:52] ISSUE: <ASSERT> #411709 D:\a\_work\1\s\src\coreclr\jit\emit.h (1171) - Assertion failed 'reg == _idReg1' in 'JIT.HardwareIntrinsics.Arm._AdvSimd.SimpleTernaryOpTest__AbsoluteDifferenceAdd_Vector128_UInt32+TestStruct:Create():JIT.HardwareIntrinsics.Arm._AdvSimd.SimpleTernaryOpTest__AbsoluteDifferenceAdd_Vector128_UInt32+TestStruct' during 'Generate code' (IL size 208; hash 0xa860f440; FullOpts)

We have the following multireg tree that needs to be unspilled before a return:

N447 (  9,  6) [000090] m------N--z                   t90 =    LCL_VAR   struct<JIT.HardwareIntrinsics.Arm._AdvSimd.SimpleTernaryOpTest__AbsoluteDifferenceWideningUpperAndAdd_Vector128_UInt32+TestStruct, 48>(P) V00 loc0          NA
                                                                simd16 field V00._fld1 (fldOffset=0x0) -> V14 tmp10         (last use)
                                                                simd16 field V00._fld2 (fldOffset=0x10) -> V15 tmp11         (last use)
                                                                simd16 field V00._fld3 (fldOffset=0x20) -> V16 tmp12         d2 (last use) REG NA,d1,d2 $540
                                                            ┌──▌  t90    struct 
N449 ( 10,  7) [000091] -----------                           RETURN    struct REG NA $VN.Void

The local is marked as GTF_SPILLED (z) flag, and the spill flags indicate that the first field needs to be unspilled. But the first field has no register assignment to unspill into.

@jakobbotsch
Copy link
Member Author

#88380 should fix the above issue.

@jakobbotsch
Copy link
Member Author

/azp run runtime-coreclr superpmi-replay, runtime-coreclr jitstress, runtime-coreclr libraries-jitstress

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib

Failures are known according to build analysis.

Some very minor diffs outside tests. arm64 has a handful in libraries.pmi. Diffs are for the normal reasons when promotion is enabled/disabled... Generally more precise liveness means some field writes/reads can sometimes be removed, while sometimes struct copies require larger code. And of course fields can be kept in registers over long ranges.

Copy link
Member

@tannergooding tannergooding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I think this is a net improvement and makes sense for typical usages of the vector types.

@jakobbotsch jakobbotsch merged commit 78c0fda into dotnet:main Jul 5, 2023
@jakobbotsch jakobbotsch deleted the fix-83749 branch July 5, 2023 22:16
@ghost ghost locked as resolved and limited conversation to collaborators Aug 5, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inefficient codegen with struct consisting of 2 SIMD vectors.
3 participants