Skip to content

Mfma matrix core timing #1700

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

Merged
merged 4 commits into from
Nov 21, 2024
Merged

Conversation

mjkpolo
Copy link
Contributor

@mjkpolo mjkpolo commented Oct 22, 2024

A lookup table is used with values taken from the mi200 and mi300 ISA
specifying how long a SIMD's matrix core unit will be consumed by MFMAs.

This time can be adjusted for all MFMAs using the --mfma-scale parameter.

@mattsinc mattsinc requested review from mattsinc and abmerop October 22, 2024 20:02
@mattsinc mattsinc added the gpu-compute gem5's GPU Compute Code label Oct 22, 2024
@mattsinc
Copy link
Contributor

Needs to be merged in after #1693

Copy link
Contributor

@mattsinc mattsinc left a comment

Choose a reason for hiding this comment

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

Since Vishnu already integrated Nagendra's patches in #1693, please remove those from this patch before we review.

@mjkpolo mjkpolo force-pushed the mfma_matrix_core_timing branch from 859071a to a8f1053 Compare October 22, 2024 20:27
Copy link
Contributor

@mattsinc mattsinc left a comment

Choose a reason for hiding this comment

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

Few small things, but overall nice job!

Copy link
Member

@abmerop abmerop left a comment

Choose a reason for hiding this comment

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

This is mostly good, thanks! I only have one additional comment on top of Matt's comments.

@erin-le
Copy link
Contributor

erin-le commented Nov 14, 2024

@mjkpolo Hello, thank you for your contribution! Could you give an update on the status of this PR? Also, if you are able to make the changes requested within the next day or so/by the end of this week, we may be able to get this PR into the staging branch.

@mattsinc
Copy link
Contributor

@mjkpolo Hello, thank you for your contribution! Could you give an update on the status of this PR? Also, if you are able to make the changes requested within the next day or so/by the end of this week, we may be able to get this PR into the staging branch.

@mjkpolo is working towards an ISCA submission, so my guess is he won't be able to address these comments in the timeline you provided.

@abmerop
Copy link
Member

abmerop commented Nov 15, 2024

Should I just add a commit with the changes? It seems like we just want comments and an assert?

@abmerop abmerop force-pushed the mfma_matrix_core_timing branch from a8f1053 to 439b3f3 Compare November 15, 2024 18:12
mjkpolo and others added 2 commits November 15, 2024 11:05
A lookup table is used with values taken from the mi200 and mi300 ISA
specifying how long a SIMD's matrix core unit will be consumed by MFMAs.

This time can be adjusted for all MFMAs using the --mfma-scale parameter.
@abmerop abmerop force-pushed the mfma_matrix_core_timing branch from 439b3f3 to 95c3a3c Compare November 15, 2024 19:07
@mattsinc
Copy link
Contributor

Should I just add a commit with the changes? It seems like we just want comments and an assert?

Thanks @abmerop ! I also fixed the print info.

@mattsinc
Copy link
Contributor

Should I just add a commit with the changes? It seems like we just want comments and an assert?

Thanks @abmerop ! I also fixed the print info.

@abmerop since I pushed the print fix it won't let me approve ... can you?

@abmerop abmerop merged commit 5583723 into gem5:develop Nov 21, 2024
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gpu-compute gem5's GPU Compute Code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants