search

openhwgroup / cv-hpdcache

RTL sources of the High-Performance L1 Dcache (HPDcache) for OpenHW CV cores
Other
58 stars 18 forks source link

Type of `HPDCACHE_VICTIM_SEL` #15

Closed dgptha closed 6 months ago

dgptha commented 7 months ago

https://github.com/openhwgroup/cv-hpdcache/blob/645e4222c3d23fbedb5b0fec1922f72fd692a40a/rtl/src/hpdcache_pkg.sv#L76C1-L82C89

The type of the HPDCACHE_VICTIM_SEL parameter defined in line 82 of rtl/src/hpdcache_pkg.sv should be hpdcache_victim_sel_policy_t and not int unsigned. Having it as int unsigned fails on Questa (version 2019.4) when used in rtl/src/hpdcache_victim_sel.sv line 49 as the types of HPDCACHE_VICTIM_SEL and HPDCACHE_VICTIM_RANDOM do not match (int unsigned vs hpdcache_victim_sel_policy_t).

PS: problem found when using CVA6 v5.0.1.

cfuguet commented 7 months ago

Thank you @dgptha for reporting this issue.

I knew about the inconsistency but I did not knew that this would generate an issue in some tools (or specific versions). I actually use QuestaSim to validate the HPDcache but more recent versions.

Currently the issue is that the type hpdcache_victim_sel_policy_t is defined in the hpdcache_pkg but the actual parameter value comes from the system specific hpdcache_params_pkg. If I define the PARAM_VICTIM_SEL parameter as hpdcache_victim_sel_policy_t in the hpdcache_params_pkg, there is a circular dependency between packages. This is not valid. That is why, I defined it as a integer into the hpdcache_params_pkg.

I will try a different approach to fix the issue.

dgptha commented 7 months ago

Oh, I see the problem you had now. I do not know if I'm not finding the problem you're talking about because I'm using a CVA6 configuration without the hpdcache or because of the Questa version, but for the moment my design without the hpdcache activated can be simulated on my Questa without issues once I do the change mentioned in my previous message. Maybe a solution would be to externalize the types definition in a different file? Only for types like the victim selection policy, not for derived types which depend on parameters (e.g. hpdcache_dir_entry_t).

cfuguet commented 6 months ago

This is now fixed with the new parametrization scheme (commit 00e36514e4fed8684be2f3dc90aea4e5e28908a9).

I close the issue but please feel free to reopen it if you still have the issue.