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

Scale P and Q with L2 cache size for SVE #4397

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Mousius
Copy link
Contributor

@Mousius Mousius commented Dec 27, 2023

The defaults in param.h now reflect an L2 size of 128KB, and that is scaled based on the actual size.

The defaults in param.h now reflect an L2 size of 128KB, and that is
scaled based on the actual size.
@Mousius
Copy link
Contributor Author

Mousius commented Dec 27, 2023

@martin-frbg , this is closer to what I was thinking previously, what do you think? I can see others have done similar in setparam-ref.c but I'm assuming that doesn't work when building for a specific core outside of DYNAMIC_ARCH?

@martin-frbg
Copy link
Collaborator

yes, for a specific cpu TARGET build I think the factor would have to be applied in common_param.h but I have limited brain capacity for that right now

@Mousius
Copy link
Contributor Author

Mousius commented Dec 29, 2023

Thanks @martin-frbg, I'll look into it 😸 !

@@ -3517,13 +3517,13 @@ Until then, just keep it different than DGEMM_DEFAULT_UNROLL_N to keep copy rout
#define ZGEMM_DEFAULT_UNROLL_N 4
#define ZGEMM_DEFAULT_UNROLL_MN 16

#define SGEMM_DEFAULT_P 128
#define DGEMM_DEFAULT_P 160
#define SGEMM_DEFAULT_P 30

Choose a reason for hiding this comment

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

How were the default P and Q chosen for 128KB cache?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#4381 demonstrated values that worked well for a 1MB L2 cache, so I divided that by 8.

If you have a more scientific approach, I'd be happy to hear it 😸

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