Skip to content

Conversation

@AlexChenIC
Copy link

Ref: #3185

This PR removes the unpacked array dimension ([N-1:0], with N=1) from the
rv_tracer packet_type_o port to avoid a scalar enum vs [0:0] array type
mismatch under strict SystemVerilog typing (e.g. DSim).

@MikeOpenHWGroup
Copy link
Member

Hi @JeanRochCoulon, can you have a look at this one?

@numero-744
Copy link

Hi,

This PR removes the [N-1:0] dimension unconditionally, but N is not always equal to 1. Also, notice that the signal is indexed in the same file you modified so it is expected to be an array, not a single enum value.

I guess the error/warning you get comes from the instantiation (in corev_apu/fpga/src/ariane_xilinx.sv or corev_apu/tb/ariane_testharness.sv for instance), where the packet_type signal declaration should contain an [N-1:0] dimension.

@MaxCThales
Copy link
Contributor

Hi,
Indeed, @numero-744 summarized the issue well regarding the permanent removal of [N-1:0]. The current encoder is derived from https://github.com/pulp-platform/rv_tracer
and is designed to handle N “blocks.” However, in our modified version, we chose to focus on a block-by-block implementation (with N = 1). With the idea of eventually being able to use the encoder as-is (as a submodule), we wanted to preserve a similar structure, even if it is not currently used (i.e., handling the N > 1 cases).

Removing the [N-1:0] dimension would therefore break this indexed logic, e.g., at line 747 or line 853 of rv_tracer.sv

@MikeOpenHWGroup
Copy link
Member

@numero-744 and @MaxCThales, you are correct about what this PR does. However as pointed out in #3185, the code in question is not legal SystemVerilog, so we should fix it.

@numero-744
Copy link

Have you tried the suggestion from my previous message? (Adding an [N-1:0] dimension to the packet_type signal at the instantiation place in corev_apu/fpga/src/ariane_xilinx.sv and corev_apu/tb/ariane_testharness.sv.)

This new dimension can be fixed as [0:0] as the instantiation specifies .N(1). Then, it can be indexed as packet_type[0] where it is read.

To reduce the number of changed line, one could use an intermediary signal as show in the pseudocode below:

packet_type_t [0:0] packet_types;
packet_type_t packet_type;

...
.N(1),
...
.packet_type_o (packet_types),
...

assign packet_type = packet_types[0];

  Add [0:0] unpacked array dimension to packet_type signal declarations
  in ariane_xilinx.sv and ariane_testharness.sv to match the rv_tracer
  output port type (it_packet_type_e [N-1:0] with N=1).

  This replaces the previous approach of removing [N-1:0] from
  rv_tracer.sv, which would have broken indexed logic for N>1.

  Ref: openhwgroup#3185
@AlexChenIC AlexChenIC force-pushed the fix/3185-packet_type_o branch from c007d89 to 5c508ef Compare February 11, 2026 14:26
@AlexChenIC
Copy link
Author

Thanks @numero-744, @MaxCThales, and @MikeOpenHWGroup for the thorough review.

You are right that removing [N-1:0] from rv_tracer.sv would break the indexed logic at lines 637/747/853 when N > 1. I apologize for the oversight in my original approach.

I have reworked the PR following @numero-744's suggestion, the fix is now applied, and keeping rv_tracer.sv file unchanged:
Changes (2 files, 1 line each):

  • corev_apu/tb/ariane_testharness.sv:699packet_type declared as
    te_pkg::it_packet_type_e [0:0]
  • corev_apu/fpga/src/ariane_xilinx.sv:840 — same change

I have verified it with the DSIM 2026 simulator, and the previous port type compatibility issue in #3185 has been resolved.

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.

4 participants