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

[QST] a redundant code #1211

Closed
zhoutianzi666 opened this issue Nov 23, 2023 · 6 comments
Closed

[QST] a redundant code #1211

zhoutianzi666 opened this issue Nov 23, 2023 · 6 comments

Comments

@zhoutianzi666
Copy link

What is your question?

if (!kSplitKSerial || gemm_k_iterations > 0) {

why add if (!kSplitKSerial || gemm_k_iterations > 0) {, I think if (gemm_k_iterations > 0) { is enough.

@zhoutianzi666 zhoutianzi666 changed the title a reduant code a redundant code Nov 23, 2023
@JieRen98
Copy link

I guess keeping it is better, which will make things clear. It hints that, for the SplitK Serial case, we do not need to do mma, just do epilogue reduction only.

@zhoutianzi666
Copy link
Author

I guess keeping it is better, which will make things clear. It hints that, for the SplitK Serial case, we do not need to do mma, just do epilogue reduction only.
ok,it means in SplitK Serial case when some thread blocks having gemm_k_iterations == 0, we do not do mma。
but there is any this case?

@JieRen98
Copy link

In addition, !kSplitKSerial will enable compile time optimization

@mnicely mnicely changed the title a redundant code [QST] a redundant code Nov 27, 2023
@hwu36
Copy link
Collaborator

hwu36 commented Nov 28, 2023

kernel/gemm.h is the old implementation. Now we use kernel/gemm_universal.h. It is called universal because it supports, batch gemm, gemm array, parallel splitk, serail splitk, standard gemm in one implementation. it is also used by cublas.

in the old days, these go through different kernel level code though they are very similar to each other.

as to this line, i think we should not check kSplitKSerial

Copy link

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@mnicely
Copy link
Collaborator

mnicely commented Jan 2, 2024

Closing due to inactivity. Feel free to reopen if needed.

@mnicely mnicely closed this as completed Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants