Skip to content

Conversation

@Acly
Copy link
Collaborator

@Acly Acly commented Nov 25, 2025

  • Removed ggml_vk_op_supports_incontiguous, by now its sole purpose was aborting for unsupported tensor strides
  • Added contiguous and type constraints to ggml_backend_vk_device_supports_op for ops that were missing them

This should play nice with backend scheduler and test-backend-ops.

@Acly Acly requested a review from 0cc4m as a code owner November 25, 2025 09:24
@github-actions github-actions bot added Vulkan Issues specific to the Vulkan backend ggml changes relating to the ggml tensor library for machine learning labels Nov 25, 2025
@Acly Acly changed the title vulkan : remove op_supports_incontiguous and add missing constraints … vulkan : move contiguous checks to device_supports_op Nov 25, 2025
Copy link
Collaborator

@jeffbolznv jeffbolznv left a comment

Choose a reason for hiding this comment

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

I spot checked several ops but didn't look at all of the shaders to verify whether they require contiguous.

&& ggml_is_contiguous(op->src[1]) && op->src[1]->type == GGML_TYPE_I32;
case GGML_OP_IM2COL:
return ggml_is_contiguous(op->src[0]) && ggml_is_contiguous(op->src[1])
&& (op->src[0]->type == GGML_TYPE_F32 || op->src[0]->type == GGML_TYPE_F16)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think src0 type doesn't matter for im2col(3d), it's only used to provide the filter shape.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, I removed the constraints on src0

@Acly
Copy link
Collaborator Author

Acly commented Nov 26, 2025

I spot checked several ops but didn't look at all of the shaders to verify whether they require contiguous.

I did look at all the shaders, and checked that test-backend-ops doesn't skip anything it didn't skip before. Though there's still a slight risk I accidentaly overconstrained something here.

@0cc4m 0cc4m merged commit b78db3b into ggml-org:master Nov 27, 2025
73 of 74 checks passed
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 Vulkan Issues specific to the Vulkan backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants