Repository navigation
Vitis Unified Backend - #1376
Conversation
…i wrapper for vitisUnified partial backend and build the skeleton code for other generation section
…ks in axi_stream - add axi_stream double to test_predict_any_numpy_dtype (review U2)
…e wrapper build - add vitisunified:ip default flow with validate_bram_weights, test and doc note (review U3)
…ame and invalid configs - cover items (d) and (e) of review U21; they flip to passing when U4, U12 and U7 land
…nd limitations - unknown boards are rejected, buffer size unit, positioning against VivadoAccelerator (review U22)
…nd support namespace in the wrapper - namespace, write_tar and the other writer options are no longer dropped (review U4)
…lement reset - add parse_vitis_unified_report reusing the Vivado csynth and cosim parsers, CI entries and a test (review U5)
…eter - VitisUnified calls super().__init__ instead of bypassing two parents (review U6)
…te time - checks in create_initial_config and a validate_config pass, asserts replaced by exceptions (review U7)
…r edits generated files - three cfg variants, vitis-comp.json points at the csim one, test added (review U9)
…lative paths - the generated project builds after a move; the test moves it and checks every path (review U11)
…gument, no hard-coded names or tool paths - one driver per AXI mode, project-named test bench, IP version from the config, XILINX_VITIS from the environment (review U12, U13, U14)
… scheduled by the flow - VitisUnified inherits the split and only overrides the HLS project path; test with a faked co-simulation (review U10)
…tcl, and write the tar once - documented no-op overrides, the tar is made at the end of write_hls (review U8)
…moved, explicit stream contract - guard message computes the allowed length, TKEEP set on every packet, sim-only depth comment (review U15 to U20)
…ions - fixtures, conversion, write, compile, Vitis HLS, bitstream; bodies unchanged
- their AXI DMA is 32 bits wide; use float, or a user platform with a 64-bit DMA - drop test_fifo_depth_passes, covered by test_fifo_depth, and comment the config tests
- the AXI DMA never completes a transfer without TKEEP, so the review's drop-TKEEP option is not possible
…e saved prediction - verified on kv260 for axi_stream, axi_master and a 2-in/2-out axi_master model
- one entry holds the last dimension of the tensor, so a sample takes N / channels entries
|
Thanks for the detailed review. All 22 findings are addressed in the 20 commits after the reviewed head Required before merge
Needed for it to be a credible default
Cleanups and longer term
Changes outside the VitisUnified backend
No behaviour change for the Vivado, Vitis, VivadoAccelerator or Coyote backends. Test results
KV260 (PYNQ 3.0.1), generated drivers, batch of 10, compared with the software prediction saved by the test:
Script: |
vloncar
left a comment
There was a problem hiding this comment.
Thanks for the detailed response, this is exactly what I was hoping to see.
I checked the update independently at the current head. I went through all the new commits against your table, ran the new test suite (same as your numbers), and reran the 32 configurations from my original review. I didn't run the hardware tests myself, but I trust your numbers. All 22 findings are addressed. The input array with the wrong dtype now predicts correctly instead of silently returning zeros, and double on axi_stream and BramFactor weights are rejected at conversion with a clear message instead of failing later in the C++ build. Every invalid configuration I tried now stops at conversion and tells the user what to do, and all 24 valid ones give results identical to the Vitis backend. The platform and part arguments together with the board-independent driver solve the board support problem properly, a board that is not in the list needs no code changes at all.
The places where you went a different way than I (in truth, Claude) suggested (like keeping TKEEP since the DMA needs it, the loop labels, the simulation-only depth) are all fine with me.
One small non-blocking follow-up: docs/backend/accelerator.rst still doesn't mention this backend, so someone reading the VivadoAccelerator page won't learn about its successor. A one-line cross-reference would do, and it can be a separate PR along with other documentation fixes we have planned.
Looks good. Approving.
|
The test failures are unrelated to this PR. Merging. Huge thanks to the team for making this. |
…kend The guard added in #1376 skipped rows of channel_info.csv with an empty instance path. The Vitis backend writes all model FIFOs with an empty instance path, so no depths were read and FifoDepthOptimizationPost failed with a KeyError (#1570). Select rows by FIFO name instead, which also skips the Vitis Unified wrapper's own streams, strip fields instead of dropping the last character of the file name, and raise a clear error when a profiled depth is missing.
* Fix FIFO depth optimization parsing of channel_info.csv for Vitis backend The guard added in #1376 skipped rows of channel_info.csv with an empty instance path. The Vitis backend writes all model FIFOs with an empty instance path, so no depths were read and FifoDepthOptimizationPost failed with a KeyError (#1570). Select rows by FIFO name instead, which also skips the Vitis Unified wrapper's own streams, strip fields instead of dropping the last character of the file name, and raise a clear error when a profiled depth is missing. * slight cleanup of Claude comments * [pre-commit.ci] auto fixes from pre-commit hooks --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Description
VitisUnified backend
Motivation
The Vitis backend stops at the HLS project. This backend takes an hls4ml model to a file set that is ready for a PYNQ board: bitstream, hardware handoff and Python driver, built with the Vitis Unified flow (
v++compile and package,v++link against a platform). It is meant as the flow for AMD SoC boards with Vitis 2023.2 or newer; models withio_parallelorap_fixedinterfaces stay with VivadoAccelerator.Features
axi_mode:axi_master(kernel accesses DDR itself, multiple inputs and outputs) oraxi_stream(AXI DMA in the platform, one input and one output).One batch per kernel start, interrupt driven; one PYNQ driver per AXI mode with register offsets from the hardware handoff.
build()with the parent keywords:synth,csim,cosim,fifo_opt,vitis_fifo_sizing,bitfile; returns the Vitis report dictionary.Boards as data:
supported_boards.json(zcu102, kv260) or your own platform viaplatform=andpart=.Configuration validated at conversion;
predict()accepts any numpy dtype.Documentation page
docs/backend/vitis_unified.rst.Limitations
io_streamonly;float/doubleinterfaces only, both the same; noBramFactorweights (rejected at conversion);axi_streamwith one input and one output; no multigraph; Python driver only;doubleonaxi_streamneeds a platform with a 64-bit DMA (the shipped ones are 32 bits).Type of change
Tests
test/pytest/test_vitis_unified.py, 41 tests in six sections ordered by flow depth:version, driver contents, own platform, name-length guardaxi_modexfloat/doublex numpy dtype, writer options forwardedreset(fake reports, no Vitis needed)RUN_SYNTHESIS=true): csim, cosim, FIFO depth optimization, both AXI modesRUN_VITIS_UNIFIED_BITSTREAM=true): kv260,axi_stream,axi_master, 2-input/2-outputaxi_masterResults: without Vitis 32 passed, 9 skipped. With Vitis 2023.2 all 41 pass.
Hardware, KV260 with PYNQ 3.0.1, generated drivers, batch of 10, compared with the software prediction saved by the test (
test/board/vitis_unified_hw_test.py):axi_streamaxi_masteraxi_masterTest reproduce
build(log_to_stdout=False)writes<step>_stdout.logand<step>_stderr.login the output directory instead of printing.Checklist
pre-commiton the files I edited or added.AI assistance disclosure
Claude Code (Claude Fable 5.1) was used in this PR; commits from March 2026 onward may contain code, tests or documentation written with it, including all fixes after the first review. Every change was reviewed, and all numbers above come from runs on my machine and my KV260.
Implementation detail
Three stages produce the files to ship:
v++ -c --mode hlsandvitis-run --packageproduce the.xolink_system.shrunsv++ -lagainst the platform, then extractssystem.bitandsystem.hwhfrom the.xclbinTemplate structure
hls4ml/templates/vitis_unified:Output structure
All paths in the generated files are relative, so the output directory can be moved or copied to another machine.
Configuration
Keyword arguments of the converter:
version(default1.0.0) setspackage.ip.versionand the driver'sbindto.Notes
vitis_workspace/<project_name>in the Vitis Unified IDE to debug the kernel; the component points at the csim config.m_axidepthin the AXI-master wrapper is for simulation only, one sample; the test bench sends one sample per kernel start.vitis_workspace/system_link/_x/link/vivado/vpl/prj.Generated warnings
From the U-Net model used in the tests, not from the backend:
Kernel synthesis prints the usual Vitis HLS warnings (unused parameter, deprecated pragma).