fix: strip MLIR %alloc names from VHLS csim output and handle ap_int in nanobind wrapper#554
Open
sunwookim028 wants to merge 4 commits into
Open
Conversation
Member
|
Can you add test cases for this? |
Contributor
Author
|
Black formatting fixed. Added unit tests in |
…n nanobind wrapper The VHLS C++ emitter emits raw MLIR SSA value names (e.g. %alloc, %alloc1) into the generated kernel.cpp. These are illegal C++ identifiers and cause g++ to fail with 'expected unqualified-id before % token' when IPModule compiles the csim nanobind wrapper. Fix 1 (vitis.py): Add re.sub(r'%(\w)', r'\1', hls_code) at the start of postprocess_hls_code() to strip the MLIR % sigil from all identifiers. The C++ modulo operator is always followed by a non-word character so this substitution is safe. Fix 2 (ip.py): Update parse_cpp_function() to handle ap_int<N>/ap_uint<N> type names (old regex stopped at '<'). Add resolve_nb_type() to map HLS types to nanobind-compatible stdint types (int8_t, uint16_t, etc.) in the generated nanobind wrapper, with reinterpret_cast back to the HLS type when calling the kernel. Fixes test_three_level_systolic_csim which was the only failing test.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add 3 new tests to test_backend_utils.py: - test_postprocess_realistic_mlir_snippet: the exact MLIR-emitted C++ pattern (int16_t %alloc[4][4]) that originally broke g++, with a C++ modulo on the same code path to verify selective stripping - test_parse_cpp_function_plain_types: regression guard ensuring the broadened _TYPE_TOKEN regex did not break plain-type parsing (int8_t, float, int) - test_generate_nanobind_wrapper_uses_stdint_for_ap_int: integration test that constructs an IPModule from a tempfile and verifies the generated wrapper uses int8_t/uint16_t in the nb::ndarray<> signature while keeping ap_int<N> in the reinterpret_cast body All 12 tests pass (up from 9). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sunwookim028
force-pushed
the
fix/vhls-mlir-percent-alloc-csim
branch
from
9479101 to
fb475cb
Compare
Contributor
Author
|
Rebased onto current |
Contributor
Author
|
Hi @chhzh123! Just a gentle ping — it's been about 9 days since the last update. CI is green and all requested tests are in place (12 cases in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When running
test_three_level_systolic_csim, the Vitis HLS C++ emitter generateskernel.cppcontaining raw MLIR SSA names like%alloc[1]— invalid C++ identifiers. This causesg++to fail with:A secondary issue: [parse_cpp_function()](cci:1://file:///home/sk3463/main/projects/allo/allo/backend/ip.py:30:0-112:17) in
ip.pyused\w+to match C++ types, which stops at<, soap_int<8>was misread (only8was captured as the type name), producing invalid generated nanobind wrapper code.Fix
vitis.py: Addre.sub(r'%(\w)', r'\1', hls_code)at the start of [postprocess_hls_code()](cci:1://file:///home/sk3463/main/projects/allo/allo/backend/vitis.py:377:0-429:18). The MLIR%sigil always precedes a word character; the C++ modulo operator does not. Safe substitution.ip.py: Broaden type-matching regex to handleap_int<N>/ap_uint<N>types. Add [resolve_nb_type()](cci:1://file:///home/sk3463/main/projects/allo/allo/backend/ip.py:16:0-27:19) to map HLS types toint{N}_tfor the nanobind interface, withreinterpret_castback to the HLS type when calling the kernel.Testing
tests/test_systolic_array.py::test_three_level_systolic_csimnow passes. This test only runs when Vitis HLS is available, which is why it was not caught by upstream CI.