[racl_ctrl,dv] Template the bits of DV that needed it (but no more)#28692
Open
rswarbrick wants to merge 7 commits intolowRISC:masterfrom
Open
[racl_ctrl,dv] Template the bits of DV that needed it (but no more)#28692rswarbrick wants to merge 7 commits intolowRISC:masterfrom
rswarbrick wants to merge 7 commits intolowRISC:masterfrom
Conversation
The motivation here comes from templated blocks. Because their RAL will depend on the template, we'll need to have some extension of dv_base_env_cfg with the appropriate RAL type. That seems reasonable: making a templated subclass of my_package_env_cfg isn't too much cruft. Unfortunately, the cfg object gets created by the test. My initial thought was to template *that* too (grumble, grumble). But that's a terrible idea if you have several different test types, because they would all need templating: you can't have a non-templated class that derives from a templated one. Yuck! This hack should work a little differently. Now, a test can look for a type name that is supplied by the (already templated) tb.sv. Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
This is motivated by blocks that are templated, so the RAL model is itself generated by templated code. It would be nice to avoid having to template everything that contains a RAL. After this commit, you can make a (simple) templated subclass that derives from dv_base_env_cfg. This was a little tricky, because the type parameters have to match! The solution here is to allow a user to use a subclass of RAL_T instead. Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
This type and its name are generated with a template, but we don't care about that in either the vseq or the scoreboard. Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
Now that the scoreboard uses a weaker RAL type in the parameter, we
can now switch the env_cfg to use it too.
This uses the framework developed a few commits ago ("Allow
dv_base_env_cfg with a templated RAL type"). The point is that we'll
now be able to template racl_ctrl_env_cfg (so that it can define
ral_type_name) without having to template its base class.
Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
This way, the code in racl_ctrl_base_test doesn't need to be templated to depend on the (templated) type of the reg block. Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
This doesn't actually do anything yet (because the CFG_T parameter still equals the thing we're wiring through), but the trick is that it will allow us to avoid talking about racl_ctrl_env_cfg (which should depend on templated code) in the non-templated racl_ctrl_base_test. Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
The clever bit of the trick is to define a templated env cfg class, which is a tiny extension of the real one, whose only modification is that it sets ral_type_name to be a templated RAL type. Annoyingly, we didn't actually have a package in which to define the class, so this commit has to do a bit of extra legwork, defining a templated package and renaming the previous version of the package to have a "_base_" in the name. But that's it! The "interesting" part of the commit is the bit where we no longer set ral_type_name in the module that is now called racl_ctrl_base_env_cfg, but do set it in the templated env cfg (racl_ctrl_env_cfg). Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
rswarbrick
force-pushed
the
racl-ctrl-template-dv-properly
branch
from
0d2c6e1 to
6d7df20
Compare
Contributor
Author
|
Force-push rebases this over other PRs that have landed in the meantime. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is following up from feedback from @Razer6 and @davidschrammel, pointing out that my "terribly clever" DV code for
racl_ctrldoesn't actually work if there is non-trivial templating. The problem was that the non-templated part of my DV code depended on names from stuff that was templated (the RAL package).There are seven commits:
[dv] Make creation of CFG_T object more general in dv_base_test[dv] Allow dv_base_env_cfg with a templated RAL type[racl_ctrl,dv] Avoid templated type in vseq / scoreboard[racl_ctrl,dv] Move the RAL type from a param to ral_type_name[racl_ctrl,dv] Avoid type name of racl_ctrl_reg_block in base test[racl_ctrl,dv] Override cfg_type from the tb[racl_ctrl,dv] Expunge the templated RAL name from racl_ctrl DVThe first two are "generic changes" (that are needed by the racl_ctrl-specific stuff that follows).
[dv] Make creation of CFG_T object more general in dv_base_test
The test object instantiates an environment config. Unfortunately, the environment config contains stuff that depends on the RAL (and thus must be templated). The obvious approach would then be to template the test type (since it has the environment config as a parameter). But we might want more than one test type, and templating all of them starts to get a bit ridiculous!
To avoid that, this commit allows the testbench (which needs to be templated already) to pass in the type that should be instantiated for the environment config, overriding the CFG_T parameter in the test class. This type still has to be a subclass of CFG_T (to avoid the code that uses the result getting confused), but it can now depend on something templated, without needing to template the test class that receives it.
[dv] Allow dv_base_env_cfg with a templated RAL type
This is a bit similar to the previous problem, but now we're seeing a slight inflexibility of using parameters in class types. The problem is that you can have a class
Pthat specialises toP1, but there's no way to make a classX1 #(P1)specialise some base classX #(P). SystemVerilog doesn't allow that, and probably shouldn't! There's no way to guarantee the covariance property that you need.The unfortunate thing is that the type we fundamentally wanted was something like
racl_ctrl_env_cfg #(racl_ctrl_reg_block)(where the latter type is templated). This can't derive from somemy_base_class #(dv_base_reg_block).This commit side-steps the issue, by making it so that the
dv_base_env_cfgtype instantiates its "core RAL model" by creating an object with a supplied type name. If nothing is supplied, that type name defaults to match theRAL_Tparameter. For the templated case, this lets us get things working: our templated type can just beracl_ctrl_env_cfg #(dv_base_reg_block)(where theRAL_Tparameter doesn't really do anything) and can set the new class variable (ral_type_name) to point at the templated RAL type. This can derive perfectly happily from a base class that doesn't mention the templated type.Those are the two complicated commits. The remainder of the PR is five commits that move the
racl_ctrltestbench to use this machinery in a way that avoids mentioningracl_ctrl_reg_blockin non-templated code.Phew, this was rather tricky to get right! I'm hoping that the "annoying architectural stuff" is now done, and we'll be able to do something similar for other templated IP blocks with rather less thought!