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

Change Vector<T> to use more compact code using S.R.CS.Unsafe #26637

Merged
merged 2 commits into from Sep 11, 2019

Conversation

@jkotas
Copy link
Member

commented Sep 10, 2019

S.R.CS.Unsafe is used in number of places in Vector already. This change modifies more places to use it to make the code more compact.

Updated Vector.tt template to match while I was on it

Change Vector<T> to use more compact code using S.R.CS.Unsafe
S.R.CS.Unsafe is used in number of places in Vector<T> already. This change modifies more places to use it to make the code more compact.

Updated Vector.tt template to match while I was on it

@jkotas jkotas force-pushed the jkotas:vector branch from 737d534 to 08a1ab9 Sep 10, 2019

@tannergooding

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

Thanks for doing this @jkotas. @GrabYourPitchforks and I had previously discussed something like this and hadn't had the time to do it yet.

@GrabYourPitchforks

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

I had some minor suggestions that could reduce the codegen size even further, but they're all just nits. LGTM even if they're not addressed.

It'd be nice if in the future we could address @tannergooding's other feedback and make these methods recursive with the newly-written code as the software fallback. But perhaps that's best left to a separate PR.

@tannergooding

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

Are there any perf metrics for ARM32 (which is, I think, the only currently supported platform that doesn't accelerate these)?

Also, do you think it is worthwhile to (eventually, in a future PR) update methods like https://github.com/dotnet/coreclr/pull/26637/files#diff-89579d6d4dd26983d2be720ac13be3ebR491 to get rid of the separate paths for Vector.IsHardwareAccelerated vs the software fallback? That is, most of the logic could be implemented using the Vector.IsHardwareAccelerated path (e.g. a simple for loop over the individual T and with a couple small helper methods to abstract numeric operations) and then we could just get rid of the Vector.tt file entirely.

@jkotas jkotas force-pushed the jkotas:vector branch from 561a9d9 to 2831e38 Sep 11, 2019

@jkotas

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2019

do you think it is worthwhile to (eventually, in a future PR) update methods like ... to get rid of the separate paths

Yes, I think it would be interesting to look into. As you have pointed out, we would want to make sure that it does not regress performance on arm too much. The changes that I have made in this PR are on the safe side. I have avoided touching anything that would be likely to regress without hardware acceleration.

@jkotas jkotas merged commit 76c0217 into dotnet:master Sep 11, 2019

54 checks passed

WIP Ready for review
Details
coreclr-ci Build #20190910.46 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 Test Pri0 CoreFX Linux x64 checked) Build Test Pri0 CoreFX Linux x64 checked succeeded
Details
coreclr-ci (Build Test Pri0 CoreFX Windows_NT x64 checked) Build Test Pri0 CoreFX Windows_NT x64 checked 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 Linux x64 checked) Build Test Pri0 Linux x64 checked succeeded
Details
coreclr-ci (Build Test Pri0 Linux_musl x64 checked) Build Test Pri0 Linux_musl x64 checked succeeded
Details
coreclr-ci (Build Test Pri0 Linux_musl x64 release) Build Test Pri0 Linux_musl x64 release succeeded
Details
coreclr-ci (Build Test Pri0 OSX x64 checked) Build Test Pri0 OSX x64 checked succeeded
Details
coreclr-ci (Build Test Pri0 R2R Linux x64 checked) Build Test Pri0 R2R Linux x64 checked succeeded
Details
coreclr-ci (Build Test Pri0 R2R OSX x64 checked) Build Test Pri0 R2R OSX x64 checked succeeded
Details
coreclr-ci (Build Test Pri0 R2R Windows_NT x64 checked) Build Test Pri0 R2R Windows_NT x64 checked succeeded
Details
coreclr-ci (Build Test Pri0 R2R Windows_NT x86 checked) Build Test Pri0 R2R Windows_NT x86 checked 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 (Formatting Linux x64) Formatting Linux x64 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 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 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 (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 (Test crossgen-comparison Linux arm checked) Test crossgen-comparison Linux arm checked succeeded
Details
license/cla All CLA requirements met.
Details

@jkotas jkotas deleted the jkotas:vector branch Sep 14, 2019

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