Skip to content

sycl: Cleanup codepaths in Get Rows in sycl backend #14215

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ShanoToni
Copy link
Contributor

This PR removes some dead codepaths and unused functions inside Get Rows implementation in the sycl backend.

@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 Jun 16, 2025
@@ -277,13 +188,8 @@ void ggml_sycl_op_get_rows(ggml_backend_sycl_context & ctx, ggml_tensor * dst) {
src1_i32, (float *)dst->data, ctx.stream());
break;
case GGML_TYPE_Q4_0:
if (ctx.opt_feature.reorder && dst->op == GGML_OP_MUL_MAT) {
Copy link
Collaborator

@Rbiessy Rbiessy Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Highlighting it for future reference, the condition dst->op == GGML_OP_MUL_MAT is always false because of this switch:

case GGML_OP_GET_ROWS:
ggml_sycl_get_rows(ctx, dst);
break;

@ericcurtin ericcurtin requested a review from Copilot June 16, 2025 22:17
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR cleans up the SYCL backend’s Get Rows implementation by removing unused reorder codepaths and simplifying the dispatch logic.

  • Eliminates the k_get_rows_reorder kernel and its host wrapper get_rows_sycl_reorder
  • Removes the conditional branch for ctx.opt_feature.reorder in the Q4_0 case
  • Leaves only the core k_get_rows and get_rows_sycl paths for all quantization types
Comments suppressed due to low confidence (1)

ggml/src/ggml-sycl/getrows.cpp:62

  • Since k_get_rows_reorder and its host wrapper have been removed, verify whether dequantize_kernel_t_reorder is still referenced elsewhere; if not, remove its typedef and related includes to eliminate unused code.
template<int qk, int qr, dequantize_kernel_t_reorder dequantize_kernel_recorder, typename dst_t>

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