[hw,ac_range_check,dv] Templify the DV code for ipgen generation

[Razer6]

This PR templifies the core files and then creates template dependencies to the SV files. This is needed to render AC ranges with different names. Otherwise it comes to fusesoc errors due to collisions and naming requirements.

[martin-velay]

If I follow the logic correctly, I think some names should also be templated.

Why do we need to give a different name to it? This is making the code hard to write as the tools to parse the code and help to code are lost.

[Razer6]

I think in DV we can get away with only unquifying the core files, the tb.sv (for the module instantiation) and the bind target (which uses the DUT name) as well as cases where packages from the RTL are used. Let me change that.

[Razer6]

@martin-velay I reworkd the templates. The only independent file where no templating is needed is ac_range_check_dut_cfg.sv. I left that for now. All others have templating requirements.

[rswarbrick]

Hi @Razer6! Is this PR still relevant? (Is there anything to extract or should it just be closed?)

[Razer6]

Hi @Razer6! Is this PR still relevant? (Is there anything to extract or should it just be closed?)

Yes it is. I am just about rebeasing. @davidschrammel is making some experiments internally.

[Razer6]

@rswarbrick @martin-velay Can you please take a look? I properly templified the DV code now to deal with different module name instances. Code is tested downstream with different instances.

[rswarbrick]

This looks sensible to me as an improvement. BUT it's a bit bonkers that we template most of this code! As a follow-up, I think it would probably be better to structure this like I did for racl_ctrl, where some non-templated DV code looks around at runtime to figure out what situation it is in.

That way, you only have to template a couple of files (tb.sv.tpl, for example) and everything else avoids getting duplicated.

[rswarbrick]

Oh! But I notice that a test is failing in CI in a bound-in pwrmgr SVA interface.

CI isn't actually passing (and I only noticed after approving the PR)

[Razer6]

Oh! But I notice that a test is failing in CI in a bound-in pwrmgr SVA interface.

Looking into it. That one seems to be related to the clkmgr change (and is also failing on master). So not related to this one.

[martin-velay]

I have some questions and remarks. But I guess nothing critical.

[martin-velay]

This looks sensible to me as an improvement. BUT it's a bit bonkers that we template most of this code! As a follow-up, I think it would probably be better to structure this like I did for racl_ctrl, where some non-templated DV code looks around at runtime to figure out what situation it is in.

That way, you only have to template a couple of files (tb.sv.tpl, for example) and everything else avoids getting duplicated.

I agree with this comment, the templatization is something we should keep at its minimum. Why don't we do something similar as Rupert did for racl_ctrl?

[Razer6]

I agree with this comment, the templatization is something we should keep at its minimum. Why don't we do something similar as Rupert did for racl_ctrl?

I am very confused. Previously, in this comment, you suggested to templify every reference of ac_range_check?

Furthermore, upon reviewing the code, you will notice that the current implementation is the minimum required for the current structure of the testbench code. In racl_ctrl's DV code, there are some pieces that are not part of the ipgen core, thus there is no templating. Note that racl_ctrl's DV code is very basic at the moment. When expanding that for the full feature set, many of the non-templified code at the moment need become a template again.

[Razer6]

I agree with this comment, the templatization is something we should keep at its minimum. Why don't we do something similar as Rupert did for racl_ctrl?

Actually, the approach that Rupert tried for racl_ctrl does not work. It uses module instance specifc types and packages in the common code. I opened #28648 to clean that up and make the code working with different module instance names.

[rswarbrick]

If that is the case, can you give some more detail about why that doesn't work? Hopefully we can get things working without needing to check in multiple copies of the same file :-)

[rswarbrick]

Furthermore, upon reviewing the code, you will notice that the current implementation is the minimum required for the current structure of the testbench code. In racl_ctrl's DV code, there are some pieces that are not part of the ipgen core, thus there is no templating. Note that racl_ctrl's DV code is very basic at the moment. When expanding that for the full feature set, many of the non-templified code at the moment need become a template again.

Clearly not a discussion for this PR (so I'll shut up after this) but I really hope this is wrong. The whole point of the way I structured things was that the parameterisation can be done "at runtime" by the DV code.

[Razer6]

Let's continue the discussion in #28648 for that :)