[XPU] Update the sycl-tla (cutlass-sycl) version #331
Conversation
|
Hi @danieldk , pls help review. Thx! |
pkgs/xpu-packages/cutlass-sycl.nix
Outdated
| src = fetchFromGitHub { | ||
| owner = "intel"; | ||
| repo = "cutlass-sycl"; | ||
| tag = "v${cutlassVersion.version}"; | ||
| repo = "sycl-tla"; | ||
| # TODO Use tag for releases, rev for development commits | ||
| ${if cutlassVersion ? useTag then "tag" else "rev"} = | ||
| if cutlassVersion ? useTag | ||
| then "v${cutlassVersion.version}" | ||
| else cutlassVersion.rev; | ||
| inherit (cutlassVersion) hash; | ||
| }; |
There was a problem hiding this comment.
I'd remove useTag from cutlassVersions and then do something like:
src = fetchFromGitHub {
owner = "intel";
repo = "sycl-tla";
inherit (cutlassVersion) hash;
} // (
if cutlassVersion ? rev then { inherit (cutlassVersion) rev; }
else { tag = "v${cutlassVersion.version}"; }
);
(with nix fmt applied, my formatting is probably off here)
build2cmake/src/torch/xpu.rs
Outdated
| .iter() | ||
| .partition(|src| src.contains("old")); | ||
|
|
||
| let sources = sources_rest |
There was a problem hiding this comment.
I don't think we can/should do this blanket for all kernels. Could you explain what problem this attempts to address?
There was a problem hiding this comment.
Yes, the current implementation is a transitional approach. Our refactored FA2 depends on the new APIs introduced in version 0.7 of sycl-tla, which has not yet been released. Additionally, the APIs and codes used by the new FA2 kernel are not compatible with those in the old FA2 kernel, so we have retained the previous implementation (XXX_old) in the current XPU for the kernels building.
Alternatively, we could remove this logic along with the XXX_old files. In that case, it would require you to separately compile torch29-cxx11-xpu20252-x86_64-linux while keeping the existing torch28-cxx11-xpu20251-x86_64-linux.
May I ask which option you would prefer? We are happy to proceed according to your preference.
There was a problem hiding this comment.
Alternatively, we could remove this logic along with the
XXX_oldfiles. In that case, it would require you to separately compiletorch29-cxx11-xpu20252-x86_64-linuxwhile keeping the existingtorch28-cxx11-xpu20251-x86_64-linux.May I ask which option you would prefer? We are happy to proceed according to your preference.
Thanks for the explanation! If you do not expect to update the Torch 2.8 version, the best thing to do is to set:
[torch]
minver = "2.9"
This would only build the torch29-cxx11-xpu20252-x86_64-linux variant. The kernels-community CI will currently not remove older versions, so only torch29-cxx11-xpu20252-x86_64-linux would be updated and torch28-cxx11-xpu20251-x86_64-linux will be left as-is.
Does that sound like a reasonable solution?
There was a problem hiding this comment.
There was a problem hiding this comment.
[torch] minver = "2.9"
Sounds great! I will use this method to reorganize this PR and FA2 PR.
YangKai0616
force-pushed
the
main
branch
2 times, most recently
from
e9c8cef to
a4a830a
Compare
|
Hey @danieldk . I've resubmitted the revisions. Could you please review them again? Thank you very much! |
Update the sycl-tla (cutlass-sycl) version to support the latest FA2 kernel. Original PR: #331 --------- Co-authored-by: YangKai0616 <[email protected]>
|
Merged in #337. |
2 participants
Update the sycl-tla (cutlass-sycl) version to support the latest FA2 kernel.
Related PR huggingface/kernels-community#92