Skip to content

fix!: align Gemm public signature - #878

Merged
voltjia merged 1 commit into
masterfrom
fix/align-gemm-signature
Aug 5, 2026
Merged

fix!: align Gemm public signature#878
voltjia merged 1 commit into
masterfrom
fix/align-gemm-signature

Conversation

@voltjia

@voltjia voltjia commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Align the public Gemm call order to Gemm(a, b, optional C, alpha, beta, trans_a, trans_b, Y).
  • Keep the valid three-tensor Gemm(a, b, Y) overload; omitted C always uses an effective beta of zero.
  • Rename identifiers that represent the writable result from C to Y throughout the affected adapters, tests, and host-overhead case.
  • Reject non-null C with the existing assertion convention until fix: support optional C in Gemm #870 adds the backend implementation.

Motivation

The legacy full overload used the writable result tensor as both C and Y, so its public signature did not match the open-source framework contract. This precursor changes the API now while keeping backend arithmetic unchanged; #870 will add independent and broadcast C support separately.

No linked issue. The mismatch was found during the operator API alignment audit.

Type of Change

  • feat - new feature / new operator / new platform
  • fix - bug fix
  • perf - performance improvement (no behavioral change)
  • refactor - code restructuring without behavior change
  • test - adding or fixing tests only
  • docs - documentation only
  • build / ci - build system or CI configuration
  • chore - tooling, formatting, or other non-code changes
  • Breaking change (requires a ! in the Conventional Commits prefix or a BREAKING CHANGE: footer)

Platforms Affected

  • CPU (WITH_CPU)
  • NVIDIA (WITH_NVIDIA)
  • Iluvatar (WITH_ILUVATAR)
  • MetaX (WITH_METAX)
  • Cambricon (WITH_CAMBRICON)
  • Moore (WITH_MOORE)
  • Ascend (WITH_ASCEND)
  • PyTorch C++ bindings (WITH_TORCH)
  • Build system / CMake / CI
  • Python bindings / user-facing API

Smoke Test Result

Exact-head GitHub CI for 362fce68 is running after a comment-only update. Its code-identical predecessor a3e4ea01 had:

  • build passed.
  • both Ruff and both clang-format 21 checks passed.
  • main and shadow Ascend, Cambricon, MetaX, and Moore jobs passed.
  • main and shadow Iluvatar jobs remain queued.
  • main and shadow NVIDIA jobs fail while compiling unchanged src/base/flash_attn_varlen_func.h; the first error is at line 56, followed by the same ShapeView/StridesView versus SmallVector ternary mismatch at lines 58, 66, and 68. The logs contain no Gemm error.

The NVIDIA difference from the last green master run is dependency drift: CI clones InfiniRT master without a pinned SHA. The master run cloned InfiniRT before a38977e0; #878 cloned that commit after it changed TensorView metadata accessors and exposed the existing FlashAttention mismatch. This compatibility fix is unrelated to Gemm and is intentionally not included here.

Focused validation for the same code at a3e4ea01:

python -m py_compile tests/conftest.py tests/test_gemm.py scripts/run_host_overhead_control.py
passed

python scripts/generate_wrappers.py --devices cpu --ops gemm --strict-ops
passed

git diff --check origin/master...HEAD
passed

Test Results on Supported Platforms

Platform Affected Build / Smoke Result Full Result / Notes
NVIDIA Yes Main/shadow blocked by unchanged FlashAttention/InfiniRT compile mismatch Predecessor logs contain no Gemm error
Iluvatar Yes Main/shadow queued Shared BLAS signature adapter
MetaX Yes Main/shadow passed Shared BLAS signature adapter
Cambricon Yes Main/shadow passed Direct operator signature change
Moore Yes Main/shadow passed Shared BLAS signature adapter
Ascend Yes Main/shadow passed Direct operator signature change
Full pytest output (optional)
Full pytest was not run locally. Exact-head platform results are listed above;
Iluvatar main/shadow jobs remain queued.

Benchmark / Performance Impact

N/A. Backend arithmetic is unchanged. Calls with omitted C force the effective beta to zero and do not read Y.

Notes for Reviewers

  • This PR changes the original virtual operator() signatures directly; it does not introduce a virtual Compute layer.
  • Non-null C is not implemented here. Debug builds assert in the constructor and call path; fix: support optional C in Gemm #870 adds the implementation.
  • Documentation and runnable examples are unchanged because they already use the still-valid three-tensor overload.
  • No new test case, Add implementation, broadcast metadata, or backend accumulation path is included.
  • The existing Gemm matrix only receives the canonical argument order, C=None behavior, output naming, matching smoke-selector keys, and removal of redundant beta cases.
  • fix: support optional C in Gemm #870 is stacked on this PR and owns the implementation and focused test coverage.

BREAKING CHANGE: Gemm now accepts optional C before its attributes and writes Y; non-null C is not implemented until #870.

@voltjia voltjia mentioned this pull request Aug 4, 2026
20 tasks
@voltjia
voltjia force-pushed the fix/align-gemm-signature branch 5 times, most recently from a3e4ea0 to 362fce6 Compare August 5, 2026 03:15
@voltjia voltjia mentioned this pull request Aug 5, 2026
19 tasks
@voltjia
voltjia force-pushed the fix/align-gemm-signature branch from 362fce6 to cec9c4a Compare August 5, 2026 05:53
BREAKING CHANGE: Gemm now accepts optional C before attributes and writes Y; non-null C is not implemented yet.
@voltjia
voltjia force-pushed the fix/align-gemm-signature branch from cec9c4a to 65923dc Compare August 5, 2026 07:12
@voltjia
voltjia marked this pull request as ready for review August 5, 2026 07:17
@voltjia
voltjia requested a review from a team August 5, 2026 07:17
@voltjia
voltjia merged commit 1b9c376 into master Aug 5, 2026
18 of 20 checks passed
@voltjia
voltjia deleted the fix/align-gemm-signature branch August 5, 2026 07:28
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.

1 participant