Skip to content

[Bug] Matmul Acc (L0C) TileType is never stamped CompactMode::normal, so a runtime-narrowed M stores at the wrong fractal stride #2470

Description

@zhangqi-chen

Background

In pypto-lib, a pl.matmul / pl.matmul_acc chain whose left operand carries a
runtime valid_shape row count
on a2a3 has a precision problem: the rows
that ARE valid come back wrong -- 30720 of 32768 valid elements, with only the
first 16 columns of each block correct and the zero tail correct.

Reproduce with:

python repro_matmul_acc_validshape.py -p a2a3 -d <device>

The repro case is not committed. Save the file below at the pypto-lib repo
root as repro_matmul_acc_validshape.py; it needs only pypto and pypto-lib's
golden/ harness (hw-native-sys/pypto-lib at 2cd5f828).

Reproduction case — repro_matmul_acc_validshape.py (save at the pypto-lib repo root)
"""Minimal repro: a runtime-narrowed matmul accumulator corrupts its VALID rows.

One INT8 matmul per core, K consumed by pl.matmul + pl.matmul_acc inside a
pl.pipeline.  The left operand is loaded with `valid_shape=[rows, K_TILE]` where
`rows` is a RUNTIME scalar read from a tensor.

`mad` is issued with M = the left tile's VALID rows, so the hardware lays the
L0C result out with an N-fractal stride of ceil(M/16)*16 = 16.  `TSTORE` reads
it back with `srcStride = TileData::Rows` -- the compile-time PHYSICAL 64 --
because pypto never stamps `CompactMode::normal` on the Acc tile.  Writer 16 vs
reader 64 is a factor-4 skew: store N-fractal j picks up matmul N-fractal 4j, so
only j=0 (the first 16 columns) survives.

    MM_FIX=0  python matmul-acc-runtime-validshape.py -p a2a3 -d 0   -> FAIL
    MM_FIX=1  python matmul-acc-runtime-validshape.py -p a2a3 -d 0   -> FAIL

MM_FIX=1 adds `pl.set_validshape(acc, rows, N_TILE)` before the store and fails
byte-identically: it moves the valid mask, not the physical `Rows` that TSTORE
keys off.

Rewriting `CompactMode::Null` to `CompactMode::Normal` on the Acc tiles in the
generated `<work_dir>/kernels/aic/mm_acc.cpp` and replaying with
`--runtime-dir <work_dir>` turns the FAIL into PASS.
"""
import os

import pypto.language as pl

BLOCKS = 8
M_TILE = 64          # nominal rows per block
VALID = 16           # runtime-valid rows (passed in through `rows`)
K = 2048
N_TILE = 256
K_TILE = 512
FIX = int(os.environ.get("MM_FIX", "0"))


@pl.jit
def mm_acc_validshape(
    x: pl.Tensor[[BLOCKS * M_TILE, K], pl.INT8],
    w: pl.Tensor[[BLOCKS * N_TILE, K], pl.INT8],
    rows: pl.Tensor[[BLOCKS, 1], pl.INT32],
    y: pl.Out[pl.Tensor[[BLOCKS * M_TILE, N_TILE], pl.INT32]],
):
    for b in pl.spmd(BLOCKS, name_hint="mm_acc", allow_early_resolve=True):
        m0 = b * M_TILE
        n0 = b * N_TILE
        v = pl.min(M_TILE, pl.read(rows, [b, 0]))
        xk0 = pl.slice(x, [M_TILE, K_TILE], [m0, 0], valid_shape=[v, K_TILE])
        wk0 = pl.slice(w, [N_TILE, K_TILE], [n0, 0])
        acc = pl.matmul(xk0, wk0, b_trans=True, out_dtype=pl.INT32)
        for k0 in pl.pipeline(K_TILE, K, K_TILE, stage=2):
            xk = pl.slice(x, [M_TILE, K_TILE], [m0, k0], valid_shape=[v, K_TILE])
            wk = pl.slice(w, [N_TILE, K_TILE], [n0, k0])
            acc = pl.matmul_acc(acc, xk, wk, b_trans=True)
        if FIX == 1:
            y[m0 : m0 + M_TILE, :] = pl.set_validshape(acc, v, N_TILE)
        else:
            y[m0 : m0 + M_TILE, :] = acc
    return y


def build_tensor_specs():
    import torch
    from golden import TensorSpec

    g = torch.Generator().manual_seed(7)
    xi = torch.randint(-127, 128, (BLOCKS * M_TILE, K), generator=g, dtype=torch.int32).to(torch.int8)
    wi = torch.randint(-127, 128, (BLOCKS * N_TILE, K), generator=g, dtype=torch.int32).to(torch.int8)
    rows = torch.full((BLOCKS, 1), VALID, dtype=torch.int32)
    return [
        TensorSpec("x", [BLOCKS * M_TILE, K], torch.int8, init_value=lambda: xi),
        TensorSpec("w", [BLOCKS * N_TILE, K], torch.int8, init_value=lambda: wi),
        TensorSpec("rows", [BLOCKS, 1], torch.int32, init_value=lambda: rows),
        TensorSpec("y", [BLOCKS * M_TILE, N_TILE], torch.int32, is_output=True),
    ]


def golden(tensors):
    import torch

    xf = tensors["x"].to(torch.float64)
    wf = tensors["w"].to(torch.float64)
    out = torch.zeros(BLOCKS * M_TILE, N_TILE, dtype=torch.float64)
    for b in range(BLOCKS):
        m0, n0 = b * M_TILE, b * N_TILE
        xb = xf[m0 : m0 + M_TILE].clone()
        xb[VALID:] = 0.0                      # valid_shape zero-pads the tail
        out[m0 : m0 + M_TILE] = xb @ wf[n0 : n0 + N_TILE].T
    tensors["y"][:] = out.to(torch.int32)


if __name__ == "__main__":
    import argparse
    from golden import run_jit

    ap = argparse.ArgumentParser()
    ap.add_argument("-p", "--platform", default="a2a3")
    ap.add_argument("-d", "--device", type=int, default=0)
    ap.add_argument("--compile-only", action="store_true", default=False)
    ap.add_argument("--runtime-dir", default=None)
    a = ap.parse_args()
    print(f"[REPRO] M_TILE={M_TILE} runtime valid rows={VALID} MM_FIX={FIX}")
    r = run_jit(fn=mm_acc_validshape, specs=build_tensor_specs(), golden_fn=golden,
                compile_only=a.compile_only, runtime_dir=a.runtime_dir,
                runtime_cfg=dict(platform=a.platform, device_id=a.device),
                rtol=0, atol=0)
    if not r.passed:
        print(r.error or "")
        raise SystemExit(1)

Reproduction environment:

Component Version
pypto-lib 2cd5f828 (branch: main)
pypto 3617ebbd (branch: detached)
simpler 3165cc89 (branch: detached)
ptoas 0.57
pto-isa 83d01313
CANN 9.0.0

All four components are on their pinned versions (runtime gitlink,
runtime/pto_isa.pin, toolchain/versions.env) -- no mismatch.

Also reproduces byte-identically on current pypto main 42376d0d
(with its pinned simpler 1f27a157 and pto-isa f51c92f6), so this is not
already fixed upstream.

Diagnosis: pypto -- the Acc (L0C) TileType deduced by
src/ir/op/tile_ops/matmul.cpp is built with valid_shape rows smaller than its
physical rows but is never stamped CompactMode::normal, so the PTO-ISA store
reads L0C at a different fractal stride than mad wrote it at. A secondary gap
in pto-isa is noted at the end.

Description

mad and TSTORE disagree about the L0C N-fractal stride whenever a matmul's
left operand has a runtime valid row count below its physical row count.

Writer. TMATMUL_IMPL takes M from the valid rows of the L0A tile and
passes it straight to mad, which lays the result out in L0C with an N-fractal
stride of ceil(M/16)*16. Nothing on this path consults TileRes::Rows
(pto-isa/include/pto/npu/a2a3/TMatmul.hpp):

uint16_t m = aMatrix.GetValidRow();          // 16
uint16_t k = aMatrix.GetValidCol();
uint16_t n = bMatrix.GetValidCol();
TMatmul<...>(cMatrix.data(), aMatrix.data(), bMatrix.data(), m, k, n, kDirectionAlign);
// -> mad(c, a, b, m, k, n, ...)

Reader. TStoreAccNz2nd uses the tile's compile-time physical Rows,
switching to the valid-row stride only for a compact tile
(pto-isa/include/pto/common/arch/memory/tstore_common.hpp):

uint16_t srcStride  = TileData::Rows;                  // 64
uint16_t srcNdStride = TileData::Rows * c0 * gShape4;
if constexpr (TileData::Compact == CompactMode::Normal) {
    srcStride = (validRow + FRACTAL_NZ_ROW - 1) / FRACTAL_NZ_ROW * FRACTAL_NZ_ROW;   // 16
    srcNdStride = srcStride * gShape4 * c0;
}

Why the flag is never set. DeduceTileMatMulType and
DeduceTileMatMulAccType (src/ir/op/tile_ops/matmul.cpp) build the Acc
TileView with physical_shape = {lhs physical M, rhs physical N} and
valid_shape = {lhs valid M, rhs valid N}, then leave tile_view.compact at its
CompactMode::null default:

TileView tile_view;
tile_view_semantics::SetTileLayout(
    tile_view, tile_view_semantics::GetImplicitTileLayout(geometry.physical_shape, MemorySpace::Acc));
tile_view.valid_shape = geometry.valid_shape;
return std::make_shared<TileType>(std::move(geometry.physical_shape), geometry.accumulator_dtype,
                                  std::nullopt, tile_view, MemorySpace::Acc);

