Skip to content

Expose AxiLookBits in axi_burst_splitter to avoid logic explosion - #436

Open
briann-bui wants to merge 8 commits into
pulp-platform:masterfrom
briann-bui:fix/issue-341-axi-look-bits
Open

Expose AxiLookBits in axi_burst_splitter to avoid logic explosion#436
briann-bui wants to merge 8 commits into
pulp-platform:masterfrom
briann-bui:fix/issue-341-axi-look-bits

Conversation

@briann-bui

Copy link
Copy Markdown

Fixes #341.

This PR exposes the AxiLookBits parameter in axi_burst_splitter (and cascades it through to axi_burst_splitter_gran and axi_to_axi_lite).

Previously, the internal demux always instantiated with AxiLookBits hardcoded to the full IdWidth, which caused a massive logic explosion (counter array size of 2**IdWidth) when dealing with large AXI IDs. By exposing this parameter (defaulting to IdWidth for backwards compatibility), users can now override it appropriately.

@imchenwu
imchenwu changed the base branch from master to devel July 24, 2026 14:48

@imchenwu imchenwu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thnaks for working on it :).

  1. axi_burst_unwrap instantiates its internal demux with the same hardcoded .AxiLookBits(IdWidth) and should also expose this parameter, otherwise it has the same issue.
  2. When exposing the new parameter, please add a one-line comment explaining its usage and the implicit constraint (AxiLookBits <= AxiIdWidth). You can simply point to the existing documentation in doc/axi_demux.md.
  3. It would be good to add a parameter check in the end of axi_burst_splitter_gran, e.g.
    assume (AxiLookBits > 0 && AxiLookBits <= IdWidth) else
      $fatal(1, "AxiLookBits (%0d) must be in ]0, IdWidth (%0d)]!", AxiLookBits, IdWidth);

also add the corresponding check in axi_to_axi_lite module.

@briann-bui

Copy link
Copy Markdown
Author

Hi @imchenwu, I've pushed a new commit to address all 3 of your requested changes. Could you please take another look when you have a moment? Thanks!

Comment thread src/axi_burst_splitter.sv
Comment thread src/axi_burst_splitter_gran.sv
Comment thread src/axi_burst_unwrap.sv Outdated
Comment thread src/axi_to_axi_lite.sv
Comment thread src/axi_to_axi_lite.sv
Comment thread src/axi_burst_splitter_gran.sv Outdated
@briann-bui

Copy link
Copy Markdown
Author

@imchenwu, I have addressed all six review threads in commit 4089aac5:

  • added the requested AxiLookBits/AXI_LOOK_BITS parameter documentation;
  • moved the AxiLookBits parameter check from axi_burst_splitter_gran_counters into axi_burst_splitter_gran;
  • verified the complete synthesis source set and axi_synth_bench compile successfully with Bender 0.30.0 and VCS X-2025.06.

All conversations are resolved. Could you please review again when convenient? Thanks!

imchenwu and others added 5 commits July 28, 2026 16:12
Document INCR/FIXED/WRAP burst behavior in the module headers of
axi_dw_converter, axi_dw_downsizer, and axi_dw_upsizer.

Closes pulp-platform#429.
… interfaces.

The original multi-dimensional interface module is not supported by many tools.
Downstream users can still trivially write their own intf wrapper.

Close pulp-platform#353.
…meter (pulp-platform#399)

* src: axi_fifo_delay_dyn: Fix incorrect power-of-two check for `Depth` parameter

The `axi_fifo_delay_dyn` module's parameter constraint checker incorrectly
identified valid `Depth` values as not being a power of two.

This commit corrects the logic to accurately verify that the `Depth`
parameter is a power of two.

* Update src/axi_fifo_delay_dyn.sv

---------

Co-authored-by: Chen Wu <chenwu@iis.ee.ethz.ch>
* Bump to common_cells v2

* fix(cc): fix cc_ module parameter names for common_cells v2

Update parameter names to match the actual cc_ module API in v2-test:
- cc_delta_counter: Width->WIDTH, StickyOverflow->STICKY_OVERFLOW
- cc_stream_register: data_t->T
- cc_fifo: FallThrough->FALL_THROUGH, Depth->DEPTH, data_t->dtype
- cc_counter: Width->WIDTH

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* hw: Align mux and demux to common_cells v2

* hw: Align params name to common cells v2-test

* hw: Remove test_i ports

* hw: clean up dead test_i ports and use cc_pkg::lzc_mode_e enum for the cc_lzc MODE parameter.

* fixed indentation

* bumped common-cells v2.0.0-beta and tech-cells-generic 0.2.14

* bumped common2 fixes for synopsis tools

* scripts: add target cc_no_deprecated

* deps: Bump `common_cells` to `v2.0.0-beta.2`

---------

Co-authored-by: Lorenzo Leone <lleone@iis.ee.ethz.ch>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Chen Wu <chenwu@iis.ee.ethz.ch>
Co-authored-by: Niccolò Giuliani <niccolo.giuliani44@gmail.com>
@imchenwu
imchenwu force-pushed the fix/issue-341-axi-look-bits branch from 4089aac to f7cae9d Compare July 28, 2026 14:15
@briann-bui

Copy link
Copy Markdown
Author

Hi @imchenwu, thanks for approving this PR. Is there anything else needed before it can be merged?

@imchenwu

Copy link
Copy Markdown
Collaborator

Hi @briann-bui this PR changes the ports and downstream calling modules will require an update, so I would merge it in the next major release, together with the official release of common_cells v2.

@nikgiu
nikgiu changed the base branch from devel to master September 9, 2026 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose AxiLookBits as a parameter in axi_burst_splitter

5 participants