Skip to content

hw: fix WMMA fp16/bf16 RTL output handling - #359

Closed
cassuto wants to merge 4 commits into
vortexgpgpu:masterfrom
cassuto:fix_wmma_bf16_fp16
Closed

hw: fix WMMA fp16/bf16 RTL output handling#359
cassuto wants to merge 4 commits into
vortexgpgpu:masterfrom
cassuto:fix_wmma_bf16_fp16

Conversation

@cassuto

@cassuto cassuto commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Issue

WMMA fp16->fp16 and bf16->bf16 did not follow fmt_d. The RTL always treated the accumulator input/output path as FP32.

Root Cause

VX_tcu_fedp_bhf was missing destination-format handling. When fmt_d selected fp16 or bf16, the RTL still produced FP32-formatted bits instead of the expected 16-bit fp16/bf16 result in the low halfword.

Proposal

Add fmt_d handling in the BHF TCU datapath so fp16 and bf16 WMMA outputs are rounded and packed in the expected destination format. Extend the SGEMM TCU regression coverage to include fp16->fp16 and bf16->bf16 cases, with ULP checks in the native 16-bit encoding space.
This fix passes the synthesis testing.

@cassuto
cassuto force-pushed the fix_wmma_bf16_fp16 branch from ff0fbc4 to ce6d361 Compare June 3, 2026 08:14
@tinebp

tinebp commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Correcting my earlier review on this PR — I had the premise backwards.

WMMA destination-format narrowing is not a supported feature of the Tensor Core Unit by
design
. The TCU accumulates and writes back FP32 (INT32 for the integer paths); fmt_d is
deliberately unused in every FEDP backend — tfr, bhf, dsp, fpnew and dpi alike — and d_val is
a 32-bit result throughout. The supported output matrix is exactly what the rtlsim test cases
cover: OTYPE=fp32 and OTYPE=int32.

That means this PR is a feature proposal, not a bug fix, and my earlier comment was wrong on
both counts: running OTYPE=bf16/OTYPE=fp16 on rtlsim is an unsupported configuration rather
than a reproduction of a defect, and the absence of narrow-output rtlsim cases in
ci/testcases/tensor.yaml is the supported matrix rather than a coverage gap. My apologies for
the noise.

What still stands from the review, as feature feedback rather than bug feedback:

  • The change targets BHF only. Adding a destination format to one backend would leave TFR —
    the default, productized, synthesizable one — and the others unable to produce it, so the
    formats a build supports would depend on VX_CFG_TCU_TYPE. Introducing a new output format is
    a design-level decision that has to cover the backends uniformly.
  • The diff no longer applies: VX_tcu_fedp_bhf.sv was substantially rewritten for 3.0/tfr
    (5-bit format ids, FP8/BF8/TF32, unified format dispatch), and the PR pipes fmt_d as 3 bits.
  • The ULP-16 comparator changes are already on master
    (tests/regression/sgemm_tcu/main.cpp:340).

If narrow output formats are something you want to pursue, it is worth raising as a design
proposal first — covering which formats, all backends, and the C-operand semantics for 16-bit
accumulate — rather than as a patch to a single backend.

@tinebp

tinebp commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Closing: as corrected above, WMMA destination-format narrowing is not a supported TCU feature by
design — the unit accumulates and writes back FP32 (INT32 for the integer paths), and fmt_d is
deliberately unused in every FEDP backend. There is no defect here to fix, so there is nothing to
merge.

Thank you for the work regardless. If narrow output formats are something you would like Vortex to
support, please open a design proposal covering the format set, all FEDP backends uniformly, and
the C-operand semantics for 16-bit accumulate — that is the right shape for this, rather than a
patch to a single backend.

@tinebp tinebp closed this Aug 30, 2026
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.

2 participants