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

Align method of sycl headers/libs finding in third_parth/intel/backend/driver.py #3147

Open
ESI-SYD opened this issue Jan 13, 2025 · 3 comments · May be fixed by #3238
Open

Align method of sycl headers/libs finding in third_parth/intel/backend/driver.py #3147

ESI-SYD opened this issue Jan 13, 2025 · 3 comments · May be fixed by #3238
Assignees
Labels

Comments

@ESI-SYD
Copy link
Contributor

ESI-SYD commented Jan 13, 2025

Describe the bug

There is a chance to update method of sycl headers/libs finding. Now intel finds sycl headers by deducing from icpx_path or intel-sycl-rt, takes libs as optional. (Adding icpx_path and libs finding with PR #3135 ), That means there still have dependent on icpx.

NV download_and_copy cudart files to backend/include, while AMD just upload hiprt to backend/include, finds libs by /sbin/ldconfig -p

Environment details

Triton XPU

@ESI-SYD ESI-SYD added bug Something isn't working enhancement New feature or request dependencies: sycl runtime and removed bug Something isn't working labels Jan 13, 2025
@dchigarev
Copy link
Contributor

I might not be fully getting what we want to achieve by resolving this issue.

Looking at the driver.py:find_sycl() code after #3135 it seems that it's already generic enough to not depend on icpx (if icpx is not installed the sycl headers are deduced from either ONEAPI_PATH or intel-sycl-rt package).

My understanding judging by this comment is that resolving this issue implies placing the sycl headers in third_party/intel/backend/includes, by either placing them manually (like AMD) or by downloading them from somewhere in setup.py (like Nvidia). But what's the point of that if Triton XPU requires us to have intel-sycl-rt installed, so we always can deduce the sycl headers with this existing logic?

cc @anmyachev (I have seen you've reviewed the original PR, so I guess you may have some thoughts on that)

@ESI-SYD
Copy link
Contributor Author

ESI-SYD commented Jan 22, 2025

SYCL has several use scenarios in Triton XPU, see #3135 (comment). The second scenario is using g++ but relying on icpx.

This issue is to align with others' practice of preparing runtime headers libs in advance and eliminate the hidden dependence on icpx, just like gcc compile cuda program using cudart without the participation of nvcc. https://github.com/intel/intel-xpu-backend-for-triton/blob/main/third_party/nvidia/backend/driver.py#L15-L47

@etiotto
Copy link
Contributor

etiotto commented Jan 29, 2025

Shipping the SYCL headers as part of Triton is not ideal because they might go out of sync with the SYCL-RT version. I think Triton should continue to use the headers shipped with the SYCL-RT.

@vlad-penkin WDYT ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants