Skip to content

SYCL: Disable reorder optimize by default and stop setting tensor extras when optimize is disabled #13254

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 2 commits into from
May 6, 2025

Conversation

qnixsynapse
Copy link
Collaborator

Workaround for #13163

@github-actions github-actions bot added ggml changes relating to the ggml tensor library for machine learning SYCL https://en.wikipedia.org/wiki/SYCL - GPU programming language labels May 2, 2025
@Alcpz
Copy link
Collaborator

Alcpz commented May 2, 2025

@qnixsynapse have you been able to replicate the issue? I've only seen it on Windows and I haven't been able to pinpoint the issue on Linux. I've seen multiple exceptions thrown when setting the tmp_buf using memcpy after the malloc_device in the reorder.

I'm asking because I am not certain if we may want to set the reorder value to disabled just for Windows builds. What do you think?

Edit: Could be a false hint though, as it's not always crashing there.

@qnixsynapse
Copy link
Collaborator Author

qnixsynapse commented May 3, 2025

@qnixsynapse have you been able to replicate the issue? I've only seen it on Windows and I haven't been able to pinpoint the issue on Linux.

I'm asking because I am not certain if we may want to set the reorder value to disabled just for Windows builds. What do you think?

Edit: Could be a false hint though, as it's not always crashing there.

You are correct that this doesn't happen on Linux which I confirmed. But I feel for now it should be disabled by default until we resolve all the issues and implement for every quantization type.

My main concern is setting tensor extras in the init_tensor function. This PR disables that when reorder optimization is disabled.

@Alcpz
Copy link
Collaborator

Alcpz commented May 6, 2025

It seems @sgeor255 found a fix in #13109 , we can merge this and once the fix is approved we can make sure it's re-enabled.

@qnixsynapse
Copy link
Collaborator Author

I see. Okay.

@qnixsynapse qnixsynapse merged commit 1e333d5 into master May 6, 2025
51 checks passed
@qnixsynapse qnixsynapse deleted the sycl/disable_extra_reorder_optimize branch May 6, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ggml changes relating to the ggml tensor library for machine learning SYCL https://en.wikipedia.org/wiki/SYCL - GPU programming language
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants