Skip to content

cublas team verified workaround for large GEMM algo68 bug - #3100

Merged
divyegala merged 3 commits into
NVIDIA:release/26.08from
divyegala:cublas-workaround
Jul 30, 2026
Merged

cublas team verified workaround for large GEMM algo68 bug#3100
divyegala merged 3 commits into
NVIDIA:release/26.08from
divyegala:cublas-workaround

Conversation

@divyegala

Copy link
Copy Markdown
Contributor

The fix in #3098 was incomplete, as according to the cublas team there is no guarantee that algo13 will be returned along with algo68 (the failing version for A matrix with > 2^31 elements). Also, the cublas team will patch this bug in version 13.6.1 so we will automatically receive the fix by guarding only against 13.6.0.

@divyegala divyegala self-assigned this Jul 30, 2026
@divyegala
divyegala requested a review from a team as a code owner July 30, 2026 17:26
@divyegala divyegala added bug Something isn't working non-breaking Non-breaking change labels Jul 30, 2026
@divyegala
divyegala changed the base branch from main to release/26.08 July 30, 2026 17:30
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: eebbc3a5-13c2-4b7f-ab68-8260f569c359

📥 Commits

Reviewing files that changed from the base of the PR and between 112b567 and 4bb33b5.

📒 Files selected for processing (1)
  • cpp/include/raft/linalg/detail/cublaslt_wrappers.hpp

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Refined the cuBLASLt 13.6 FP32 workaround and heuristic selection to apply the leading-dimension adjustment only for 16-byte-aligned cases.
    • Simplified cuBLASLt heuristic querying to reliably choose a single preferred result and avoid the known-bad algorithm when the workaround is active.
  • Tests
    • Updated cuBLASLt 13.6 workaround tests to cover version boundary behavior and validate the returned heuristic arguments, including correct leading-dimension handling.

Walkthrough

The cuBLASLt 13.6 workaround now uses version and leading-dimension checks, conditionally adjusts heuristic lda, manages preference handles with RAII, validates the selected algorithm, and updates GEMM tests.

Changes

cuBLASLt workaround

Layer / File(s) Summary
Conditional heuristic argument adjustment
cpp/include/raft/linalg/detail/cublaslt_wrappers.hpp, cpp/tests/linalg/gemm_basic.cpp
The workaround conditionally adjusts heuristic lda, removes unused includes, uses an RAII preference wrapper, queries one heuristic result, rejects algorithm 68, and tests heuristic argument mapping.
Version-based workaround predicate
cpp/include/raft/linalg/detail/cublaslt_wrappers.hpp, cpp/tests/linalg/gemm_basic.cpp
The predicate uses only the matmul key and cuBLASLt version, with updated version boundary expectations.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • NVIDIA/raft#3098: Modifies the same cuBLASLt 13.6 algorithm 68 workaround and related GEMM tests.

Suggested reviewers: cjnolet, vyasr

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: tightening the cuBLAS large GEMM algo68 workaround.
Description check ✅ Passed The description matches the PR goal of guarding the workaround to cuBLAS 13.6.0 after the earlier fix proved incomplete.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/include/raft/linalg/detail/cublaslt_wrappers.hpp`:
- Around line 229-254: Make the cublasLt matmul preference handle in the
heuristic setup exception-safe: replace the raw preference lifecycle around
query_heuristic with an existing RAII wrapper or add a scoped cleanup guard that
destroys it on every path, including when cublasLtMatmulAlgoGetHeuristic throws.
Preserve the existing heuristic queries and ensure destruction occurs exactly
once without relying on the trailing manual destroy call.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e057ad64-8188-4d8f-b841-0068397d0a00

📥 Commits

Reviewing files that changed from the base of the PR and between edc3997 and 5557f52.

📒 Files selected for processing (2)
  • cpp/include/raft/linalg/detail/cublaslt_wrappers.hpp
  • cpp/tests/linalg/gemm_basic.cpp

Comment thread cpp/include/raft/linalg/detail/cublaslt_wrappers.hpp Outdated
@divyegala
divyegala merged commit 97d22c8 into NVIDIA:release/26.08 Jul 30, 2026
80 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants