ggml : require contiguous src for ROLL on CUDA and Metal (#25928)

ggml_roll only asserts nb[0] == ggml_type_size, so a permuted src is a
valid input, but the CUDA and Metal roll kernels index by ne alone and
never read the nb strides. A non-contiguous src therefore produced
silently wrong results. Neither backend declared a contiguity
requirement in supports_op, so the scheduler did not fall back to the
CPU implementation, which does handle strides correctly.

Add the requirement to both backends, matching the existing
GGML_OP_ROPE guard, and add a permuted test_roll case.
This commit is contained in:
Yash Raj Pandey 2026-08-10 08:01:44 -04:00 committed by GitHub
parent 4dee52f82d
commit f8def7fe16
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 14 additions and 5 deletions

View file

@ -5185,7 +5185,7 @@ static bool ggml_backend_cuda_device_supports_op(ggml_backend_dev_t dev, const g
return max_bias == 0.0f;
}
case GGML_OP_ROLL:
if(op->src[0]->type == GGML_TYPE_F32) {
if(op->src[0]->type == GGML_TYPE_F32 && ggml_is_contiguous(op->src[0])) {
return true;
}
return false;

View file

@ -1268,8 +1268,9 @@ bool ggml_metal_device_supports_op(ggml_metal_device_t dev, const struct ggml_te
case GGML_OP_ARGSORT:
case GGML_OP_TOP_K:
case GGML_OP_ARANGE:
case GGML_OP_ROLL:
return true;
case GGML_OP_ROLL:
return ggml_is_contiguous(op->src[0]);
case GGML_OP_FLASH_ATTN_EXT:
// for new head sizes, add checks here
if (op->src[0]->ne[0] != 32 &&

View file

@ -6712,19 +6712,26 @@ struct test_roll : public test_case {
const int shift1;
const int shift3;
const int shift4;
const bool permute;
std::string vars() override {
return VARS_TO_STR4(shift0, shift1, shift3, shift4);
return VARS_TO_STR5(shift0, shift1, shift3, shift4, permute);
}
test_roll(int shift0 = 3, int shift1 = -2, int shift3 = 1, int shift4 = -1)
: shift0(shift0), shift1(shift1), shift3(shift3), shift4(shift4) {}
test_roll(int shift0 = 3, int shift1 = -2, int shift3 = 1, int shift4 = -1, bool permute = false)
: shift0(shift0), shift1(shift1), shift3(shift3), shift4(shift4), permute(permute) {}
ggml_tensor * build_graph(ggml_context * ctx) override {
int64_t ne[4] = {10, 5, 4, 3};
ggml_tensor * a = ggml_new_tensor(ctx, GGML_TYPE_F32, 4, ne);
ggml_set_name(a, "a");
if (permute) {
// ggml_roll only requires nb[0] == type size, so a permuted src is valid
a = ggml_permute(ctx, a, 0, 2, 1, 3);
ggml_set_name(a, "a_permuted");
}
ggml_tensor * out = ggml_roll(ctx, a, shift0, shift1, shift3, shift4);
ggml_set_name(out, "out");
@ -9459,6 +9466,7 @@ static std::vector<std::unique_ptr<test_case>> make_test_cases_eval() {
test_cases.emplace_back(new test_pad_reflect_1d());
test_cases.emplace_back(new test_pad_reflect_1d(GGML_TYPE_F32, {3000, 384, 4, 1}));
test_cases.emplace_back(new test_roll());
test_cases.emplace_back(new test_roll(3, -2, 1, -1, true));
test_cases.emplace_back(new test_arange());
test_cases.emplace_back(new test_arange(GGML_TYPE_F32, 0.0f, 1048576.0f, 1.0f));
test_cases.emplace_back(new test_timestep_embedding());