XLS: support all hls4ml rounding modes. - #1532
Open
vasdommes wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the XLS backend’s fixed-point rounding support to cover all hls4ml rounding modes by introducing an hls4ml-compatible DSLX rounding module and wiring it into existing fixed-point utilities.
Changes:
- Added a new DSLX rounding implementation (
ap_round.x) adapted from XLS stdlib to support the full hls4mlRoundingModeset. - Updated
fixed_point_util.xto use the newap_roundmodule directly (removing the previous partial adapter/conversion logic). - Added DSLX unit tests to validate all rounding modes and verify equivalence for modes that map to XLS stdlib rounding.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
hls4ml/templates/xls/firmware/ap_types/fixed_point_util.x |
Switches rounding to use ap_types.ap_round and removes the old partial rounding-mode conversion layer. |
hls4ml/templates/xls/firmware/ap_types/ap_round.x |
New rounding implementation and tests supporting all hls4ml rounding modes (with conversions for stdlib-equivalent modes). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ry #![feature(type_inference_v2)
Contributor
|
Is it worth adding explicit pytests for the rounding modes? |
Contributor
|
Probably a good idea to add those. |
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.
Description
Currently, XLS backend supports only rounding modes that exist (under different names) in DSLX stdlib:
TRN,TRN_ZERO,RND_INF,RND_CONV.hls4ml/hls4ml/templates/xls/firmware/ap_types/fixed_point_util.x
Lines 582 to 596 in f982546
This PR adds missing rounding modes.
Implementation:
enum RoundingModeto the one from hls4mlround().Type of change
Tests
Unit tests in DSLX code can be run e.g. as (for xls build from sources in
$HOME/xls):All existing hls4ml tests pass.
I also ran softmax tests based on the commit e5bf4af0 from PR #1476. These tests originally failed with
(which means
RoundingMode::RND)Checklist
pre-commiton the files I edited or added.