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

ref: Restore explicit vectorization in the Vc AoS plugin #118

Merged
merged 4 commits into from
Aug 26, 2024

Conversation

niermann999
Copy link
Contributor

@niermann999 niermann999 commented Apr 12, 2024

This refactors the Vc AoS plugin to use explicitly vectorizing Vc types again. Also adds it to the benchmarks and tests. Since not all functionality is implemented, yet, I split the test suite into three blocks, so that new plugins can be implemented incrementally with testing enabled. Finally, I harmonized the naming between this plugin and the new SoA plugin.

Also removes the warmup from the bencharmks, since google benchmark can be configured to do that for us.

Edit: I refactored the vc_aos::transform3 and matrix44 types, so that are shared between Vc AoS and SoA now

@niermann999

This comment was marked as outdated.

@niermann999 niermann999 force-pushed the ref-VcAos-plugin branch 2 times, most recently from 562414f to d4e78a6 Compare April 12, 2024 15:36
@niermann999 niermann999 marked this pull request as ready for review April 12, 2024 16:06
@niermann999
Copy link
Contributor Author

niermann999 commented Apr 12, 2024

Based on #97 , so that the storage vector type can be reused and vc_array4 replaced with something more generic. This will be helpful for generalizing the plugin to higher dimensional types later

@niermann999 niermann999 force-pushed the ref-VcAos-plugin branch 6 times, most recently from 81e07f8 to 54c567f Compare April 15, 2024 14:43
@beomki-yeo
Copy link
Contributor

It seems Vc AoS gets slower with double for some cases, is that normal?

@niermann999
Copy link
Contributor Author

niermann999 commented Apr 16, 2024

It seems Vc AoS gets slower with double for some cases, is that normal?

It is expected for vectorized code to be about half as slow in double precision as in single precision (half the number of values fits into the same number of bits in the registers). I believe that the cmath plugin is slower in double precision could be hint that it is in fact partly autovectorized. Why Vc AoS is so much slower than cmath in double precision I don't know, but this is what I have seen before as well.

@beomki-yeo
Copy link
Contributor

Yes I was asking why Vc double is slower than cmath double (not w.r.t float). Thanks for the answer

@niermann999
Copy link
Contributor Author

These benchmarks are a bit outdated though (and also done with CPU scaling, so that e.g. addition and subtraction show different results). It works much better running:

./bin/algebra_benchmark_array_vector --benchmark_min_warmup_time=1000 --benchmark_repetitions=50 --benchmark_time_unit=ms --benchmark_display_aggregates_only=true --benchmark_enable_random_interleaving=true

with CPU scaling disabled

@niermann999 niermann999 force-pushed the ref-VcAos-plugin branch 6 times, most recently from 7913fe8 to 58d17de Compare April 25, 2024 09:16
@niermann999
Copy link
Contributor Author

The CI failure does not seem to be related to this PR, see #123

@niermann999 niermann999 requested a review from krasznaa April 25, 2024 09:49
@niermann999 niermann999 force-pushed the ref-VcAos-plugin branch 4 times, most recently from 44d9055 to ee48a3b Compare July 9, 2024 15:25
@niermann999 niermann999 force-pushed the ref-VcAos-plugin branch 5 times, most recently from 02805e8 to a9e17d2 Compare July 25, 2024 08:03
Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

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

Did not have a chance for looking at the MSVC error yet. 😦 But in general this looks good to me.

storage/vc_aos/CMakeLists.txt Outdated Show resolved Hide resolved
@niermann999 niermann999 force-pushed the ref-VcAos-plugin branch 2 times, most recently from 7dea7bd to 5d749a9 Compare August 13, 2024 16:35
@niermann999 niermann999 requested a review from krasznaa August 13, 2024 16:36
Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

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

Just a couple of technical questions and comments. No fundamental issue from my side. Generally I like what you did here. 😉

benchmarks/CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

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

Other than what I found before, I'm happy with this. 👍

@niermann999 niermann999 merged commit 4bfcc95 into acts-project:main Aug 26, 2024
24 checks passed
niermann999 added a commit to niermann999/algebra-plugins that referenced this pull request Nov 11, 2024
This refactors the Vc AoS plugin to use explicitly vectorizing Vc types again. Also adds it to the benchmarks and tests. Since not all functionality is implemented, yet, I split the test suite into three blocks, so that new plugins can be implemented incrementally with testing enabled. Finally, I harmonized the naming between this plugin and the new SoA plugin.

Also removes the warmup from the bencharmks, since google benchmark can be configured to do that for us.

Edit: I refactored the vc_aos::transform3 and matrix44 types, so that are shared between Vc AoS and SoA now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants