Skip to content

feat(tools): hrx2kineto — HRX dispatch shapes and a TraceLens extension - #323

Merged
bong-water-water-bong merged 6 commits into
mainfrom
1bit/hyperloom-hrx2kineto
Oct 6, 2026
Merged

bong-water-water-bong merged 6 commits into
mainfrom
1bit/hyperloom-hrx2kineto

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

What

tools/hyperloom/hrx2kineto.py — converts HRX dispatch profiles (iree-profile dispatch --format=jsonl) into a Kineto trace, so Hyperloom/TraceLens and TraceDiff can analyse HRX runs.

  • Base mode: one cpu_op + cuda_runtime + kernel event per dispatch.
  • --shapes/--commands: joins each dispatch to the ggml dispatch match that emitted it (fed by the llama.cpp fork's GGML_HRX_DISPATCH_SHAPE_LOG) and emits one hrx::<ggml op> cpu_op per match execution carrying input dims/type/strides, the fused op chain, FLOPs and bytes, nested under hrx::step / hrx::layer so TraceDiff aligns runs layer by layer.
  • The same file doubles as a TraceLens --extension_file (GEMM/GroupedGEMM/SDPA/NORM/RoPE/reduce/SSM/elementwise perf models), with --write-arch (gfx1151 roofline JSON), --summary (per-op roofline table with concurrent-dispatch overlap) and --tracediff-top.
  • tools/hyperloom/hrx-tracelens.sh — one-command driver: profile prompt or decode → convert → TraceLens report, optional TraceDiff, with a thermal watchdog, a box lock and a memory pre-check.

Why only one commit

This was recovered from an unreported branch of 18 commits. 17 of them are already in main — merged by PR #306, whose squash 91dc6f9 has exactly the branch's merge-base as its parent. Verified:

$ git diff --stat 91dc6f9 1bit/hrx2kineto-shapes
 tools/hyperloom/README.md        |  40 ++
 tools/hyperloom/hrx-tracelens.sh |  91 ++++
 tools/hyperloom/hrx2kineto.py    | 865 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 996 insertions(+)

So opening a PR from that branch would have re-introduced ~18,864 deletions. This PR is just the one genuinely unmerged commit, cherry-picked onto current main.

Dependency

The --shapes/--commands path needs the llama.cpp fork branch 1bit/hrx-dispatch-shape-log (GGML_HRX_DISPATCH_SHAPE_LOG), which is not in the engine pin — so CI can build and lint this, but cannot exercise --shapes. It needs a Strix Halo run with that build to verify. The positional (base) path is byte-compatible with the previous behaviour.

Risks flagged in review (offline tooling, not runtime — no GPU/NPU lifetime impact)

  • Unclosed file handles: hrx2kineto.py:328, :575 and :843 use json.dump(..., open(dst, "w")) with no with, relying on CPython refcount flushing at exit.
  • Unguarded dereference: model_match indexes primary["out"] at :232, :248, :262, :274-277 with no null check — a shape-log node without out aborts the conversion (no try/except anywhere).
  • Silent mis-attribution: map_command_buffers (:369) assumes program uids increase with execution order and each command buffer is used once. A reused buffer or non-monotonic uid attaches the wrong program → wrong op identity/shapes/FLOPs. Wrong answer, not an error.
  • Execution→layer alignment: convert_shapes (:428-446) matches by graph_uid only; unmatched executions are counted (missed) and silently fall back to build-time names, so layer numbers can be wrong.
  • --summary overlap (:623-628) only looks back 16 events, so %ovl understates wide fan-outs (e.g. concurrent Q/K/V).
  • hrx-tracelens.sh:51-54: if no k10temp hwmon, $K is empty and the arithmetic error inside while : silently disables the 93 °C guard; pkill -f "$BIN/llama-" uses a build path as an unescaped regex against the whole process table.
  • hrx-tracelens.sh:45-48 hard-codes personal defaults ($HOME/wt/hrx2kineto-src/build/bin, $HOME/wt/hrx-disasm-test2/...) — worth parameterising before this is used by anyone else.

tools/copyright.py --check passes.

Not to be merged: the release is on hold until the owner calls it.

@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit fb0abc2)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

306 - Partially compliant

Compliant requirements:

  • Removed Vulkan and ROCm/ROCmFPX backends
  • Removed ZINC submodule, build, device flag, registry column, packaging
  • Removed CUDA backend
  • Removed ComfyUI.cpp submodule
  • Removed NPU xclbin route, kept full ELFs only
  • Moved Site, blog, kernel build to separate repositories
  • Kept HRX (llama.cpp fork + hrx-system), NPU (xdna-driver, full-ELF route), Laya, MoE NVMe streaming, registry + census, 1bit OS, Hyperloom, KernelForge, tokenizers
  • Kept DwarfStar, HIP only for gfx1151, in build/ds4/hip
  • Kept ONNX support with Linux builds of ryzenai-server from source, Windows uses Lemonade's installed version
  • Kept Windows build with CPU llama-server plus --device onnx
  • Verified default build passes ctest 17/17
  • Verified strixhalo weekly release configuration passes ctest 19/19
  • Verified vulkan/rocm/zinc refusals and comfy removal behave as intended
  • Verified NPU GGUF route answers on full ELFs
  • Verified Laya end to end passes
  • Verified DwarfStar builds on HIP
  • Verified Max Tctl 71 °C

Non-compliant requirements:

  • Not yet verified: Windows cross-build, and --device onnx against a real Lemonade install on Windows

Requires further human verification:

  • Windows cross-build verification
  • --device onnx against a real Lemonade install on Windows
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Potential Performance Bottleneck

The overlap_ns function in hrx2kineto.py has a nested loop that can become a performance bottleneck for large dispatch event sets. The inner loop iterates over up to 16 previous events for each event, which can result in O(n^2) complexity in worst-case scenarios. This could significantly slow down trace processing for large profiles.

def overlap_ns(keep):
    """Per dispatch event: ns of its run that other dispatches also ran (HRX issues independent dispatches
    without a barrier, so concurrent kernels share the GPU and each one's duration includes the other)."""
    evs = sorted((r for _, ex in keep for g in ex["groups"].values() for r in g), key=lambda r: int(r["start_tick"]))
    spans = []
    for r in evs:
        start = int(r["start_tick"]) * TICK_NS
        spans.append((start, start + float(r.get("duration_ns") or int(r["duration_ticks"]) * TICK_NS), r))
    out = {}
    for i, (s0, e0, r) in enumerate(spans):
        cuts = []
        for j in range(max(0, i - 16), len(spans)):
            s1, e1, _ = spans[j]
            if j == i:
                continue
            if s1 >= e0:
                break
            lo, hi = max(s0, s1), min(e0, e1)
            if hi > lo:
                cuts.append((lo, hi))
        covered, end = 0.0, s0
        for lo, hi in sorted(cuts):
            lo = max(lo, end)
            if hi > lo:
                covered += hi - lo
                end = hi
        out[id(r)] = covered
    return out
Possible Memory Leak in TraceLens Extension

The TraceLens extension classes in hrx2kineto.py (e.g., _HRXOp, HRXGemm, etc.) do not explicitly manage memory or handle cleanup. While Python's garbage collector will eventually clean up these objects, in long-running or high-frequency profiling scenarios, this could lead to memory accumulation or unexpected behavior if these objects hold references to large data structures.

class _HRXOp:
    """Duck-typed TraceLens perf model: reads the "hrx" arg the converter wrote."""
    category = "other"
    bwd_category = None

    def __init__(self, event, arch=None, python_path=None, **kwargs):
        self.event = event
        self.arch = arch
        self.hrx = (event.get("args") or {}).get("hrx") or {}
        self.param_details = {k: tuple(v) if isinstance(v, list) else v
                              for k, v in (self.hrx.get("params") or {}).items()}

    def flops(self):
        return float(self.hrx.get("flops") or 0.0)

    def bytes(self, *args, **kwargs):
        b = self.hrx.get("bytes")
        return None if b is None else float(b)

    def get_compute_precision(self):
        return self.hrx.get("precision")

    def get_maf_type(self):
        return self.hrx.get("maf")

    def flops_bwd(self):
        raise NotImplementedError

    def bytes_bwd(self, *args, **kwargs):
        raise NotImplementedError


class HRXGemm(_HRXOp):
    category = "GEMM"


class HRXGroupedGemm(_HRXOp):
    category = "GroupedGEMM_fwd"


class HRXAttention(_HRXOp):
    category = "SDPA_fwd"


class HRXNorm(_HRXOp):
    category = "NORM_fwd"
    sheet_category = "Normalization"


class HRXRoPE(_HRXOp):
    category = "RoPE_fwd"


class HRXReduce(_HRXOp):
    category = "reduce"
    sheet_category = "Reduce"


class HRXSSM(_HRXOp):
    category = "SSM_fwd"


class HRXElementwise(_HRXOp):
    category = "elementwise"
    sheet_category = "BinaryElementwise"


class HRXCopy(_HRXOp):
    category = "elementwise"
    sheet_category = "UnaryElementwise"


_CLASS_OF = {"gemm": HRXGemm, "grouped_gemm": HRXGroupedGemm, "attention": HRXAttention, "norm": HRXNorm,
             "rope": HRXRoPE, "reduce": HRXReduce, "ssm": HRXSSM, "copy": HRXCopy}
perf_model_extension = {}
for _op in GGML_OPS + PRIMARY_ORDER:
    _cat = CATEGORY_OF.get(_op, "copy" if _op in ("GET_ROWS", "SET_ROWS", "CPY", "CONT", "DUP", "CONCAT")
                           else "elementwise")
    perf_model_extension["hrx::" + _op.lower()] = _CLASS_OF.get(_cat, HRXElementwise)
op_category_extension = {"hrx::step": "other", "hrx::layer": "other"}
Potential Race Condition in Thermal Watchdog

The thermal watchdog in hrx-tracelens.sh uses a background process to monitor temperature and kill the process if it exceeds 93°C. However, there's a potential race condition where the kill command might not be properly synchronized with the main process, potentially leading to inconsistent behavior or premature termination of the profiling run.

( while :; do
    t=$(( $(cat $K/temp1_input) / 1000 ))
    [ $t -ge 93 ] && { echo "$(date +%T) THERMAL ABORT $t C" >> "$O/run.log"; pkill -f "$BIN/llama-"; }
    sleep 1
  done ) & TG=$!
trap 'kill $TG 2>/dev/null' EXIT

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 0d61005

…ns.sh

hrx2kineto.py (HRX dispatch profile -> Kineto trace) keeps its old
positional behaviour byte for byte and adds --shapes/--commands: with the
llama.cpp fork's GGML_HRX_DISPATCH_SHAPE_LOG (fork branch
1bit/hrx-dispatch-shape-log, not in the pin yet) every dispatch is joined to
the ggml dispatch match that emitted it and emitted as an hrx::<ggml op>
cpu_op with Input Dims/types/strides, the fused op chain, FLOPs and bytes
(quantized weights at their real bits per weight, MUL_MAT_ID reading only
the routed experts), nested under hrx::step / hrx::layer so TraceDiff
aligns runs with and without graph splits. The same file is a TraceLens
--extension_file (GEMM / GroupedGEMM / SDPA / NORM / RoPE / reduce / SSM /
elementwise perf models), --write-arch writes a gfx1151 arch JSON for the
roofline columns, --summary prints a per-op roofline table with the
concurrent-dispatch overlap, and --tracediff-top summarises TraceDiff.

hrx-tracelens.sh profiles one prompt or decode run, converts it and writes
the TraceLens report (optionally with TraceDiff against another run).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 03947eb

The profile executor looks for a Kineto trace it never got: it probes
<workspace>/torch_trace, <workspace>/capture_traces and
<workspace>.parent/capture_traces for *.trace.json.gz. This entrypoint
ignored PROFILE and the profiler dir Hyperloom hands it (see
bypass_scriptable.py: PROFILE=1 plus VLLM/SGLANG_TORCH_PROFILER_DIR), so all
three candidates were empty, the profile phase failed three times and the
whole 2026-09-30 pass ran in degraded mode with no roofline to direct the
search -- which is how it reported 0.00% gain against a blind search space.

With PROFILE=1, run the existing, already-proven HRX trace production
(hrx-tracelens.sh -> hrx2kineto.py -> trace.json) and write it where the
executor globs: <trace dir>/rank_0.trace.json.gz.

The box lock is taken earlier in this script, so BOX_LOCK is cleared for the
helper -- a second flock on the same file from a child would deadlock.

Failure here is loud but non-fatal: the benchmark number still counts.
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 4d56c4a

The profile leg traced HRX_BIN, defaulting to a separate shape-log build, while
the benchmark measures $REPO/build/bin. That would aim the roofline -- and so
the whole optimisation -- at a binary nobody measured. Hyperloom's own
profile_custom.yaml warns about exactly this: a trace of a different run than
the headline number is worthless.

Now prefer this checkout's build, and fall back to a shape-log build only when
this one lacks GGML_HRX_DISPATCH_SHAPE_LOG, printing a warning that says the
trace then describes a DIFFERENT build. `strings` on libggml-hrx.so.0 is the
test, since the instrumentation is a runtime shape log rather than a CLI flag.

Re-tested with stubs: fallback path warns and still produces a valid gzip;
a build carrying the shape log takes the direct path with no warning; PROFILE
unset remains a no-op so the baseline leg is untouched.
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit fb0abc2

…produces a trace

The profile leg failed end to end for two reasons that only running it exposes:

- iree-profile was hard-coded to a path that does not exist, and the two tools on
  the box are a month older than the v2 profile the current HRX writes, so they
  abort with "unsupported IREE HAL profile file version". Derive the tool from
  HRX_BIN's own deps build instead (IREE_PROFILE, then PATH).
- hrx2kineto's op-level match needs the command buffer's dispatch order to equal
  the logged program's build order; the runtime may reorder it (multisets match,
  orders do not), so no trace is written and Hyperloom's profile phase degrades.
  Fall back to a kernel-level trace, which TraceLens accepts, so the phase yields
  real data instead of failing. README records all three limitations and the
  order-independent matcher that would remove the fallback.
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Preparing review...

bong-water-water-bong and others added 2 commits October 5, 2026 22:24
…ofile leg fell back

`strings $B/libggml-hrx.so.0 | grep -q GGML_HRX_DISPATCH_SHAPE_LOG` under `set -o pipefail`
returns non-zero: `grep -q` exits on the first match, `strings` is killed by SIGPIPE, and the
pipeline status is the failure. On a build that carries the shape log the check therefore ALWAYS
took the fallback branch and traced $HRX_SHAPELOG_BIN (a build that does not exist here), so the
Hyperloom profile leg produced no trace and the pass degraded -- exactly as observed on 2026-10-05.
`grep -c` reads to EOF, so nothing is killed. Verified with the check under pipefail.
@bong-water-water-bong
bong-water-water-bong marked this pull request as ready for review October 6, 2026 04:02
@bong-water-water-bong
bong-water-water-bong merged commit da39701 into main Oct 6, 2026
6 of 8 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/hyperloom-hrx2kineto branch October 6, 2026 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant