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: Add S.P.CoreLib instrinsics for ReverseBitOrder #27582

Conversation

@TamarChristinaArm
Copy link
Contributor

TamarChristinaArm commented Oct 31, 2019

This implements the ReverseBitOrder intrinsics for Arm64.

See dotnet/corefx#26581 and if first split of #26769
CoreFX ref assembly update at dotnet/corefx#42358

/CC @tannergooding @CarolEidt @echesakovMSFT

Thanks,
Tamar

@tannergooding

This comment has been minimized.

Copy link
Member

tannergooding commented Oct 31, 2019

Until the repo consolidation happens (Nov 22nd according to the new timeline: #27549), the easiest thing to do is to separate this into 3 PRs.

The first only updates S.P.Corelib

This will then flow back into CoreFX and the PR consuming those changes will fail and will require the System.Runtime.Intrinsics.Experimental ref assembly to be updated

This will then flow back into CoreCLR, after which they can be implemented and tests can be added

@TamarChristinaArm

This comment has been minimized.

Copy link
Contributor Author

TamarChristinaArm commented Oct 31, 2019

Until the repo consolidation happens (Nov 22nd according to the new timeline: #27549), the easiest thing to do is to separate this into 3 PRs.

The first only updates S.P.Corelib

Understood.

  • It is generally good to pre-prepare the ref assembly changes so they can be easily cherry-picked: dotnet/corefx#42075

This will then flow back into CoreCLR, after which they can be implemented and tests can be added

So I only update the System.Runtime.Intrinsics.Experimental.cs not the src/Common/src/CoreLib/System/Runtime/Intrinsics/Arm/AdvSimd.cs one? So those are mirrored form the S.P.Corelib PR?

@tannergooding

This comment has been minimized.

Copy link
Member

tannergooding commented Oct 31, 2019

So I only update the System.Runtime.Intrinsics.Experimental.cs not the src/Common/src/CoreLib/System/Runtime/Intrinsics/Arm/AdvSimd.cs one? So those are mirrored form the S.P.Corelib PR?

Correct

@TamarChristinaArm TamarChristinaArm force-pushed the TamarChristinaArm:implement-arm64-reverse-element-bits-upstream branch from 4867b4a to a11134d Oct 31, 2019
Copy link
Member

echesakovMSFT left a comment

LGTM with a few notes.

@jkotas jkotas added the area-CodeGen label Nov 2, 2019
@TamarChristinaArm TamarChristinaArm force-pushed the TamarChristinaArm:implement-arm64-reverse-element-bits-upstream branch from a11134d to 2d6021a Nov 4, 2019
@TamarChristinaArm

This comment has been minimized.

Copy link
Contributor Author

TamarChristinaArm commented Nov 4, 2019

Thanks for the review. I've made the requested changed. Unfortunately can I request this not be merged yet as I'm still waiting for internal approval to contribute to CoreFx. So if committing this will cause a failure over there then it's best to hold off.

I'll continue working on the intrinsics locally and will push them all in batches once everything is agreed.

@TamarChristinaArm TamarChristinaArm changed the title Arm64: Implement ReverseBitOrder intrinsic Arm64: Add S.P.CoreLib instrinsics for ReverseBitOrder Nov 4, 2019
@TamarChristinaArm

This comment has been minimized.

Copy link
Contributor Author

TamarChristinaArm commented Nov 4, 2019

It's all sorted and I have submit a PR to CoreFX.

@tannergooding tannergooding merged commit c2ae7de into dotnet:master Nov 4, 2019
50 checks passed
50 checks passed
WIP Ready for review
Details
coreclr-ci Build #20191104.4 succeeded
Details
coreclr-ci (Build Linux arm checked) Build Linux arm checked succeeded
Details
coreclr-ci (Build Linux arm64 checked) Build Linux arm64 checked succeeded
Details
coreclr-ci (Build Linux arm64 release) Build Linux arm64 release succeeded
Details
coreclr-ci (Build Linux x64 checked) Build Linux x64 checked succeeded
Details
coreclr-ci (Build Linux_musl x64 checked) Build Linux_musl x64 checked succeeded
Details
coreclr-ci (Build Linux_musl x64 release) Build Linux_musl x64 release succeeded
Details
coreclr-ci (Build Linux_rhel6 x64 release) Build Linux_rhel6 x64 release succeeded
Details
coreclr-ci (Build OSX x64 checked) Build OSX x64 checked succeeded
Details
coreclr-ci (Build OSX x64 release) Build OSX x64 release succeeded
Details
coreclr-ci (Build Test Pri0 Linux arm checked) Build Test Pri0 Linux arm checked succeeded
Details
coreclr-ci (Build Test Pri0 Linux arm64 checked) Build Test Pri0 Linux arm64 checked succeeded
Details
coreclr-ci (Build Test Pri0 OSX x64 checked) Build Test Pri0 OSX x64 checked succeeded
Details
coreclr-ci (Build Test Pri0 OSX x64 release) Build Test Pri0 OSX x64 release succeeded
Details
coreclr-ci (Build Test Pri0 Windows_NT arm checked) Build Test Pri0 Windows_NT arm checked succeeded
Details
coreclr-ci (Build Test Pri0 Windows_NT arm64 checked) Build Test Pri0 Windows_NT arm64 checked succeeded
Details
coreclr-ci (Build Test Pri0 Windows_NT x64 checked) Build Test Pri0 Windows_NT x64 checked succeeded
Details
coreclr-ci (Build Test Pri0 Windows_NT x86 checked) Build Test Pri0 Windows_NT x86 checked succeeded
Details
coreclr-ci (Build Windows_NT arm checked) Build Windows_NT arm checked succeeded
Details
coreclr-ci (Build Windows_NT arm release) Build Windows_NT arm release succeeded
Details
coreclr-ci (Build Windows_NT arm64 checked) Build Windows_NT arm64 checked succeeded
Details
coreclr-ci (Build Windows_NT arm64 release) Build Windows_NT arm64 release succeeded
Details
coreclr-ci (Build Windows_NT x64 checked) Build Windows_NT x64 checked succeeded
Details
coreclr-ci (Build Windows_NT x64 debug) Build Windows_NT x64 debug succeeded
Details
coreclr-ci (Build Windows_NT x64 release) Build Windows_NT x64 release succeeded
Details
coreclr-ci (Build Windows_NT x86 checked) Build Windows_NT x86 checked succeeded
Details
coreclr-ci (Build Windows_NT x86 debug) Build Windows_NT x86 debug succeeded
Details
coreclr-ci (Checkout (Unix)) Checkout (Unix) succeeded
Details
coreclr-ci (Checkout (Windows)) Checkout (Windows) succeeded
Details
coreclr-ci (Formatting Linux x64) Formatting Linux x64 succeeded
Details
coreclr-ci (Formatting Windows_NT x64) Formatting Windows_NT x64 succeeded
Details
coreclr-ci (Run Test Pri0 Linux arm checked) Run Test Pri0 Linux arm checked succeeded
Details
coreclr-ci (Run Test Pri0 Linux arm64 checked) Run Test Pri0 Linux arm64 checked succeeded
Details
coreclr-ci (Run Test Pri0 Linux x64 checked) Run Test Pri0 Linux x64 checked succeeded
Details
coreclr-ci (Run Test Pri0 Linux_musl x64 checked) Run Test Pri0 Linux_musl x64 checked succeeded
Details
coreclr-ci (Run Test Pri0 Linux_musl x64 release) Run Test Pri0 Linux_musl x64 release succeeded
Details
coreclr-ci (Run Test Pri0 OSX x64 checked) Run Test Pri0 OSX x64 checked succeeded
Details
coreclr-ci (Run Test Pri0 Windows_NT arm checked) Run Test Pri0 Windows_NT arm checked succeeded
Details
coreclr-ci (Run Test Pri0 Windows_NT arm64 checked) Run Test Pri0 Windows_NT arm64 checked succeeded
Details
coreclr-ci (Run Test Pri0 Windows_NT x64 checked) Run Test Pri0 Windows_NT x64 checked succeeded
Details
coreclr-ci (Run Test Pri0 Windows_NT x86 checked) Run Test Pri0 Windows_NT x86 checked succeeded
Details
coreclr-ci (Run Test Pri0 CoreFX Linux x64 checked) Run Test Pri0 CoreFX Linux x64 checked succeeded
Details
coreclr-ci (Run Test Pri0 CoreFX Windows_NT x64 checked) Run Test Pri0 CoreFX Windows_NT x64 checked succeeded
Details
coreclr-ci (Run Test Pri0 R2R Linux x64 checked) Run Test Pri0 R2R Linux x64 checked succeeded
Details
coreclr-ci (Run Test Pri0 R2R OSX x64 checked) Run Test Pri0 R2R OSX x64 checked succeeded
Details
coreclr-ci (Run Test Pri0 R2R Windows_NT x64 checked) Run Test Pri0 R2R Windows_NT x64 checked succeeded
Details
coreclr-ci (Run Test Pri0 R2R Windows_NT x86 checked) Run Test Pri0 R2R Windows_NT x86 checked succeeded
Details
coreclr-ci (Test crossgen-comparison Linux arm checked) Test crossgen-comparison Linux arm checked succeeded
Details
license/cla All CLA requirements met.
Details
@tannergooding

This comment has been minimized.

Copy link
Member

tannergooding commented Nov 4, 2019

Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corefx that referenced this pull request Nov 4, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corert that referenced this pull request Nov 4, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/mono that referenced this pull request Nov 4, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
vargaz added a commit to mono/mono that referenced this pull request Nov 4, 2019
stephentoub added a commit to dotnet/corefx that referenced this pull request Nov 4, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
@TamarChristinaArm TamarChristinaArm deleted the TamarChristinaArm:implement-arm64-reverse-element-bits-upstream branch Nov 4, 2019
jkotas added a commit to dotnet/corert that referenced this pull request Nov 5, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
@TamarChristinaArm

This comment has been minimized.

Copy link
Contributor Author

TamarChristinaArm commented Nov 7, 2019

@tannergooding Out of curiosity, how long does it usually take for me to be able to build using the updated CoreFX so I can send the Codegen changes and tests?

@tannergooding

This comment has been minimized.

Copy link
Member

tannergooding commented Nov 7, 2019

IIRC, Official builds happen on a rolling basis or twice a day (depending on the repo) and the pump flowing changes between CoreFX/CoreCLR runs once a day (at ~5am Seattle Time).

So, it normally takes about 48 hours for changes to flow all the way around if things are running smoothly. The process can be manually sped up if/when required (to within about 8 hours) but it can also take longer if other changes or failures are blocking things.

Once the repos are consolidated and the dotnet/runtime repo is open for changes (still targeting Nov 22nd: #27549) changes can be submitted all together and there will be no delay.

@TamarChristinaArm

This comment has been minimized.

Copy link
Contributor Author

TamarChristinaArm commented Nov 8, 2019

Ah, Thanks for the info!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.