Skip to content

T4a replaces the ROCm Dp4a hardware dot with the scalar expansion the checker exists to prevent #2939

Description

@localai-org-maint-bot

Row: GFX1100-TG200

51f5222dc in #2790 (T4a) rewrites Dp4a in
src/vt/rocm/rocm_grouped_gemm.hip from the hardware intrinsic to the scalar
expansion:

// before
  char4_native va = *reinterpret_cast<const char4_native*>(&a);
  char4_native vb = *reinterpret_cast<const char4_native*>(&b);
  return __ockl_sdot4(va, vb, acc, false);

// after
  const int8_t* a8 = reinterpret_cast<const int8_t*>(&a);
  const int8_t* b8 = reinterpret_cast<const int8_t*>(&b);
  return acc + a8[0] * b8[0] + a8[1] * b8[1] + a8[2] * b8[2] + a8[3] * b8[3];

and rewrites the comment above it to "The HW dot instruction
(v_dot4_i32_i8 / __ockl_sdot4) is a perf lever, not a correctness
requirement."

scripts/check-rocm-dp4a-intrinsic.py exists to stop precisely this, and says
so in its own docstring:

The scalar expansion -- four int8 multiplies plus four adds -- is
bit-identical but ~1.4x slower on the KQuantGemmK prefill path. A CPU-only
ctest gate stays green with either form, because the ROCm kernel is not
compiled on the CPU tier. This checker reads the source and fails when the
intrinsic is absent, so the performance lever cannot regress silently.

Dp4a is the integer core of every K-quant GEMM on this backend. The pull
request body does not mention the change, offers no measurement for it, and
leaves tests/scripts/test_check_rocm_dp4a_intrinsic.py red in preflight -- its
test_live_scalar_mutation_fails case fails with "mutation did not apply",
because the text it mutates is no longer there.

Both sides agree the two forms are bit-identical, so restoring the intrinsic
cannot change a single output. It restores the speed and the gate.

One inconsistency worth settling separately: rocm_quant_dot.hip asserts
"gfx1100 has no signed byte dot" while rocm_grouped_gemm.hip and this checker
both state __ockl_sdot4 emits v_dot4_i32_i8 on gfx1100. Those cannot both be
right. That does not block this repair, which only puts back what main has.

Found while running preflight on the merged external-contributor stack. Repaired
in the same flow on the landed merge rather than on #2790's branch, which sits
under nineteen later branches that edit the same files.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions