Skip to content

[Math] Protect against C == nullptr in SOFIE::Gemm_Call pullback#21490

Merged
guitargeek merged 1 commit intoroot-project:masterfrom
guitargeek:sgemm_derivative
Mar 9, 2026
Merged

[Math] Protect against C == nullptr in SOFIE::Gemm_Call pullback#21490
guitargeek merged 1 commit intoroot-project:masterfrom
guitargeek:sgemm_derivative

Conversation

@guitargeek
Copy link
Copy Markdown
Contributor

It's possible to call gemm without a C parameter (constant offset), so the custom derivative can't assume that C and _d_C are always set.

This avoids segfaults when C is nullptr.

It's possible to call `gemm` without a `C` parameter (constant offset),
so the custom derivative can't assume that `C` and `_d_C` are always
set.

This avoids segfaults when `C` is `nullptr`.
@guitargeek guitargeek requested a review from lmoneta March 4, 2026 15:00
@guitargeek guitargeek self-assigned this Mar 4, 2026
@guitargeek guitargeek changed the title [Math] Protect against C=nullptr in SOFIE::Gemm_Call pullback [Math] Protect against C == nullptr in SOFIE::Gemm_Call pullback Mar 4, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 4, 2026

Test Results

    22 files      22 suites   3d 7h 57m 45s ⏱️
 3 805 tests  3 803 ✅ 1 💤 1 ❌
76 590 runs  76 580 ✅ 9 💤 1 ❌

For more details on these failures, see this check.

Results for commit 00b413c.

@silverweed
Copy link
Copy Markdown
Contributor

How hot is this for loop? Does it make sense to introduce 2 branches rather than enforcing those params to be non-null (e.g. by throwing or asserting)?

@guitargeek
Copy link
Copy Markdown
Contributor Author

Thanks for the review! The loop is not very hot. The expensive part of the function is the matrix multiplication just before the loop over the offsets C, so I don't expect that optimizing this loop changes the performance characteristics of the general matrix multiplication gradient function here.

Copy link
Copy Markdown
Member

@lmoneta lmoneta left a comment

Choose a reason for hiding this comment

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

LGTM!

@guitargeek guitargeek merged commit d7e3a05 into root-project:master Mar 9, 2026
29 of 33 checks passed
@guitargeek guitargeek deleted the sgemm_derivative branch March 9, 2026 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants