Skip to content

ci(bump-hrx): hold hrx-system when our ggml-hrx uses LoomC symbols the new revision dropped - #365

Merged
bong-water-water-bong merged 1 commit into
mainfrom
ci/bump-hrx-loomc-guard
Oct 9, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
ci/bump-hrx-loomc-guard

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Why

main lost its HRX build twice this week. #353/#357 and then #361 moved third_party/hrx-system to 4ba76c18eafe, whose loom commit 32d0c76a ("Make AMDGPU runtime globals compiler-owned") deleted loomc_amdgpu_runtime_global_flags_t, loomc_amdgpu_emit_options_t and LOOMC_STRUCTURE_TYPE_AMDGPU_EMIT_OPTIONS. Our ggml/src/ggml-hrx/loom-jit.cpp uses them, so -DONEBIT_HRX=ON fails with 10 errors. #359 and #363 pinned hrx-system back to 98d05d94 by hand; KNOWN-ISSUES.md records it. The build check here does not build HRX, so nothing stopped the merges, and the scheduled run will propose the same pair again every day.

What

A new step between the hrx-system merge and the PR: collect every loomc_* / LOOMC_* identifier used under ggml/src/ggml-hrx at the new llama.cpp revision and check each is still declared somewhere under loom/binding/c/include at the new hrx-system revision. Both trees are already present as blob-less clones, so this costs a few blob fetches.

The PR branch name now carries the hrx-system revision actually pinned, so a held bump and a later real bump get different branches.

Header-level, so it cannot catch a signature change on a symbol that still exists; it catches the removal class that bit us. The fork port of loom-jit.cpp to the compiler-owned globals is in progress separately; once it lands this guard simply stops firing.

Checked

python3 -c 'import yaml; yaml.safe_load(open(".github/workflows/bump-hrx.yml"))' parses. Workflow-only change, no code or pin moves.

🤖 Generated with Claude Code

…e new revision dropped

AMD removes public LoomC symbols without notice: loom 32d0c76a (in hrx-system
4ba76c18) dropped loomc_amdgpu_runtime_global_flags_t and
loomc_amdgpu_emit_options_t, which our ggml/src/ggml-hrx/loom-jit.cpp uses.
CI here builds without HRX, so #353, #357 and #361 merged a submodule pair that
does not compile with ONEBIT_HRX=ON, and #359 and #363 pinned hrx-system back by
hand (KNOWN-ISSUES.md).

The bump now checks, at the header level, that every loomc_* / LOOMC_*
identifier used under ggml/src/ggml-hrx at the new llama.cpp revision is still
declared under loom/binding/c/include at the new hrx-system revision. If one is
missing, the PR moves llama.cpp only, keeps hrx-system where it is, and says so
in its body and in a workflow warning. When neither submodule moves, no PR is
opened (#361 moved one gitlink only).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

353 - Partially compliant

Compliant requirements:

  • Both submodules moved to the AMD-pinned pair
  • Conflicts in ggml/src/ggml-hrx/ handled with resolution toward our line
  • No changes to .github/workflows files
  • Modify/delete conflict in vector_decode_pair.loom flagged as a judgement call

Non-compliant requirements:

  • The workflow's conflict loop does not handle modify/delete branches properly

Requires further human verification:

  • The scheduled bump workflow's conflict handling needs manual intervention due to modify/delete conflicts

357 - Partially compliant

Compliant requirements:

  • Both submodules moved to the AMD-pinned pair
  • Our commits rebased onto AMD's pin
  • The PR introduces a guard to prevent CI failures from missing LoomC symbols

Non-compliant requirements:

  • The workflow's conflict loop does not handle modify/delete branches properly

Requires further human verification:

  • The scheduled bump workflow's conflict handling needs manual intervention due to modify/delete conflicts

361 - Partially compliant

Compliant requirements:

  • Both submodules moved to the AMD-pinned pair
  • Our commits rebased onto AMD's pin
  • The PR introduces a guard to prevent CI failures from missing LoomC symbols

Non-compliant requirements:

  • The workflow's conflict loop does not handle modify/delete branches properly

Requires further human verification:

  • The scheduled bump workflow's conflict handling needs manual intervention due to modify/delete conflicts
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Potential Performance Bottleneck

The new compatibility check step in the workflow fetches and processes all files under ggml/src/ggml-hrx and loom/binding/c/include to identify missing LoomC symbols. This could introduce a performance bottleneck, especially if the number of files or their content is large, as it involves multiple git show operations and grep searches.

  used=$(cd patched && git ls-tree -r --name-only "$REBASED" -- ggml/src/ggml-hrx \
    | grep -E '\.(c|cc|cpp|h|hpp)$' \
    | while read -r f; do git show "$REBASED:$f"; done \
    | grep -oE '\b(loomc_[A-Za-z0-9_]+|LOOMC_[A-Z0-9_]+)\b' | sort -u)
  headers=$(cd hrxsystem && git ls-tree -r --name-only "$HRX_REBASED" -- loom/binding/c/include \
    | while read -r f; do git show "$HRX_REBASED:$f"; done)
  missing=""
  for sym in $used; do
    printf '%s\n' "$headers" | grep -qF -- "$sym" || missing="$missing $sym"
  done
  if [ -n "$missing" ]; then
    echo "::warning::hrx-system ${HRX_REBASED:0:12} no longer declares:${missing}. Keeping third_party/hrx-system at ${OURS_HRX:0:12} until ggml-hrx is ported (KNOWN-ISSUES.md, engine#359/#363)."
    hrx_pin="$OURS_HRX"
    held="hrx-system HELD at \`${OURS_HRX:0:12}\`: our ggml-hrx still uses LoomC symbols that \`${HRX_REBASED:0:12}\` dropped (${missing## }). Port ggml/src/ggml-hrx first, then move it."
  else
    echo "every LoomC symbol our ggml-hrx uses ($(printf '%s\n' $used | wc -l)) is declared at hrx-system ${HRX_REBASED:0:12}"
  fi
fi
{
  echo "hrx_pin=$hrx_pin"
  echo "held=$held"
} >> "$GITHUB_OUTPUT"
Possible Incorrect Symbol Detection

The script uses grep -oE '\b(loomc_[A-Za-z0-9_]+|LOOMC_[A-Z0-9_]+)\b' to extract LoomC symbols from source files. This regex might not be robust enough to correctly identify all symbol usages, especially in complex macro expansions or inline assembly, potentially leading to false negatives or positives in symbol detection.

used=$(cd patched && git ls-tree -r --name-only "$REBASED" -- ggml/src/ggml-hrx \
  | grep -E '\.(c|cc|cpp|h|hpp)$' \
  | while read -r f; do git show "$REBASED:$f"; done \
  | grep -oE '\b(loomc_[A-Za-z0-9_]+|LOOMC_[A-Z0-9_]+)\b' | sort -u)

@bong-water-water-bong
bong-water-water-bong merged commit b0e1cb1 into main Oct 9, 2026
7 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the ci/bump-hrx-loomc-guard branch October 9, 2026 05:37
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