Skip to content

Fix col_reduce_longcolumn for negative-stride views - #4529

Open
mateuuszzzzz wants to merge 2 commits into
ml-explore:mainfrom
mateuuszzzzz:fix-col-reduce-negative-stride
Open

mateuuszzzzz wants to merge 2 commits into
ml-explore:mainfrom
mateuuszzzzz:fix-col-reduce-negative-stride

Conversation

@mateuuszzzzz

@mateuuszzzzz mateuuszzzzz commented Sep 17, 2026

Copy link
Copy Markdown

Fixes: #4528

Details

The thread coordinate (lid.x) in col_reduce_longcolumn is an unsigned integer and causes implicit conversion of in_idx to an unsigned integer in the expression in_idx + lid.x. This implicit conversion wraps the input offset for negative-stride views. PR fixes this by adding an explicit type cast of lid.x to IdxT, ensuring both operands share the same signed type and avoiding the implicit conversion of in_idx.

Tests cover operations relying on reduce kernels and all dispatch paths for the col_reduce* family.
I verified that all test cases picking col_reduce_longcolumn fail on main and pass after applying the fix.

  • ☑️ I understand it is strictly prohibited to use AI to write PR description

  • AI usage disclosure: Opus 5 assisted with:

    • identifying what other functions except sum are impacted by the bug (and pointed me to col_reduce* family)
    • writing tests that cover dispatches to all col_reduce* kernels
    • checking that current fix does not introduce any regressions.

    I verified the exact reproduction conditions, implemented a fix, and ran local checks and tests before pushing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Metal col_reduce_longcolumn produces wrong results for negative-stride views

1 participant