Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Arm64] AdvSIMD LoadPairVector64 and LoadPairVector128 #45020

Conversation

echesakov
Copy link
Contributor

This is combined work:

  1. based on Carol's changes in Support mulx returning ValueTuple #37928 (that originally were done as implementation of Bmi2 MultiplyNoFlags2) that adds support for hardware intrinsics returning their result in multiple registers and
  2. my changes that affect Arm64 side of the JIT and implement LoadPairVector64 and LoadPairVector128 ([Arm64] LoadPairVector64 and LoadPairVector128 #39243).

Background: Based on our discussion with Carol we decided to include her changes that enable support for hardware intrinsics multiple registers return value in #37928 minus Bmi2.MultiplyNoFlags2 related changes (this would require an approval of #44926 in an API review meeting) as a part of this PR.

Fixes: #39243

CarolEidt and others added 18 commits November 18, 2020 19:23
…gentree.cpp src/coreclr/src/jit/gentree.h src/coreclr/src/jit/lsra.cpp
…alues in multiple registers in src/coreclr/src/jit/lsra.h src/coreclr/src/jit/lsraarm64.cpp src/coreclr/src/jit/lsraxarch.cpp
… src/tests/JIT/HardwareIntrinsics/Arm/Shared/Helpers.tt
@echesakov echesakov added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Nov 20, 2020
@echesakov echesakov self-assigned this Nov 20, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@@ -218,6 +218,16 @@ void Lowering::LowerBlockStore(GenTreeBlk* blkNode)
#endif
}
}
else if (src->OperIsHWIntrinsic())
Copy link
Contributor Author

@echesakov echesakov Nov 20, 2020

Choose a reason for hiding this comment

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

@CarolEidt I think I need to implement a similar logic on Arm64 to avoid store-reload of returned SIMD values for ldp/ldnp as in the following example:

        2C404410          ldnp    s16, s17, [x0]
        FD000FB0          str     d16, [fp,#24]
        FD0013B1          str     d17, [fp,#32]
        FD400FA8          ldr     d8, [fp,#24]
        FD4013A9          ldr     d9, [fp,#32]

Is it correct understanding?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I believe that's correct. However, this path is only for the case where you wind up with a STORE_BLK, as opposed to a STORE_LCL_VAR. If the lhs is a multi-reg lclVar, you should have the latter.

@echesakov echesakov mentioned this pull request Nov 20, 2020
29 tasks
@ViktorHofer
Copy link
Member

// Auto-generated message

69e114c which was merged 12/7 removed the intermediate src/coreclr/src/ folder. This PR needs to be updated as it touches files in that directory which causes conflicts.

To update your commits you can use this bash script: https://gist.github.com/ViktorHofer/6d24f62abdcddb518b4966ead5ef3783. Feel free to use the comment section of the gist to improve the script for others.

@ghost ghost closed this Jan 25, 2021
@ghost
Copy link

ghost commented Jan 25, 2021

Draft Pull Request was automatically closed for inactivity. It can be manually reopened in the next 30 days if the work resumes.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 24, 2021
@echesakov echesakov deleted the Arm64-ASIMD-LoadPairVector64-LoadPairVector128 branch April 16, 2021 23:54
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI new-api-needs-documentation
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants