Skip to content

arithmetic: enable TRACK_INSTRUCTIONS on the legacy op libs #351

Description

@LeoBuron

Summary

ODT_TRACK_INSTRUCTIONS (the CMake cache var gating per-element instruction
counters, TRACK_INSTRUCTIONS compile define) shipped for the new
PointwiseFused library (#328 groundwork) — lerpFloat32TensorsInplace,
addcmulFloat32TensorsInplace, addcdivDenomFloat32TensorsInplace each
increment a dedicated counter once per element, exposed via
getLerpInstructionCounter/getAddcmulInstructionCounter/
getAddcdivDenomInstructionCounter (src/arithmetic/PointwiseFused.c,
src/arithmetic/include/PointwiseFused.h).

The same TRACK_INSTRUCTIONS define already exists on two legacy
src/arithmetic/ libraries — Square.c and Matmul.c — but neither is safe
to enable. This issue tracks fixing both and generalizing the counter
convention PointwiseFused established (per-element increments, a getter per
op, no shared global namespace collision) so the mechanism can be turned on
repo-wide.

Verified blockers

  1. Square.c:4 name mismatch — fails compilation under the define.

    #ifdef TRACK_INSTRUCTIONS
    #define SQUARE_FUNC_INT squareInt32WithInstructionCounter
    

    No function named squareInt32WithInstructionCounter is defined anywhere
    in the file. The actual counting function is squareIntWithInstructionCounter
    (Square.c:21). squareInt32(a) (Square.c:26-28) calls
    SQUARE_FUNC_INT(a), which expands to the non-existent
    squareInt32WithInstructionCounter — this fails to link (undefined
    reference) the moment TRACK_INSTRUCTIONS is defined for this TU.

  2. Missing getSquareInstructionCounter declaration in Square.h.
    Square.c:43 defines getSquareInstructionCounter(), but
    src/arithmetic/include/Square.h only declares squareInt32 and
    squareFloat32 — no external caller (including a test) can read the
    counter without redeclaring it locally.

  3. No reset helper — counters are cumulative process-globals.
    squareInstructionCounter (Square.c:15) and matmulInstructionCounter
    (Matmul.c:24) both persist across the whole test binary's lifetime with
    no reset entry point. PointwiseFused's tests work around this by
    asserting the delta across a call (see
    test/unit/arithmetic/UnitTestPointwiseFused.c::testLerpCounterDeltaIsElementCount
    and siblings) rather than an absolute value — the same delta-based idiom
    would be needed for Square/Matmul, or a reset function should be added.

  4. Matmul per-call vs. PointwiseFused/scalar per-element semantics —
    inconsistent counting granularity, plus a SYM double-increment bug.

    Square's counter increments once per scalar call
    (squareIntWithInstructionCounter/squareFloatWithInstructionCounter,
    Square.c:21,34) — consistent with per-element counting when the scalar
    op is invoked in a loop. Matmul's counter increments once per whole
    matmul call
    (matmulIntTensorsWithInstructionCounter,
    matmulFloatTensorsWithInstructionCounter, Matmul.c:98-102,187-191),
    not per output element or per multiply-add — a fundamentally different
    granularity that would need to be reconciled before the two libraries'
    counters mean the same thing.

    Worse, the SYM_INT32 path double-increments: matmulSymIntTensorsWithInstructionCounter
    (Matmul.c:222-232) calls matmulInt32Tensors(...) (Matmul.c:213),
    which — under TRACK_INSTRUCTIONS — dispatches through
    MATMUL_FUNC_INT to matmulIntTensorsWithInstructionCounter
    (Matmul.c:104-106), incrementing matmulInstructionCounter once
    (Matmul.c:101); matmulSymIntTensorsWithInstructionCounter then
    increments the same global a second time itself (Matmul.c:231). A
    single SYM matmul call currently counts as 2 instructions instead of 1.

Template to follow

PointwiseFused (#328) is the reference implementation for the pattern this
issue should generalize:

  • the define/ifdef swap lives in the .c file only, keyed off a single
    TRACK_INSTRUCTIONS macro compiled in via a per-library
    target_compile_definitions(... PUBLIC $<$<BOOL:${ODT_TRACK_INSTRUCTIONS}>:TRACK_INSTRUCTIONS>)
    (src/arithmetic/CMakeLists.txt:238-239), driven by the ODT_TRACK_INSTRUCTIONS
    CMake cache var (ON in the unit_test_debug/unit_test_asan presets);
  • one counter per operation (not one shared counter across a whole library);
  • a getter per counter, matching the existing getMatmulInstructionCounter/
    getSquareInstructionCounter naming convention;
  • tests assert the delta across a call, since the counters are cumulative
    process-globals with no reset.

Scope

Not scoped here: whether Matmul's counter granularity should become
per-element (matching Square/PointwiseFused) or stay per-call with a
documented rationale — that's a design decision for whoever picks this up,
informed by what the eventual consumer (a resource/instruction estimator)
actually needs to measure.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions