Vitis Unified Backend

[Tanawin1701d]

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 with io_parallel or ap_fixed interfaces stay with VivadoAccelerator.

Features

• axi_mode: axi_master (kernel accesses DDR itself, multiple inputs and outputs) or axi_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 via platform= and part=.

• Configuration validated at conversion; predict() accepts any numpy dtype.

• Documentation page docs/backend/vitis_unified.rst.

Limitations

io_stream only; float/double interfaces only, both the same; no BramFactor weights (rejected at conversion); axi_stream with one input and one output; no multigraph; Python driver only; double on axi_stream needs a platform with a 64-bit DMA (the shipped ones are 32 bits).

Type of change

• New feature (non-breaking change which adds functionality)

Tests

test/pytest/test_vitis_unified.py, 41 tests in six sections ordered by flow depth:

1. conversion: invalid configurations and BRAM weights rejected before anything is written
2. write: config files, custom project name, version, driver contents, own platform, name-length guard
3. predict: parity with the Vitis backend for axi_mode x float/double x numpy dtype, writer options forwarded
4. build: report dictionary and reset (fake reports, no Vitis needed)
5. Vitis HLS (RUN_SYNTHESIS=true): csim, cosim, FIFO depth optimization, both AXI modes
6. bitstream (RUN_VITIS_UNIFIED_BITSTREAM=true): kv260, axi_stream, axi_master, 2-input/2-output axi_master

Results: 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):

Bitstream	AXI mode	Ports	max abs diff hw vs sw	Second run identical
simple U-Net	axi_stream	1 in, 1 out	0.0	yes
simple U-Net	axi_master	1 in, 1 out	0.0	yes
2-in / 2-out CNN	axi_master	2 in, 2 out	0.0	yes

Test reproduce

cd test/pytest
pytest test_vitis_unified.py                                                   # no Vitis needed
RUN_SYNTHESIS=true pytest test_vitis_unified.py                                # + csim, cosim, FIFO (v++, vitis-run on PATH)
RUN_SYNTHESIS=true RUN_VITIS_UNIFIED_BITSTREAM=true pytest test_vitis_unified.py   # + kv260 bitstreams (vivado, xclbinutil, XILINX_VITIS)

build(log_to_stdout=False) writes <step>_stdout.log and <step>_stderr.log in the output directory instead of printing.

Checklist

• I have read the guidelines for contributing.
• I have commented my code, particularly in hard-to-understand areas.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings. (see the section below)
• I have installed and run pre-commit on the files I edited or added.
• I have added tests that prove my fix is effective or that my feature works.

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:

• file generation: HLS sources, AXI wrapper, test bench, bridge, Vitis config files, driver
• kernel synthesis: v++ -c --mode hls and vitis-run --package produce the .xo
• link: link_system.sh runs v++ -l against the platform, then extracts system.bit and system.hwh from the .xclbin

Template structure

hls4ml/templates/vitis_unified:

├── ap_types/                       ap_axi_sdata.h for simulation without Vitis
├── build_lib.sh                    builds the shared library for predict()
├── drivers/
│   ├── axi_master_driver.py.hls4ml
│   └── axi_stream_driver.py.hls4ml
├── hls_kernel_config.cfg           Vitis HLS config, one copy per step is written
├── myproject_axi_master.cpp / .h   AXI-master wrapper
├── myproject_axi_stream.cpp / .h   AXI-stream wrapper
├── myproject_bridge.cpp            bridge for predict()
├── myproject_test.cpp              test bench for csim and cosim
├── nnet_utils/nnet_helpers_axi.h   stream helpers for simulation
├── vitis_workspace/
│   ├── kernel_project/vitis-comp.json
│   └── system_link/link_system.cfg, link_system.sh
├── kv260/tcl_scripts/              platform generation
└── zcu102/tcl_scripts/

Output structure

<output_dir>/
├── firmware/                          HLS sources: model, AXI wrapper, weights
├── tb_data/                           testbench input and reference output
├── <project_name>_test.cpp            C testbench of the AXI wrapper
├── <project_name>_bridge.cpp          bridge used by hls_model.predict()
├── build_lib.sh
├── hls4ml_config.yml
├── fifo_depths.json                   with FIFO depth optimization only
├── <step>_stdout.log, <step>_stderr.log   with log_to_stdout=False only
├── vitis_workspace/
│   ├── <project_name>/
│   │   ├── vitis-comp.json            Vitis Unified component, open this folder in the IDE
│   │   ├── hls_kernel_config_csim.cfg
│   │   ├── hls_kernel_config_cosim.cfg
│   │   ├── hls_kernel_config_cosim_fifo_sizing.cfg
│   │   └── vitis_unified_project/     hls/, logs/, reports/, <project_name>_axi_*.xo
│   ├── system_link/
│   │   ├── link_system.cfg, link_system.sh
│   │   ├── <project_name>.xclbin      link output (bitfile=True)
│   │   └── _x/                        Vivado project of the system link
│   └── <board>/tcl_scripts/           platform generation, output/<board>_*.xsa
├── export/
│   ├── system.bit, system.hwh         bitfile=True
│   └── axi_master_driver.py or axi_stream_driver.py
└── final_reports/                     timing, utilization, power, link summary, hls_compile.rpt

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:

board='zcu102',              # zcu102 or kv260 from supported_boards.json; any name together with platform and part
part=None,                   # taken from the board entry if not given
platform=None,               # your own .xpfm or .xsa; the board entry is then only used for part
clock_period=5,
clock_uncertainty=None,      # Vitis backend default, 27%
io_type='io_stream',         # only io_stream
driver='python',             # only python
input_type='float',          # float or double, must equal output_type
output_type='float',
in_stream_buf_size=128,      # FIFO depth between wrapper input and model, in stream entries (last dim of the input shape)
out_stream_buf_size=128,     # same for the output
axi_mode='axi_master',       # axi_master or axi_stream
**kwargs,                    # Vitis/Vivado writer options: namespace, write_tar, ...

version (default 1.0.0) sets package.ip.version and the driver's bindto.

Notes

• Open vitis_workspace/<project_name> in the Vitis Unified IDE to debug the kernel; the component points at the csim config.
• The m_axi depth in the AXI-master wrapper is for simulation only, one sample; the test bench sends one sample per kernel start.
• The Vivado project of the link is under vitis_workspace/system_link/_x/link/vivado/vpl/prj.
• Tutorial: the accelerator backend section of hls4ml-tutorial (prediction, simulation, bitstream, own platform).

Generated warnings

From the U-Net model used in the tests, not from the backend:

WARNING:absl:Skipping variable loading for optimizer 'Adam', because it has 17 variables whereas the saved optimizer has 1 variables.
WARNING: Config parameter "algorithm" overwrites an existing attribute in layer "up_sampling2d" (Resize)

Kernel synthesis prints the usual Vitis HLS warnings (unused parameter, deprecated pragma).

[vloncar]

Review of hls4ml PR #1376 "Vitis Unified Backend" against the VivadoAccelerator history

Date: 2026-09-09. PR head: 41b483764 (branch VitisUnifiedClean, 139 commits, based on main e99874595).

1. What was examined and how

• Issues: 77 issue threads found by keyword search (VivadoAccelerator, accelerator backend, pynq, axi_stream, axi master, zcu102, alveo, bitfile, VitisAccelerator, VitisUnified). 39 are about accelerator/board flows.
• Pull requests: 29 threads that touch accelerator backends (#349 #420 #508 #509 #552 #588 #597 #626 #642 #646 #653 #683 #694 #724 #752 #755 #812 #851 #986 #991 #1037 #1134 #1160 #1198 #1347 #1448 #1451 #1483 #1495), including their review comments.
• Git history: all 64 commits on main that touched hls4ml/backends/vivado_accelerator/, hls4ml/writer/vivado_accelerator_writer.py, hls4ml/templates/vivado_accelerator/, docs/backend/accelerator.rst (2021-06 to 2026-08), each diff read and classified as accelerator-only, shared refactor, or merge.
• VivadoAccelerator code on main and all files of PR #1376, read in full.
• Experiments: 32 conversions run one after the other with the PR checkout in an isolated venv. Each case: convert -> write -> compile() (g++ only) -> predict() -> compare with the plain Vitis backend on the same model.

Line numbers below refer to the PR head.

2. What the VivadoAccelerator history shows (the problems a successor must not repeat)

Grouped by pattern; each one is backed by issues, PR discussion and commits.

#	Pattern	Evidence
H1	Writer rewrites files the parent writer already generated, by matching strings in generated C++/Tcl and renaming a temp file over the original (modify_build_script, write_wrapper_test, full rewrite of project.tcl, delete-and-rebuild of the tar). Every parent change then needs a mirrored change; several broke silently.	vivado_accelerator_writer.py:236-359,361-398,407-411; commits ffbffd47d (inherited from pynq_writer), 6c300c053, 8c2ec95e9, 8c7a6b0b2, d1bdd3781, 4b7e12de8; issue #1157 (missing maximum_size in the duplicated project.tcl), #1194 (drifted build_lib.sh copy, fixed by copying again in #1198); stale rule 'unsigned short' in line left after c0f882c43. vloncar in #683: "I would strongly discourage this style of code in the writer. What's the point of templates if they are blank and all the logic is written by the writer".
H2	String replacement on user-controlled names. Test bench rewrite replaces every occurrence of the input/output variable name and type name on any line that contains them. A layer named input broke the project (#442); the myproject_cosim match only worked when the project was literally named myproject (eb255871a, fixed in #626).	vivado_accelerator_writer.py:296-310; #442, #626, #1495 (hard-coded type name inside a format string).
H3	Constructor bypasses the parent's __init__ (super(VivadoBackend, self).__init__), so every parent registration must be copied by hand; a missing copy broke every RNN model (#722, fixed by #724 adding one line).	vivado_accelerator_backend.py:10-12; #722, #724, 78ce8aadf.
H4	Accelerator configuration object is built inside the writer, so board/interface/type/I-O-count validation happens at write time, and the writer mutates the user's config (Part silently replaced by the board's part, with a warning printed on every default run).	vivado_accelerator_writer.py:420, vivado_accelerator_config.py:20-26,55-58; commits c28512e84, 3433c10f0; #489, #491, #516.
H5	Options accepted but not implemented: interface='axi_master', 'axi_lite', driver='c' are documented and stored, then crash at write time (depth=a emitted for axi_master). create_initial_config has no **kwargs, so Vivado writer options (namespace, write_tar) raise TypeError.	vivado_accelerator_backend.py:98-147, vivado_accelerator_config.py:138-140.
H6	Type hard-coding and type bugs in the wrapper: ap_fixed wrapper type emitted as fixed<W,I,TRN,WRAP,0> (cannot compile); input/output bit widths written swapped into project.tcl; width rounded to a multiple of 8 while keeping the integer bits; Alveo RTL fixed at 32 bits; #pragma HLS DATA_PACK (Vivado HLS only).	vivado_accelerator_writer.py:37-92,171,396-397, vivado_accelerator_config.py:85-98, krnl_rtl_int.sv:34.
H7	Single input, single output, single sample: asserts in the config, [0] everywhere, BRAM-weight ports of the core (BramFactor) not forwarded so the wrapper does not compile (#1406).	vivado_accelerator_config.py:55-62, vivado_accelerator_writer.py:130; #1406.
H8	Per-board copy-paste: pynq-z2 and zcu102 drivers byte-identical and patched in parallel (455e7345d then 1ebd1c1df seven weeks later, #420); every board request arrives as a copied driver plus a copied block-design Tcl (#752, #755, #812) and stalls; Tcl scripts hard-code part, board_part version, IP version, 100 MHz clock; drivers depend on literal block-design names (hier_0.axi_dma_0). vloncar in #755: adding a board must not require code changes.	templates/vivado_accelerator/*/; #752, #755, #812, #933 (board files not installed), #616 (Vivado 2020.1-only packaging).
H9	Build step: os.chdir + os.system inside try/except that can never fire, no exit-code check, bitfile=True needs export=True too; a failed Vivado run still returns a report.	vivado_accelerator_backend.py:46-96; #508 review (vloncar asked for subprocess), #1347 review.
H10	FIFO depth optimization pass calls model.write() and model.build() from inside an optimizer pass; reviewers asked for pre/post passes with the writer scheduled in the flow (#642, jmitrevs). The pass was copied per backend (#509 review), and later copied again into Vitis (#1037) and Coyote (#1347).	backends/vivado_accelerator/passes/fifo_depth_optimization.py; #509, #642, #1037.
H11	Accelerator knowledge leaked into shared files: _axi top name and in_local/out_local FIFO names in the shared Vivado build_prj.tcl, nnet_helpers.h (copy_data_axi), report parser.	templates/vivado/build_prj.tcl:62, nnet_helpers.h; commit 3559053b7.
H12	Silent hardware failures and no validation path: all-zero or hung boards with no maintainer answer (#282, #776, #913, #1026, #1048, #1339); driver ignores its constructor arguments and returns its internal buffer (#420 diff); no timeout on DMA waits; Alveo driver frees the overlay after the first profiled call; allocate_mem has no return (#771).	driver files; #282 #771 #776 #913 #1026 #1048 #1339.
H13	No tests, weak docs: no pytest references VivadoAccelerator at all; accelerator.rst example calls a non-existent hls4ml.build(); CLI hls4ml build does not know the backend (#401, #922); board JSON missing from the package (#515).	test/pytest, docs/backend/accelerator.rst:52; #401, #515, #922.

Lessons stated by maintainers in those threads that a successor should satisfy: configuration validated at conversion time, own templates instead of rewriting parent output, subprocess with exit codes, a board added as data (no code change), a driver whose API is stable across boards, a software validation path that uses the same data conversion as the driver, and a test in CI.

3. Findings on the VitisUnified backend (PR #1376)

Severity: blocker = wrong result, crash or unusable output for a configuration the backend accepts; major = plausible use silently does not work, or the design repeats a pattern that caused the VivadoAccelerator problems; minor = code quality. "Hn" marks the VivadoAccelerator pattern it repeats. "Verified" = reproduced in the experiments. Each finding ends with a Fix: proposal.

3.1 Wrong or silently wrong behaviour

#	Sev	Finding and suggested fix	Evidence
U1	blocker	predict() returns all zeros, without any message, when the numpy dtype does not match input_type. The bridge has both <project>_float and <project>_double entry points (as ModelGraph._get_top_function requires), but the writer emits a body only for the one whose dtype equals input_type; the other function is empty, so the outputs stay zero. Verified three times (float types + float64 input, double types + float32 input, both AXI modes). This is exactly the "board returns zeros with no explanation" class of #1026/#1048, now reproduced in software. Fix: emit both bodies and convert at the boundary, the way VivadoWriter.write_bridge does for every dtype with nnet::convert_data: copy the incoming array into a local buffer of the wrapper's type, call the wrapper, copy back. If a conversion is deliberately unsupported, the generated body must abort with a message instead of being empty.	hls4ml/writer/vitis_unified_writer.py:340 (if dtype == ...get_input_type(): with no else); templates/vitis_unified/myproject_bridge.cpp:55-69.
U2	blocker	input_type='double' with axi_mode='axi_stream' does not compile: the backend's AXI helpers hard-code hls::stream<hls::axis<float,0,0,0>> while the wrapper's packet type is hls::axis<double,...>. Verified: no matching function for call to convert_data_axis<double,double,N_IN>(...). H6 (type hard-coding) again. Fix: template the helpers on the packet type (template <class pack_T, class src_T, size_t SIZE> void convert_data_axis(src_T *src, hls::stream<pack_T> &dst)) and instantiate them with the dma_data_packet typedef the writer already generates; the writer knows the typedef name at every call site.	templates/vitis_unified/nnet_utils/nnet_helpers_axi.h:14,23,37; vitis_unified_writer.py:359,365,688.
U3	blocker	BRAM weights (Strategy: Resource with a small BramFactor) do not compile in either AXI mode: the core top function gains weight arguments, the wrapper calls myproject(stream_in, stream_out) without them. The writer even collects model_brams for the test bench includes, so the case was known. Verified: too few arguments to function 'void myproject(...)'. Same defect as VivadoAccelerator #1406 (H7). Fix: either forward the ports (append model_brams to the wrapper signature and inner call, load them in the test bench and bridge, and give them m_axi/s_axilite interface pragmas so weights become run-time loadable — the feature #113 asked for in 2018), or reject the configuration at conversion with "BramFactor weights are not supported by VitisUnified". Silent acceptance followed by a C++ error is the one wrong option.	vitis_unified_writer.py:548-555,310,658; templates/vitis_unified/myproject_axi_master.cpp:41-45, myproject_axi_stream.cpp:38-43.
U4	major	create_initial_config swallows every Vivado/Vitis writer option (namespace, write_tar, write_weights_txt, write_emulation_constants, tb_output_stream) through **_ and does not forward them to super().create_initial_config(part, clock_period, clock_uncertainty, io_type). Verified: namespace='nsone' and write_tar=True give Namespace: null, WriteTar: false and no tar file, with no warning. VivadoAccelerator raised TypeError (H5); silent is worse. The writer does contain namespace code paths that can never be reached this way. Fix: name the backend-specific parameters explicitly, collect the rest as **kwargs, and pass them through: super().create_initial_config(part=part, clock_period=clock_period, clock_uncertainty=clock_uncertainty, io_type=io_type, **kwargs). Then any option the parent grows is inherited automatically.	vitis_unified_backend.py:119-145; vitis_writer.py:58-70.
U5	major	build() returns None: no report is parsed; the CI helper run_synthesis_test then fails its assert data and expected_keys.issubset(...) check on the None return (save_report(None) itself writes null without raising), and synthesis_helpers.py has neither a build_args nor an EXPECTED_REPORT_KEYS entry for this backend, so a generic synthesis test calls build() with all flags False, runs nothing, and fails on the empty result. The public build() signature also differs from every other backend: validation, export removed; reset, vsynth accepted but ignored; synth=True always also runs package; cosim=True without synth=True runs cosim on a project that was never synthesized. Fix: keep the parent's keyword set and raise or warn on the unsupported ones instead of dropping them from the signature; implement reset (delete vitis_unified_project and the linker work dir); make cosim imply synth the way fifo_opt already implies cosim; parse vitis_unified_project/reports/* (csynth and cosim reports) into the dict shape parse_vivado_report produces and return it; add a build_args['VitisUnified'] entry to synthesis_helpers.py.	vitis_unified_backend.py:17-101; test/pytest/synthesis_helpers.py:137-165.

3.2 Architecture: the same anti-patterns, in a new place

#	Sev	Finding and suggested fix	Evidence
U6	major	Constructor bypasses two parents: super(VivadoBackend, self).__init__ skips VitisBackend.__init__ and VivadoBackend.__init__ and re-does _register_layer_attributes() and _register_flows() by hand. This is H3, the pattern that produced #722; any future registration added to VitisBackend.__init__ is silently missed here. (nghielme raised the same point on #1134.) Fix: the parent __init__ methods only set the name and call the two _register_* methods, and _register_flows is already overridden in this class — so plain super().__init__(name='VitisUnified') cannot be used only because the parents hard-code their names. Either give the backend constructors a name parameter upstream (small, benefits every derived backend), or keep the bypass but add a comment listing exactly what is being re-done, plus a test that fails when VitisBackend.__init__ gains a step (compare the set of registered flows/attributes against a fresh VitisBackend).	vitis_unified_backend.py:11-15.
U7	major	Config object created inside the writer (_set_unified_config in write_hls), so board, axi_mode, driver, type equality and input/output-count checks all run at write time. Verified: unknown board, axi_stream with 2 inputs, and input_type != output_type all pass convert_from_keras_model and fail only in compile(); the last one with an assert whose message reads "must be the same type different". create_initial_config accepts any board and silently substitutes the zcu102 part. H4 again (without the Part override, which is an improvement). Fix: validate everything that does not need the graph (board, axi_mode, driver, types) in create_initial_config, and everything that needs the graph (input/output counts vs axi_mode) in a ModelOptimizerPass registered in the backend's default flow, so convert_from_keras_model fails immediately; replace the asserts with exceptions (asserts vanish under python -O) and fix the message. The writer then only consumes an already validated config.	vitis_unified_writer.py:63-66,787-799; vitis_unified_config.py:11-20,61,67,70-81; vitis_unified_backend.py:134-143.
U8	major	Parent writer output is left in the project as dead or misleading files. VitisWriter.write_hls still copies build_prj.tcl and build_opt.tcl (the Tcl flow this backend does not use) and VivadoWriter.write_test_bench still writes <project>_test.cpp next to the backend's own myproject_test.cpp; the two test benches call different top functions (with the default project name the file names coincide and the backend's version happens to overwrite the parent's). project.tcl is not written at all, which is why write_board_script_override had to be turned into a no-op. write_tar runs up to three times. Verified in the generated directories. This is the mirror image of H1: instead of rewriting parent files, the subclass leaves them in place and adds its own beside them. Fix: override write_build_prj_override and write_build_opts as documented no-ops (the mechanism already used for write_board_script_override), and override write_test_bench so the backend's wrapper test bench is the test bench — written once, under <project>_test.cpp (which also removes the myproject_test hard-coding, U12). A user opening the project should not have to guess which files the flow actually uses.	vitis_unified_writer.py:22-23,35-38,140-147,649-653,787-799; vitis_writer.py:59-76; vivado_writer.py:1134-1148.
U9	major	Build step rewrites generated files and derives configs by substring editing. prepare_sim_config_file copies hls_kernel_config_{csim,cosim}.cfg over hls_kernel_config.cfg at build time and string-replaces a placeholder; the csim variant is produced by commenting out any template line containing enable_fifo_sizing or -DRTL_SIM. vitis-comp.json points at hls_kernel_config.cfg, which does not exist until build() runs, so the advertised "open the workspace in the Vitis IDE" does not work on a freshly written project. Fix: resolve both cfg files completely at write time (the FIFO-sizing flag is a config value the writer already has, not something only build() knows) and pass the wanted file directly to v++ --config / vitis-run --config; point vitis-comp.json at the csim cfg. Then build() never edits generated files and the IDE works from the moment the project is written.	vitis_unified_backend.py:103-117; vitis_unified_writer.py:167-196,209-218.
U10	major	FIFO depth pass calls model.write() and model.build() from inside transform() (H10; copied from the Vitis pass, itself copied from VivadoAccelerator). The cus_hls_prj_path default builds a path (<out>/<project>/_prj/solution1) that does not exist for this backend. Fix: apply the #642 proposal where it is inherited from — split the Vitis pass into a pre-pass (enlarge FIFOs), the normal write+build scheduled by the flow, and a post-pass (read the measured depths, set them) — and let this backend inherit the split instead of adding a fourth copy of the pattern.	backends/vitis_unified/passes/fifo_depth_optimization.py:26-27,61-72,104-110.
U11	major	Absolute output paths baked into generated files: syn.file={OUTDIR}/..., tb.file={OUTDIR}/... in both cfg files and configFiles in vitis-comp.json. The project cannot be moved, tarred, or built on another machine (a common user workflow in the issue history). VivadoAccelerator's Tcl used relative paths. Fix: emit paths relative to the directory the tool is started from (the backend controls cwd for every command it launches) or relative to the cfg file, and compute them with os.path.relpath. A generated project must build after mv.	templates/vitis_unified/hls_kernel_config.cfg:7-18; vitis_unified_writer.py:182-183,216-217.
U12	major	Hard-coded names and tool/platform versions (H8): test bench file name myproject_test regardless of project name (verified: myproject left in the cfg files of a project named mynet); XILINX_VITIS default /opt/Xilinx/Vitis/2023.2; link_system.sh hard-codes _x/link/vivado/vpl/prj/prj.gen/sources_1/bd/vitis_design/hw_handoff/vitis_design.hwh and needs xclbinutil and vivado on PATH without checking; package.ip.version=1.0.0 fixed and the driver binds to xilinx.com:hls:<top>:1.0, so the Version config that #851 added is ignored; flow_target=vivado fixed; clock_uncertainty default 12.5% (Vivado) although the Vitis backend uses 27%. Fix: make _get_sim_file_name return <project>_test (folds into U8); derive XILINX_VITIS from the environment and fail with a clear message when unset; glob for the .hwh under _x/link instead of a fixed path; substitute the Version config into package.ip.version and the driver's bindto; inherit the clock-uncertainty default from the Vitis backend; check for xclbinutil/vivado in build() next to the existing v++/vitis-run checks.	vitis_unified_writer.py:81-82, vitis_unified_config.py:105-108, hls_kernel_config.cfg:19-21, link_system.sh:7-12, vitis_unified_backend.py:124.
U13	major	Adding a board still requires editing package files: boards live in supported_boards.json inside the package plus a per-board directory of drivers and Tcl; there is no way to pass a custom .xpfm/.xsa path (the PR description still advertises xpfmPath, the code no longer has it). This is the exact situation of #752/#755/#812 (H8) that vloncar asked to solve with an extension point. Fix: accept a platform argument in create_initial_config holding a path to an .xpfm or .xsa; when given, skip the board lookup for the platform (the board entry then only supplies the part and driver choice, both of which can also be arguments). Since v++ links against a platform file, this makes an unlisted board usable with zero code changes — the main structural advantage this backend has over VivadoAccelerator, currently not exposed.	vitis_unified_config.py:5-58,86-109; PR description "configuration" section.
U14	major	Drivers are per-board copies again (H8): zcu102/python_drivers/* and kv260/python_drivers/* are byte-identical (verified with diff). The AXI-master driver computes the register map in Python (0x10 base, 12-byte stride per pointer, batch-size register after the pointers) instead of reading it from the generated .hwh/HLS driver header; the AXI-stream driver hard-codes axi_dma_0 and axi_dma_0/s2mm_introut; both call PL.reset() on every predict, use one dtype for input and output, and keep a long docstring about ap_fixed encode/decode although only float/double are allowed. c_drivers are empty placeholders and driver is asserted to be python at write time (H5). Fix: keep one driver template per AXI mode under templates/vitis_unified/drivers/ and drop the per-board directories (they are already identical); on the board, take the register offsets from pynq's ip.register_map (pynq parses the .hwh, so the offsets are always right) instead of the Python-side arithmetic; move PL.reset() to __init__ or make it an argument; separate input and output dtype; delete the ap_fixed docstring or implement encode/decode; remove the c_drivers placeholders until a C driver exists.	templates/vitis_unified/zcu102/python_drivers/axi_master_driver.py.hls4ml:53-110,116-137; axi_stream_driver.py.hls4ml:62-66; vitis_unified_writer.py:616-621; vitis_unified_config.py:61.

3.3 Minor findings

#	Sev	Finding and suggested fix	Evidence
U15	minor	String-matching template fill with fragile ordering: elif 'myproject' in line before PROJECT_FILE_NAME; 'STREAM_BUF_IN_SZ' in line: line.replace('VAL', ...); markers consumed differently in .cpp (replaced) and .h (appended). Works for the shipped templates, breaks the moment a template line contains two placeholders. The AXI-master template also contains a stray label mem_rd: before a declaration. Fix: one substitution style: unique {PLACEHOLDER} tokens applied with unconditional line.replace on every line (no elif chains keyed on substrings), insertion markers only for multi-line blocks; remove the stray labels.	vitis_unified_writer.py:302-320,410-451,482-569,575-584; myproject_axi_master.cpp:11,27.
U16	minor	Copy-paste and dead code: exec_dir = self.get_vitis_hls_dir(model) (should be get_vitis_hls_exec_dir), _, _, _, _ = ...get_corrected_types(), write_tar override that only calls super(), _get_kernel_declaration always uses the axi_stream name even in axi_master mode (same length today, wrong by construction), get_XPFMPath/get_platform_path aliases, getattr defaults for attributes that are always set. Fix: delete each; make _get_kernel_declaration take the mode from _is_axi_master().	vitis_unified_writer.py:22-23,84-87,201,655; vitis_unified_config.py:163-176.
U17	minor	The 64-character kernel-name guard is a real improvement, but the message "Project name must not exceed 18 characters" hard-codes the arithmetic of the current suffixes. Fix: compute the allowed length from the actual suffix lengths and put both numbers in the message.	vitis_unified_writer.py:40-47.
U18	minor	The AXI-stream framing contract is implicit. The last-beat condition (q == batch_size-1) && ((chunk_idx+1)*(elem_idx+1) == N_OUT) is correct only because equality is reachable solely at both loop maxima — nothing says so; TLAST is raised once per batch, not per sample; incoming TLAST is ignored and exactly batch_size*N_IN beats are consumed (a short transfer hangs, the failure mode of #776/#913/#1339). Fix: write the condition as (chunk_idx == N_OUT/OUTPUT_LAYER_TYPE::size - 1) && (elem_idx == OUTPUT_LAYER_TYPE::size - 1), and state the contract (beats expected per start, TLAST behaviour in both directions) in the docs and in the driver docstring.	templates/vitis_unified/myproject_axi_stream.cpp:24-35,3-18.
U19	minor	The m_axi pragmas set depth=<one sample> while the kernel reads batch_size samples per start; depth only affects simulation, so cosim with a batch larger than 1 is either misleading or warns. Fix: set depth from a documented cosim batch (or max_batch_size config value) and add a comment that depth is simulation-only.	vitis_unified_writer.py:485-498.
U20	minor	TKEEP/TSTRB side channels are carried but handled asymmetrically. The AXI-stream packet type is the full hls::axis<T,0,0,0>: the output path sets keep = -1 on every beat (the Vitis-example idiom, functionally fine for full-width float/double payloads), but TSTRB is never written anywhere (undefined on the wire), the simulation helpers create packets without setting keep at all (simulation and hardware traffic disagree), and the input path ignores TKEEP, so a sparse beat from a master would be consumed as if full, silently. Since this backend only ever transfers whole 4- or 8-byte elements, TKEEP carries no information here. Fix: use the signal selection the bundled ap_axi_sdata.h provides — hls::axis_data<T, AXIS_ENABLE_LAST> — so the interface has only TDATA/TVALID/TREADY/TLAST: the keep = -1 assignment disappears and no side channel can be left undriven or inconsistent (absent TKEEP means "all bytes valid" per the AXI-Stream specification, and IPI ties off the DMA's TKEEP input; VivadoAccelerator's own data+last struct relied on the same behaviour; one board test should confirm the v++ stream_connect path accepts it). If TKEEP is kept for platform compatibility, use `AXIS_ENABLE_LAST	AXIS_ENABLE_KEEP, set keep` in every place a packet is created (wrapper and both simulation helpers), and state in the U18 contract that input TKEEP is not honored mid-packet — which matches what AXI DMA MM2S produces for aligned buffers.

3.4 Tests and documentation (required before merge)

#	Sev	Finding and suggested fix	Evidence
U21	major	Test coverage is far too narrow for a backend that aims to be the default. The suite covers one Conv2D U-Net and one 2-in/2-out CNN, io_stream, Latency, float, clock_period=10 hard-coded; bitstream tests are skipped by default; nothing asserts on generated code except the driver port counts. None of U1-U4 would have been caught. Fix: a small matrix that covers many axes with few builds, all in the g++-only software path so it runs in normal CI: (a) parametrize the existing predict-parity test over axi_mode x input_type in {float,double} (4 cases — catches U2 and any future type regression); (b) one test that calls predict with the wrong numpy dtype and asserts a correct result or a raised error, not zeros (catches U1); (c) one Resource + BramFactor conversion asserting either a working compile or a clean conversion-time rejection (catches U3); (d) one conversion with project_name='custom', namespace, write_tar=True asserting the tar exists, the namespace appears in the generated header, and no myproject literal remains outside nnet_utils (catches U4, U12); (e) one invalid-configuration test asserting that unknown board / mismatched types / multi-input axi_stream fail at convert_from_keras_model, not at compile() (locks in U7 once fixed). That is roughly seven cheap tests; the existing synthesis/cosim/bitstream tests stay opt-in as they are.	test/pytest/test_vitis_unified.py.
U22	major	Documentation gaps. docs/backend/vitis_unified.rst says an unknown board falls back to the zcu102 part, but the config object rejects unknown boards at write time (contradiction with the code); nothing says how this backend relates to VivadoAccelerator (default for AMD SoC boards? deprecation? coexistence?) and accelerator.rst still presents VivadoAccelerator unchanged; the PR description advertises xpfmPath, which no longer exists; the build() flags and their differences from other backends are undocumented; the AXI-stream framing contract (U18) and the exact meaning of in_stream_buf_size/out_stream_buf_size in each mode are not stated; there is no "how to add a board / bring your own platform" section although that is the request that dominated the VivadoAccelerator backlog (#752/#755/#812). Fix: correct the unknown-board paragraph to match the code (or the code to match it, per U7/U13); add a short positioning paragraph to accelerator.rst naming VitisUnified as the recommended flow for AMD SoC boards and what remains VivadoAccelerator-only; document build()'s arguments and outputs; document the stream contract and buffer sizes; add the bring-your-own-platform section once U13 lands (they belong in the same change).	docs/backend/vitis_unified.rst:70-74; docs/backend/accelerator.rst; PR description.

3.5 What the PR does better than VivadoAccelerator

Own templates for wrapper, bridge and test bench (no rewriting of parent-generated files, no os.rename); subprocess.Popen with return-code checks and per-step logs instead of os.chdir/os.system; set -e in the link script; multiple inputs and outputs in axi_master mode (verified in software: 2-in/2-out and 1-in/3-out give results identical to the Vitis backend); whole-batch execution per kernel start with a batch_size register; interrupt-driven driver; v++ platform linking (a platform is data, not a hand-maintained block-design script per board, at least for zcu102/axi_master); a current tool (Vitis HLS, .cfg flow) instead of the discontinued Vivado HLS and DATA_PACK; backend-local simulation headers copied by the backend's own writer step (ap_axi_sdata.h taken from AMD's Apache-2.0 Xilinx/hls-utilities, with the source recorded in the file); a kernel-name-length guard; a pytest file; a documentation page; Trace works through the wrapper (verified in both modes).

4. Adaptability beyond the shipped examples

Results of the 32 sequential cases (software path only: write, g++ compile, predict, compared with the Vitis backend on the same model). "identical" = max abs difference 0.0.

Situation	Result
Dense MLP, axi_master / axi_stream	works, identical
MLP with sizes 9 -> 11 -> 7 (axi_stream)	works, identical
Conv1D + Flatten + Dense, both modes	works, identical
Conv2D U-Net (the PR's example), both modes	works, identical
LSTM + Dense (axi_master)	works, identical
2 inputs / 2 outputs, axi_master	works, identical
1 input / 3 outputs, axi_master	works, identical
2 inputs / 2 outputs, axi_stream	rejected, but only at compile() (write time), not at conversion
io_type='io_parallel'	rejected at conversion ("io_type must be io_stream"). VivadoAccelerator supported io_parallel; small Dense models lose the option recommended in #425
input_type=output_type='double', axi_master	works, identical
input_type=output_type='double', axi_stream	compile error (U2)
types double, predict with float32 array	all zeros, no error (U1)
types float, predict with float64 array, either mode	all zeros, no error (U1)
input_type='float', output_type='double'	rejected at write time by an assert with a garbled message (U7)
input_type='ap_fixed<16,6>'	rejected at conversion. VivadoAccelerator accepted it (with bugs, H6); the new backend is float/double only
Trace=True + trace() , both modes	works, all layer outputs returned
Resource strategy with BramFactor=0 (BRAM weight ports), both modes	compile error (U3)
project_name='mynet', both modes	works; but the test bench is still named myproject_test.cpp and the cfg files reference it (U12)
namespace='nsone'	silently ignored (U4)
write_tar=True	silently ignored (U4)
IOType placed in hls_config instead of the kwarg	not seen by the backend at all (kwarg default used); generic hls4ml behaviour
board pynq-z2 (not in the JSON)	rejected at write time, after conversion succeeded (U7); no way to supply a platform path (U13)
board kv260, axi_stream	works
no axi_mode given	defaults to axi_master, works

Not executed, judged from the code: PyTorch/ONNX frontends go through the same ModelGraph and should behave like Keras (the backend never looks at the frontend); multigraph is rejected explicitly; a board other than zcu102/kv260 needs a new package directory with Tcl and drivers, exactly as before; Alveo/PCIe cards are out of scope (the #991 VitisAccelerator effort was to be merged into this PR according to #991, but nothing of it is here); a C/C++ host driver (#1339) is absent.

Overall: the software path adapts well across layer types and input/output counts (in axi_master mode), which is a real step beyond VivadoAccelerator. It does not adapt across io types (io_stream only, by design), interface types (float/double only, with double broken for axi_stream and dtype mismatches silently returning zeros), weight storage (BRAM ports), writer options (silently dropped), or boards/platforms (package data only).

5. What should change, in order

Every numbered finding (U1-U22) appears in at least one item below, so working through the list covers the whole review, not only the items marked as required. All of these are proposals: the authors should push back on any item where they consider the finding not relevant or the proposed fix the wrong approach — a reasoned disagreement recorded on the PR is a valid way to close an item.

Required before merge:

1. Fix the silent-zero bridge: both dtype entry points convert instead of one being empty (U1).
2. Template the AXI helpers on the packet type so double works in axi_stream mode (U2).
3. Forward BRAM weight ports or reject BramFactor at conversion (U3).
4. Expand the tests with the small wide-coverage matrix of U21 (about seven g++-only cases: axi_mode x float/double parity, wrong-dtype behaviour, BramFactor, custom name + namespace + tar, invalid configurations rejected at conversion).
5. Fix the documentation (U22): unknown-board paragraph matching the code, positioning against VivadoAccelerator in accelerator.rst, build() arguments and outputs, the AXI-stream contract and buffer sizes, and removal of stale options (xpfmPath) from the description.

Needed for it to be a credible default:

6. Validate at conversion time and forward parent options: config checks in create_initial_config plus a graph-level pass, exceptions instead of asserts, **kwargs pass-through, parent constructors called or their bypass tested (U4, U6, U7).
7. Make the build honest and compatible: parent-compatible build() keywords, a returned report, cfg files resolved at write time, relative paths so the project survives a move (U5, U9, U11).
8. Boards and platforms as data: a platform path argument, one shared driver template with the register map read from the hardware description, no per-board copies, no hard-coded tool paths (U12, U13, U14).
9. Take a position on the capability gaps against VivadoAccelerator listed in section 4 (io_parallel, ap_fixed interface types): either plan them, or document them as deliberate non-goals as part of the U22 documentation work.

Cleanups and longer term:

10. Neutralize the unused parent writer steps and unify the test bench under the project name (U8, U12).
11. Split the FIFO pass in the Vitis backend (pre-pass / flow-scheduled build / post-pass, the #642 design) and inherit it here (U10).
12. The minor items: one substitution style in the writer, dead-code removal, clearer guard message, explicit stream contract in the wrapper source, simulation-depth comment, side-channel selection instead of per-beat keep = -1 (U15-U20).

6. Verdict

The PR is a real improvement in tool generation (Vitis HLS, v++ platform linking), in the build step (subprocess, logs, error propagation), and in interface generality (multiple inputs and outputs, batch execution), and it touches no file outside its own backend and template directories. It does not yet meet the bar of "improvement in all areas": two silent-failure modes in the software path (dtype mismatch returns zeros — U1; writer options dropped without a message — U4), two accepted configurations that do not compile (double + axi_stream — U2; BramFactor weights — U3), test coverage too narrow to have caught any of these (U21), documentation that contradicts the code in places and does not position the backend against VivadoAccelerator (U22), capability narrower than VivadoAccelerator in three places (io_stream only, float/double only, no BRAM weights), and five of the VivadoAccelerator patterns that caused most of that backend's issue history (constructor bypass, write-time validation, hard-coded names and tool versions, per-board driver copies, FIFO pass building the project from inside a pass). Items 1-5 of section 5 are required before merge; items 6-9 are what would make it a credible default.

[Tanawin1701d]

Thanks for the detailed review. All 22 findings are addressed in the 20 commits after the reviewed head 41b4837, now merged into the PR branch. Status per item below, grouped as in section 5 of the review. "Note" marks where I did something different from the proposed fix, or something worth knowing.

Required before merge

U	Status	Note
U1	Fixed	Both bridge entry points are filled; predict() converts at the boundary for any numpy dtype (3e349ab).
U2	Fixed	AXI helpers templated on the packet type (d52a136). New: double on axi_stream is rejected with the shipped platforms, their AXI DMA is 32 bits wide; a user platform with a 64-bit DMA is still accepted (6cdc412).
U3	Fixed	Chose rejection: BramFactor weights stop at conversion with a clear error, ports are not forwarded (c36e3f1).
U21	Fixed	41 tests in six sections by flow depth: config rejection, BRAM rejection, custom name/namespace/tar, driver contents, custom platform, dtype parity vs the Vitis backend, report parsing; csim/cosim/FIFO and bitstream tests stay opt-in (70334c2, 6910ae9).
U22	Fixed	Unknown-board paragraph matches the code, build() options table, stream contract, buffer sizes, own-platform option, limitations (cbb5e2e). Positioning sentence is on the VitisUnified page; accelerator.rst is untouched, out of scope of this PR. The PR description now shows the platform argument; the old xpfmPath / XPFM_PATH text is gone.

Needed for it to be a credible default

U	Status	Note
U4	Fixed	Named backend parameters plus **kwargs forwarded to the parent (4b89553).
U6	Fixed	name parameter on the Vivado and Vitis constructors, super().__init__(name='VitisUnified') (8a7d812). VivadoAccelerator and Coyote constructors untouched.
U7	Fixed	Graph-free checks in create_initial_config (vitis_unified_validation.py), graph checks in a pass on the default flow; exceptions instead of asserts (0683968).
U5	Fixed	Parent keyword set, unsupported ones warn, reset implemented, cosim implies synth, report dict returned, CI helper entries (940307b).
U9	Fixed	Three cfg files written at write time, vitis-comp.json points at the csim one, build() never edits generated files (e9761d9).
U11	Fixed	All paths relative to the component directory; a moved project builds (bab531f).
U12	Fixed	<project>_test, XILINX_VITIS from the environment with a clear error, .hwh searched (exactly one expected), version into package.ip.version and the driver bindto, clock uncertainty inherited (27%), xclbinutil/vivado checked (a2b91a0). flow_target=vivado stays, by design.
U13	Fixed	platform (.xpfm/.xsa) and part arguments; a board outside the JSON needs no code change (a2b91a0).
U14	Fixed	One driver per AXI mode under templates/vitis_unified/drivers/, per-board copies and c_drivers removed, register offsets read from the .hwh, PL.reset() in __init__ (reset_pl), separate output dtype, dma_name argument, ap_fixed docstring removed (a2b91a0).
item 9	Done	io_parallel and ap_fixed interfaces documented as non-goals of this backend.

Cleanups and longer term

U	Status	Note
U8	Fixed	build_prj.tcl/build_opt.tcl no longer written, one test bench under <project>_test.cpp, tar written once (18011f4).
U10	Fixed	Vitis FIFO pass split into fifo_depth_profiling (pre pass, write, cosim, post pass) and fifo_depth_optimization flows; VitisUnified inherits them (d102cca).
U15	Fixed	One substitution style (_fill_template, {PLACEHOLDER} tokens, markers only for blocks) (35c6731). The mem_rd/mem_wr labels are kept: they now label the loops and help when reading synthesis reports.
U16	Fixed	Dead code removed, _get_kernel_declaration uses the AXI mode (35c6731).
U17	Fixed	Limit computed from the real suffix lengths, both numbers in the message (35c6731).
U18	Fixed	Last-beat condition written per loop index; contract stated in the docs, the driver docstring and the wrapper source (35c6731, 25eaacd).
U19	Fixed	Comment that depth is simulation-only; kept at one sample, which is what the test bench sends per start.
U20	Fixed	Option 2: `hls::axis_data<T, AXIS_ENABLE_LAST

Changes outside the VitisUnified backend

File	Change	Why	For
backends/vitis/passes/fifo_depth_optimization.py, backends/vitis/vitis_backend.py	FIFO pass split into pre / profile / post passes and two flows	So VitisUnified inherits the #642 design instead of adding a fourth copy of the pass	U10
backends/vivado/vivado_backend.py, backends/vitis/vitis_backend.py	name parameter on the constructors, defaults unchanged	Lets the subclass call super().__init__() instead of bypassing it	U6
report/vivado_report.py, report/__init__.py	csynth and cosim parsers extracted into functions	Same report formats, only the paths differ; the unified report reuses them	U5
writer/vivado_writer.py	write_tb_data split out of write_test_bench	The unified test bench needs the same tb_data files without a second <project>_test.cpp	U8
test/pytest/conftest.py, test/pytest/synthesis_helpers.py	VitisUnified entries	The CI synthesis helper is table-driven per backend	U5

No behaviour change for the Vivado, Vitis, VivadoAccelerator or Coyote backends.

Test results

• test/pytest/test_vitis_unified.py, without Vitis: 32 passed, 9 skipped (gated).
• With Vitis 2023.2: all 41 pass. csim, cosim and FIFO depth optimization for both AXI modes; kv260 bitstreams for axi_stream, axi_master and a 2-input/2-output axi_master model.

KV260 (PYNQ 3.0.1), generated drivers, batch of 10, compared with the software prediction saved by the test:

Bitstream	AXI mode	Ports	max abs diff hw vs sw	Second run identical
simple U-Net	axi_stream	1 in, 1 out	0.0	yes
simple U-Net	axi_master	1 in, 1 out	0.0	yes
2-in / 2-out CNN	axi_master	2 in, 2 out	0.0	yes

Script: test/board/vitis_unified_hw_test.py.

[vloncar]

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.

[vloncar]

The test failures are unrelated to this PR. Merging. Huge thanks to the team for making this.