Add multicast support to XBAR #398

colluca · 2025-09-22T12:15:36Z

This PR adds multicast support to the AXI XBAR, such that write requests from a master can be multicast to multiple slaves.

To support multicast with low area overhead, we adopt the mask-based encoding proposed in https://arxiv.org/pdf/2502.19215.

In detail, this PR implements the following changes:

Adds the any_outstanding_trx_o port to the axi_demux_id_counters module.
The axi_mcast_demux_simple implements a superset of the functionality of the axi_demux_simple, including all multicast capabilities, which require a few more signals on the interface, namely mst_is_mcast_o and mst_aw_commit_o. So axi_demux_simple is rewritten as a wrapper around axi_mcast_demux_simple, tying off unused ports and parameters.
The same is done for the axi_mcast_mux and axi_mux modules, the axi_mcast_xbar_unmuxed and axi_xbar_unmuxed modules, and the axi_mcast_xbar and axi_xbar modules.
The axi_demux_id_counters module is moved to its own file, which is anyways good practice, and it would otherwise not be clear whether it should now belong to axi_demux_simple.sv or axi_mcast_demux_simple.sv.
Placing the spill registers downstream of the address decoders requires registering the ax_select signals from the address decoders. To eliminate this overhead (in the case of multicast it would even increase due to additional output signals from the decoders), we move the address decoders downstream of the spill registers in theaxi_mcast_demux_mapped module. The _mapped prefix is to identify a demux whose slaves are memory-mapped, i.e. that uses address decoders to generate the ax_select signals. Since several modules use the demux with a non-address-based ax_select signal generation, the axi_demux module preserves the same interface and functionality as before, and does not wrap the axi_mcast_demux_mapped module, unlike the other unicast and multicast IP pairs.

Auxiliary changes:

The axi_demux_id_counters is moved to its own file, as good practice.
Update IP levels in Bender.yml after IP refactoring (including fixes to some dependencies that were not previously tracked, e.g. axi_demux_simple->axi_burst_splitter_gran, and others)

Additional notes:

The definition of default port should be adapted for multicast requests: "a port the request should be (also) forwarded to if any of the (multiple) addresses carried by the multicast request falls outside of the union of all the ports' address maps". For unicast requests it's cheap to determine if the address falls within the union of the address rules, by simply iterating over the address rules. For multicast requests this would be more complicated/expensive, so we opted for separately providing the union through the default master port rule (as an example, see https://github.com/pulp-platform/snitch_cluster/blob/6248ae825d821dca81c7b9c0f3cf3fd59c0ecdaa/hw/snitch_cluster/src/snitch_cluster.sv#L775-L782).
Details on how the default master port rule is used are in the multiaddr_decode module (see also multiaddr_decode: Add default port logic common_cells#235).

micprog

This PR is quite cumbersome to review, as most of the 5k line changes are code movement changes, and the code changes are not directly visible in the interface here. For some files, I added the corresponding git commands I used to check the changes. It might make sense to move some of these changes not related to multicast to dedicated PRs, simplifying the review and traceability of changes.

A further complication in reviewing this PR is the non-standard feature, as multicast is not supported in any available AXI specification. Please add documentation in the corresponding README files for what multicast is supported, how it works, and how it is intended to be used.

Also, given that this is not part of the spec, it might make sense to put this in a separate project, keeping this axi repository to what is available in the spec, but we can have that discussion offline.

Primarily, I was under the impression that this PR does not change the existing functionality when multicast is disabled or the existing modules are used, but this does not seem to be the case!

Along with comments placed to help understanding, there are a few other items that need to be addressed. Please see below.

scripts/run_vsim.sh

micprog · 2026-01-15T14:54:36Z

Bender.yml


 dependencies:
-  common_cells: { git: "https://github.com/pulp-platform/common_cells.git", version: 1.37.0 }
+  common_cells:        { git: "https://github.com/pulp-platform/common_cells.git", rev: "multicast-xbar" }


Please update this to use a proper version, not a branch name.

Unfortunately this is waiting on pulp-platform/common_cells#235 (>1y...).

It's been marked as v2, so it can't be accelerated much, but this should be hopefully sorted out soon.

I would make it a priority to only use mainline releases for integration in main. This significantly improves compatibility in projects when there are multiple references to common_cells (and also axi). Therefore, we should wait for this to be integrated before merging this PR.

Bender.yml

micprog · 2026-01-15T14:57:24Z

src/axi_demux_id_counters.sv

+  // outstanding transactions
+  output logic                 any_outstanding_trx_o


Just so I understand correctly, the only adjustment you made to this module is adding this port, otherwise it is copied from axi_demux_simple.sv.

git diff master:src/axi_demux_simple.sv multicast:src/axi_demux_id_counters.sv

Correct yes, I've added it to the PR description.

Thanks! Maybe it makes sense to split this off as an individual PR for traceability...

src/axi_demux_simple.sv

micprog · 2026-01-15T17:50:58Z

src/axi_mcast_demux_mapped.sv

+);
+
+  // Account for additional error slave
+  localparam int unsigned NoMstPortsExt = NoMstPorts + 1;


Is an error slave needed when a default port is enabled?

I would say it isn't, since when the default port is enabled the address decoder does not produce a decoding error.

I just kept the previous behaviour here, and I think it also makes sense, since the default port enable is a dynamic signal and not a compile-time parameter.

micprog · 2026-01-15T17:56:10Z

src/axi_mcast_demux_mapped.sv

+  spill_register #(
+    .T       ( aw_chan_t  ),
+    .Bypass  ( ~SpillAw   )
+  ) i_aw_spill_reg (
+    .clk_i,
+    .rst_ni,
+    .valid_i ( slv_req_i.aw_valid    ),
+    .ready_o ( slv_resp_o.aw_ready   ),
+    .data_i  ( slv_req_i.aw          ),
+    .valid_o ( slv_req_cut.aw_valid  ),
+    .ready_i ( slv_resp_cut.aw_ready ),
+    .data_o  ( slv_req_cut.aw        )
+  );


This module has a different timing behavior than the existing implementation, as outlined in the PR description. While this simplifies some things and reduces the number of registers, the address decoding can add quite a bit of timing overhead and can now no longer be pipelined out of the demux path when using any xbar configuration. Do you have an analysis of this timing impact before we integrate this change?

That is a relevant observation.

I did not do a complete AT plot analysis, and I only evaluated this as an incremental change over the multicast design (not the original unicast design), but I did do some synthesis trials at 1 GHz in GF12 quite a long time ago now.

When constrained at 1 GHz, there was no significant frequency degradation (critical path went from 0.996 to 0.993 ns, most likely tool noise). The levels of logic reported by Fusion Compiler, which increased from 30 to 32, indicate that there might be a small timing degradation if pushed to higher frequencies.

But, as mentioned, we also save spill register area, so I expect that the AT plots of the unicast design with address decoders upstream of the spill registers vs. downstream would probably look pretty much the same.

I forwarded a few old WRs where I reported these results to you for further details.

I'm worried about increasing the levels of logic. In a separate paper I did, the decoding logic accounted for quite a bit of timing overhead. Creating a configuration like the one you have now is fairly straightforward with the previous IPs (i.e., simply adding an axi_cut module before each xbar port), but the previous implementation is now almost impossible to access. So while your configuration might not change, others might be impacted, which worries me.

micprog · 2026-01-15T18:00:52Z

src/axi_mcast_demux_mapped.sv