The only place in the compiler that sets CompactMode::normal is the L0A/L0B
tile.extract deducer (src/ir/op/tile_ops/transform.cpp, added for #2232). The
same reasoning was never extended to L0C -- and the contrast is visible on two
adjacent lines of the generated kernel for this repro, where the L0A operand of a
matmul is compact but its own accumulator is not:

// pto: %acc__tile_l0_a
Tile<TileType::Left, int8_t, 64, 128, ..., CompactMode::Normal> v60 = ...(v39, v9);
// pto: %acc__tile_l0_c_first
Tile<TileType::Acc, int32_t, 64, 256, ..., CompactMode::Null>   v70 = ...(v39, v11);
//                                         ^^^^^^^^^^^^^^^^^^^  v39 = min(runtime_rows, 64) = 16

Consequence. Writer stride 16 vs reader stride 64 is a factor-4 skew, so the
store's N-fractal j picks up the matmul's N-fractal 4j. j = 0 is correct and
everything above it is wrong or never written -- exactly the observed
2048-correct / 30720-wrong split.

There is no compile diagnostic and no runtime error. valid_shape with a
compile-time constant is unaffected, because the physical shape narrows with it
and both sides agree. pl.set_validshape on the result before the store does not
help either (MM_FIX=1 in the repro): it moves the valid mask, not the physical
Rows that TSTORE keys off.

Steps to Reproduce

  1. Save the repro above at the pypto-lib repo root as repro_matmul_acc_validshape.py.
  2. Run MM_FIX=0 python repro_matmul_acc_validshape.py -p a2a3 -d <device>.
  3. Observe the golden comparison FAIL.

To confirm the diagnosis without changing any DSL:

  1. Compile once (--compile-only) and note the work dir printed under build_output/.
  2. In <work_dir>/kernels/aic/mm_acc.cpp, rewrite CompactMode::Null to
    CompactMode::Normal on the 12 Tile<TileType::Acc, int32_t, 64, 256, ...>
    declarations. (Patch kernels/aic/, not ptoas/ -- the latter is the ptoas
    input and is not what gets compiled.)
  3. Replay with --runtime-dir <work_dir>; the run now PASSes.

Expected Behavior

The valid rows of the product are correct, and the narrowing does real work: a
16-row TLOAD, mad with M=16, and a 16-row TSTORE. The whole point of a
runtime valid_shape on the left operand is to skip the redundant rows.

Actual Behavior

The sizes are all correct -- the generated kernel really does load 16 rows, issue
mad with M=16, and store 16 rows -- but the stored values are scrambled:

[RUN]   'y' FAIL  shape=(512, 256) dtype=torch.int32
Output(s) does not match golden: ['y']
  'y' FAIL  shape=(512, 256) dtype=torch.int32
    Mismatched elements: 30720/131072  rtol=0 atol=0
    first 20 mismatches:
    [16] actual=265517, expected=-111003
    [17] actual=174441, expected=-177350
    [18] actual=-54238, expected=264675
    ...

Element 16 is row 0, column 16 -- i.e. the first element of N-fractal 1. N-fractal
0 (columns 0-15) is correct for all 16 valid rows; that is the 2048 elements that
match. The zero-padded tail (rows 16-63) is also correct.

With MM_FIX=1 (a pl.set_validshape(acc, rows, N_TILE) before the store) the
failure is byte-identical.

Suggested Fix

Mirror the tile.extract treatment in the matmul Acc deducers: when the deduced
Acc valid_shape rows are not provably equal to the physical rows, stamp
tile_view.compact = CompactMode::normal. Verified on device via the generated-code
patch described in "Steps to Reproduce" (steps 4-6).

One caveat, which is why this is reported rather than sent as a PR. The store
side of PTO-ISA already honours the flag on every Acc->GM variant
(TStoreAccNz2nd, TStoreAccNz2nz, TStoreAccNz2NC1HWC0), but the Acc->L1
path does not: TExtractAccToMat
(pto-isa/include/pto/npu/a2a3/TExtract.hpp) reads

constexpr uint16_t srcStride = SrcTileData::Rows;

with no CompactMode branch at all. That path is already wrong today for a
runtime-narrowed M, and stamping the flag in pypto will not fix it -- it needs a
matching pto-isa change, and the same sweep should cover a5. Whoever picks this up
should decide whether the two land together.

Git Commit ID

3617ebbd6daf5b94d9d2035c880d6251974deb79 (also reproduces on main at 42376d0)

NPU Kind

a2a3 -- npu-smi reports Ascend910.

Host Platform

Linux (aarch64)

Additional Context

Impact on pypto-lib: this blocks a MoE routed-expert tiling change
(RECV_TILE 16 -> 64) that would let an expert's weights stream once instead of
ceil(rows/16) times. The only correct workaround is to drop valid_shape from
the matmul's left operand and always compute the full nominal tile -- which
forfeits exactly the work-skipping the narrowing was for.

Related, but distinct:

Activity

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions