TheSystemDevelopmentKit / rtl

Package for rtl (i.e. Verilog and VHDL ) simulation control
Other
1 stars 2 forks source link

Refactoring simulations directory #15

Closed ojarvin closed 3 years ago

ojarvin commented 3 years ago

Hi,

This PR aims to refactor the simulations directory and the sub-directories. The previously hard-coded file path to the interactive control file is now a property. The do-file changes are handled by the rtl module as follows:

  1. Check if deprecated do-file exists in ./Simulations/rtlsim/dofile.do
    • If yes, copy it to the current simulation directory and print a message with instructions to delete the obsolete file and define the contents in the testbench.
  2. If the do-file contents are given in the testbench, create the do-file under the current simulation directory.

I think this shouldn't break any existing testbenches (unless other hard-coded references to Simulations exist).

To test, pull refactor_simulations branch in rtl and thesdk, and optionally in inverter, which contains an example of defining the do-file contents in the testbench.

ojarvin commented 3 years ago

True, the location of a possible external do-file is not decided. As of now, the path can be overridden and any file path can be specified. If a standard expected location is needed, it could be the same directory as the source HDL-file in my opinion. I think the simulation directory should be used only for temporary and sydekick-generated files.

As far as precedence:

  1. Deprecated do-file exists -> use that
  2. External do-file (manually set interactive_controlfile) exists -> use that
  3. interactive_control_contents exists -> use that

I don't know if the preferred use is to always provide the contents in python or in an external file. This should be decided to possibly flip 2 and 3. For now, I think case 3 is the expected behavior.

Also, it seems you are in another branch in thesdk. If you want to test the refactored simpath location, pull refactor_simlations in thesdk as well (unless I screwed something up).

chiplet commented 3 years ago

Looks like I was using v1.7_RC in the previous tests. Seems to work fine with refactor_simulations as well.

The precedence seems to work the way you described based on my own testing. I'd still advocate printing an error message that indicates which source for the dofile is used if both sources are defined. It could make more sense to filp 2 and 3 in precedence if we want to have the ability to programmatically override dofile contents if the file location is configured.

As for the default location, I don't think the default dofile path should be hardcoded in thesdk, but rather we should provide an example configuration in inverter. The dofiles can get pretty large with larger designs (we have a 107 line dofile in ACoreChip) so IMO it would be cleaner to keep it in a separate file.

ojarvin commented 3 years ago

That sounds reasonable. Since you guys are the ones using this do-file and more extensive RTL verification, I will let you decide what is the most comfortable and flexible approach here.

I think external do-file is good for the reasons you mentioned (it can be extensive). However, the testbench-generated file is good if the contents depend on the simulation context and testbench values.

So if I understood correctly, the precedence should be something like:

  1. Obsolete stuff found, use it and instruct user (done).
  2. Contents defined in testbench through interactive_control_contents -> write a temporary do-file to simulation directory and use that.
  3. interactive_control_contents empty, look for stuff in interactive_controlfile, for which there is a default path that can be overridden.
chiplet commented 3 years ago

Yep, and for the final case that both interactive_controlfile and interactive_control_contents are defined it should output a warning saying that interactive_control_contents is used instead of interactive_controlfile to avoid a situation where someone would be trying to use the controlfile and thesydekick would be silently overwriting that file with contents defined somewhere in the codebase.

ojarvin commented 3 years ago

Any preference for the default do-file location? IMO it should not be under simulations, but rather under ./sv/ and ./vhdl for example, since these are some type of source directories. A new directory can also be created for this purpose, but it might not be justified unless there can be several do-files that need to exist simultaneously.

chiplet commented 3 years ago

IMO it doesn't really fit in ./sv/ or ./vhdl/ since it should be source language agnostic. I don't think there's a need to create more than one dofile anyways so you could just leave it at the entity root (unless @mkosunen has some strong opinion about this). :shrug:

ojarvin commented 3 years ago

Alright I think this is done now. The default location of the do-file is the entity root ./dofile.do. A warning is issued when this file exists and interactive_control_contents is used instead.

chiplet commented 3 years ago

Great! Seems to fully work as expected now! :+1:

mkosunen commented 3 years ago

IMO the location for the modelsim dofile should be \<entity>/interactive_control_files/modelsim/dofile.do

Preferably no floating individual files in main directory.

ojarvin commented 3 years ago

Alright, I set the default path to <entity>/interactive_control_files/modelsim/dofile.do.