hpmevent_cfg_t fails synthesis for hpm_num=0

[mikaelsky]

Describe the bug
The definition of the array results in a 3 to 2 index when hpm_num is set to 0 when hpm counters are disabled.

The fix is to not subtract 1, which should be the default case I believe.

  -- hpm event configuration CSRs --
--  type hpmevent_cfg_t is array (3 to (hpm_num_c+3)-1) of std_ulogic_vector(hpmcnt_event_size_c-1 downto 0);
  type hpmevent_cfg_t is array (3 to (hpm_num_c+3)) of std_ulogic_vector(hpmcnt_event_size_c-1 downto 0);

Same issue is part of the generate for loop, except the generate statement is disabled in this particular corner case.

To Reproduce
Found during synthesis where VHDL elab fails

Expected behavior
NA

Screenshots
NA

Environment:

• OS RHEL7
• GCC Version 13.2
• Libraries used newlib

Hardware:

• Hardware version (1.9.3.0) but bug patched

Additional context
NA

[stnolting]

There is some redundancy in the HPM's ISA extension configuration:

• if CPU_EXTENSION_RISCV_Zihpm is false the entire ISA extension is discarded and no HPMs are implemented
• if HPM_NUM_CNTS is zero the ISA extension is available but not actual HPM counters are implemented

The HDL codes does not check this at all points.

The fix is to not subtract 1, which should be the default case I believe.

This would fix the issue. But I think it would be cleaner to add further if/else and if-generate statements to catch the corner case

• CPU_EXTENSION_RISCV_Zihpm = true, HPM_NUM_CNTS = 0
• CPU_EXTENSION_RISCV_Zihpm = false, HPM_NUM_CNTS = 0

I'll try to come up with a fix for this 😉

Thanks for finding this design flaw!

[mikaelsky]

No worries will run the fix through proto-synthesis Monday :) This is all part of "maturing" the core, finding all the little things :)

@stnolting the issue this cause some odd behavior from synthesis. We learned a few things.

1. After removing component declarations (yeah :) ) synthesis warns about missing entities if the compile order isn't strictly in dependency order. Simulations are more... meh about the compile order.
2. After reordering the files to be read in according to dependency (as one had to do back in 1999) the root cause of the elaboration error finally popped out is the false path HPM instance.

So for synthesis I, at least, would recommend to just keep the files in dependency order. I know FPGA tools can auto-reorder, and tools should do that in general. Buuut if something like this pops out, some tools (no one shamed here) really go off the rails.

[stnolting]

No worries will run the fix through proto-synthesis Monday :) This is all part of "maturing" the core, finding all the little things :)

👍 🎉

So for synthesis I, at least, would recommend to just keep the files in dependency order. I know FPGA tools can auto-reorder, and tools should do that in general. Buuut if something like this pops out, some tools (no one shamed here) really go off the rails.

This is really a problem... Simulators like GHDL and Questa Sim are quite good at detecting the right compile order by themselves. Synthesis is a little bit more complex. Quartus has absolutely no problems, Xilinx is sometimes a little bit picky and Lattice is... well... you really have to hold its hand all the time 😅

I have no idea how good/bad ASIC tools like Cadence can handle this.

I like the cleaner concept of entity instantiation, but maybe we should go back to component instantiation to keep the setup burden low (for beginners)??? 🤔

[mikaelsky]

I would keep using entity instantiation, its the way to go if I'm honest.

When I coded VHDL for $$ from '99 to '07 we never used components and no tools had any issues with. The one caveat was we typically wrote our own dependency checker script, as make couldn't figure out vhdl dependency at the time. This is what the simulators basically do today, read in the files -> pre-compile -> determine dependency -> re-order in memory -> re-compile.

I will withhold any comments on Lattice :) Suffice to say that FPGA tools, from all vendors, have... historically been... not amazing. E.g. to build with vhdl+verilog I'm using Synplify as Vivado just hangs.

With ASIC tools I would let the implementer deal with it for now. In ASIC land we use .f files for reading in files both simulation and synthesis. Everyone basically writes up their own <please read in these files "list"> files, so having to manually reorder isn't an issue as right. Any ASIC person that wants to use the core would have to type up their own .f file anyways. We could help by just adding a set of .f files in the rtl directory that had the recommended compile order, which could then be used.

The use of .f files allows ASIC teams to utilize IP based flows where design blocks, like the core, comes with a .f file that users read in with their readhdl commands. That .f file has the list of files and potentially pointer to other .f files.
This makes source code super moveable as everything is relative to the IP directory and not an absolute path.

[stnolting]

I would keep using entity instantiation, its the way to go if I'm honest.

Yeah, let's keep it then ;)

I will withhold any comments on Lattice :)

Radiant comes with build-in Synplify Pro, which a quite amazing tool I have to say.

The use of .f files allows ASIC teams to utilize IP based flows where design blocks, like the core, comes with a .f file that users read in with their readhdl commands. That .f file has the list of files and potentially pointer to other .f files.
This makes source code super moveable as everything is relative to the IP directory and not an absolute path.

I really like this idea! We should definitely add this to the rtl folder.