+      dec_aw_addr = '0;
+      dec_aw_mask = '0;
+
+      if (slv_req_cut.aw.user.collective_mask == '0) begin


I'm not sure this will work for all tools if the custom user field is not present in the passed struct...

So, the syntax in the following example is a bit different, but still uses some fields which are conditionally present in a struct type:
https://github.com/pulp-platform/snitch_cluster/blob/6248ae825d821dca81c7b9c0f3cf3fd59c0ecdaa/hw/snitch_cluster/src/snitch_cluster.sv#L1409-L1430
https://github.com/pulp-platform/snitch_cluster/blob/6248ae825d821dca81c7b9c0f3cf3fd59c0ecdaa/hw/snitch_cluster/src/snitch_cluster_pkg.sv.tpl#L114-L125

In Snitch, this code is tested and works correctly with QuestaSim, VCS, Verilator, Fusion Compiler and Yosys.

The important thing AFAIK is that the code is guarded by a generate construct, which is the case also here: NoMulticastPorts > 0.

Ok, I see the code is guarded by a generate (line 257), which I missed previously. The second generate (line 269) is confusing in this regard.

src/axi_mcast_demux_simple.sv

micprog · 2026-01-15T18:13:38Z

src/axi_mcast_demux_simple.sv

+      else $fatal(1, "slv_ar_select_i illegal while ar_valid.");
+    w_underflow: assert property( @(posedge clk_i)
+        ((w_open == '0) && (w_cnt_up ^ w_cnt_down) |-> !w_cnt_down)) else
+        $fatal(1, "W counter underflowed!");


Are the following assertions that you removed fully no longer applicable, or is there a version we could keep?

- aw_select: assume property( @(posedge clk_i) (slv_req_i.aw_valid |-> - (slv_aw_select_i < NoMstPorts))) else - $fatal(1, "slv_aw_select_i is %d: AW has selected a slave that is not defined.\ - NoMstPorts: %d", slv_aw_select_i, NoMstPorts); - internal_aw_select: assert property( @(posedge clk_i) - (aw_valid |-> slv_aw_select_i < NoMstPorts)) - else $fatal(1, "slv_aw_select_i illegal while aw_valid.");

Yes, they are no longer applicable.

When aw_select is an index then it makes sense to check that it's within bounds. For example, if NoMstPorts is 3 and aw_select is a 2-bit signal, it could encode 3 which is out of bounds.

But when aw_select is a bitmask it no longer makes sense, since the number of bits is equal to the number of NoMstPorts so there is no risk of encoding illegal values.

Then maybe these assertions should stick around in the IP still using index bits, namely axi_demux_simple?

To be improved: - 50% multicast AW throughput - Potential for combinational loops in mux

Multiple outstanding multicast transactions are allowed only if the slaves selected by these transactions are the same.

--------- Co-authored-by: Luca Colagrande <luca.colagrande3@gmail.com> Co-authored-by: Raphael <raroth@student.ethz.ch>

colluca added 3 commits September 17, 2025 15:07

axi_xbar: Correct signal names in waveform file

9cd8157

docs: Correct typos

bab24f7

axi_mcast_xbar: Create copies of original IPs as starting point

d416ee9

colluca force-pushed the multicast branch 5 times, most recently from d7b7105 to 8e04779 Compare September 26, 2025 17:48

colluca marked this pull request as ready for review October 6, 2025 06:58

fischeti mentioned this pull request Dec 9, 2025

hw: Support custom axi_xbar config structs pulp-platform/cheshire#254

Merged

micprog requested changes Jan 15, 2026

View reviewed changes

colluca and others added 19 commits January 23, 2026 11:30

axi_mcast_xbar: Add basic multicast logic

1700789

axi_mcast_xbar: Correct deadlock condition

28be1a7

axi_mcast_xbar: Cut valid->ready->commit combinational path

e257da5

To be improved: - 50% multicast AW throughput - Potential for combinational loops in mux

axi_mcast_demux: Retrieve AW select index form from the mask

ac562d5

axi_mcast_xbar: Move AW address decoders after spill registers in demux

9bda7e8

axi_mcast_demux: Merge B responses appropriately

c1a86d5

axi_mcast_xbar: Allow both regular and mask-based address rules

9ccf8fa

axi_mcast_xbar: Allow multiple outstanding multicast transactions

c454215

Multiple outstanding multicast transactions are allowed only if the slaves selected by these transactions are the same.

axi_mcast_xbar: Filter multicast requests to unconnected slaves

dfac76f

axi_mcast_xbar: Add to CI

6848c5b

Improved route decoding for collective operations (#1)

7fecbf1

--------- Co-authored-by: Luca Colagrande <luca.colagrande3@gmail.com> Co-authored-by: Raphael <raroth@student.ethz.ch>

Rebase onto latest master

93aef89

Continue rebase, make tests pass again

3c3b6c0

Implement unicast XBAR IPs from multicast IPs

a32d7f2

Restrict UniqueIds to 0 when multicast is enabled

0dc633b

Rename axi_mcast_demux to axi_mcast_demux_mapped

ad78d75

Minor cleanup

4e06079

Add assertions on multicast rule conversion

3e2f497

Address final TODOs

6258b62

colluca added 6 commits January 23, 2026 11:30

axi_mcast_mux: Remove dependency of valid on ready

3f0be77

axi_mcast_demux_mapped: Improve readability of assertions

6b51b99

axi_mcast_demux_mapped: Allow multicast XBAR with NoMulticastRules == 0

bbac87b

axi_mcast_demux_mapped: Fix VCS compilation when NoMulticastPorts==0

8154eb7

Bender.yml: Fix IP sorting by levels

6a9c287

tb_axi_xbar_pkg: Remove legacy TODO

07f3cb4

colluca force-pushed the multicast branch from 06410c3 to 07f3cb4 Compare January 25, 2026 16:05

		// outstanding transactions
		output logic any_outstanding_trx_o

Add multicast support to XBAR #398

Are you sure you want to change the base?

Add multicast support to XBAR #398

Uh oh!

Conversation

colluca commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

micprog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

colluca Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

colluca commented Sep 22, 2025 •

edited

Loading

colluca Jan 25, 2026 •

edited

Loading