Skip to content

bump-xdna: correct the XRT path in the PR's test recipe - #82

Merged
bong-water-water-bong merged 4 commits into
mainfrom
bump-xdna-path
Sep 25, 2026
Merged

bong-water-water-bong merged 4 commits into
mainfrom
bump-xdna-path

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Found while verifying #76: build-xdna.sh <prefix> installs XRT at <prefix>/root/opt/xilinx/xrt, so the recipe's -DONEBIT_XRT_ROOT=<prefix> fails to configure. The recipe now uses the install path, and sets XILINX_XRT so the lane test loads the new driver plugin.

🤖 Generated with Claude Code

bong-water-water-bong and others added 2 commits September 25, 2026 10:38
build-xdna.sh installs there; ONEBIT_XRT_ROOT=<prefix> fails to configure (found verifying #76).
XILINX_XRT points XRT at the new driver plugin for the lane test.

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

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 45c2963)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

76 - Partially compliant

Compliant requirements:

  • Move third_party/xdna-driver from efddccd30568 to upstream main 49ab57645766
  • Update XRT submodule to d8ececf95767
  • Correct the XRT path in the test recipe for Strix Halo

Non-compliant requirements:

  • Ensure CI instructions work on Strix Halo hardware (requires manual verification)

Requires further human verification:

  • CI instructions verification on Strix Halo hardware
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Incorrect XRT Path in Test Recipe

The test recipe in the PR description incorrectly references the XRT installation path. The script build-xdna.sh <prefix> installs XRT at <prefix>/root/opt/xilinx/xrt, but the previous recipe used -DONEBIT_XRT_ROOT=$HOME/.cache/xdna-pin/prefix which would not point to the correct location. This has been corrected to use the full install path, ensuring that the lane test loads the new driver plugin correctly.

\`scripts/build-xdna.sh ~/.cache/xdna-pin/prefix && cmake -B build -G Ninja -DONEBIT_NPU=ON -DONEBIT_XRT_ROOT=\$HOME/.cache/xdna-pin/prefix/root/opt/xilinx/xrt && cmake --build build --target onebit && XILINX_XRT=\$HOME/.cache/xdna-pin/prefix/root/opt/xilinx/xrt tests/npu_lane_e2e.sh build/1bit <model dir> <model dir>/npu <reference logits>\`"

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit a806941

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit b0cd956

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 45c2963

@bong-water-water-bong
bong-water-water-bong merged commit 9bd79a4 into main Sep 25, 2026
3 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the bump-xdna-path branch September 25, 2026 13:44
bong-water-water-bong added a commit that referenced this pull request Oct 5, 2026
…to our fork (Loom NaN backport) (#320)

* Bump HRX: llama.cpp 522dab4789ce (long-prompt fix), hrx-system to our fork 98d05d94a9f9

third_party/llama.cpp e57beb9721af -> 522dab4789ce (fork #89 runtime overhead,
#90: graph inputs never written back to host — every multi-ubatch prompt was
wrong since #82 — and the non-replay upload race).
third_party/hrx-system moves from ROCm/hrx-system 51b1739ae5fd to our fork
1bit-MONSTER/hrx-system 1bit/main 98d05d94a9f9 = the same AMD commit plus the
GFX11 wave64 lane-mask drain backport (#313 NaN) and two libhrx device knobs.
bump-hrx.yml now moves only the llama.cpp pin; hrx-system is rebased by hand.

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

* registry: regenerated at llama.cpp 522dab4789ce

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

---------

Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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