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

Fix gemm splik #991

Closed
wants to merge 1 commit into from
Closed

Conversation

PeixuanZuo
Copy link

No description provided.

@PeixuanZuo
Copy link
Author

@illsilin Could you please review this pr? Thanks.

@zjing14 zjing14 requested review from zjing14 and illsilin October 16, 2023 15:40
@zjing14
Copy link
Contributor

zjing14 commented Oct 16, 2023

@PeixuanZuo Could you create the branch in the CK repo instead of forked repo? Otherwise, the PR won't run on our CI.

@illsilin
Copy link
Collaborator

Why do you think that it's a bug?

@PeixuanZuo
Copy link
Author

Why do you think that it's a bug?

If we want to use both fp8 and fp16 data types, we need to make sure they are both enabled. Please correct me if I've misunderstood the logic.

@illsilin
Copy link
Collaborator

Well, almost 99% of the kernels are only dealing with one datatype. We have only added a handful of kernels that will use different datatypes as inputs.
The purpose of the DTYPES filters is primarily to reduce the build time for the entire CK library, especially when building for multiple targets. So we don't necessarily want to rip out every mention of a data type in case we are not planning to use it, we just don't want to build any instances, test, or examples that are using that data type EXCLUSIVELY.
However, if an instance is using more than one data type, and one of those types is selected for the build, we have decided that we will build all the instances that use that data type, even if the other type was not selected. Hence the "or" condition in the code.

@PeixuanZuo
Copy link
Author

Thanks for your details explanation. This fix is to solve a build error. I will open a new issue for the problem I encountered. Thank you.

@PeixuanZuo PeixuanZuo closed this Oct 17, 2023
@PeixuanZuo
Copy link
Author

I found an existing issue(#928), which is the issue I encountered.

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