Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relative paths in run_modelsim.py and other changes to make Modelsim work on non-Utah machines #247

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

nachiket
Copy link
Contributor

@nachiket nachiket commented Feb 20, 2021


name: Pull request
about: Push a change to this project

Motivate of the pull request

  • To address an existing issue. If so, please provide a link to the issue. Standard-cell flow #225 (later comments on vvp hanging)
  • Breaking new feature. If so, please decribe details in the description part.

Describe the technical details

  • Modelsim simulation backend isn’t operational.
  1. Removed the end_flow_with_test line from end of task.conf file
  2. Modified run_modelsim.py to use relative paths for task_dir instead of assuming OPENFPGA_ROOT contains all task files
  3. Tweaked openfpga shell script to adjust the simulation TCL file path generated by the modelsim backend.
  4. Need to add —modelsim_ini runtime flag to customize modelsim path to your own machines installation. This should be explained in the documentation.

Which part of the code base require a change

In general, modification on existing submodules are not acceptable. You should push changes to upstream.

  • VPR
  • OpenFPGA libraries
  • FPGA-Verilog
  • FPGA-Bitstream
  • FPGA-SDC
  • FPGA-SPICE
  • Flow scripts
  • Architecture library
  • Cell library

Checklist of the pull request

  • Require code changes.
  • Require new tests to be added
  • Require an update on documentation

Impact of the pull request

  • Require a change on Quality of Results (QoR)
  • Break back-compatibility. If so, please list who may be influenced.

@ganeshgore
Copy link
Collaborator

LGTM, these features are not tested on CI. @tangxifan You can merge if everything is good.

@nachiket nachiket changed the title Relative paths in run_modelsim.py Relative paths in run_modelsim.py and other changes to make Modelsim actually work Feb 20, 2021
@nachiket nachiket changed the title Relative paths in run_modelsim.py and other changes to make Modelsim actually work Relative paths in run_modelsim.py and other changes to make Modelsim work on non-Utah machines Feb 20, 2021
@nachiket
Copy link
Contributor Author

nachiket commented Feb 20, 2021 via email

@tangxifan
Copy link
Collaborator

You should check why the Modelsim tcl files were being generated in a sub folder SimulationDeck for the write_verilog_testbench command. I couldn’t figure out how to make run_modelsim pick them up from that sub folder without hard coding in the paths so I just modified the openfpga shell script instead to drop that folder path.

On Feb 19, 2021, at 10:05 PM, ganeshgore @.***> wrote:  LGTM, these features are not tested on CI. @tangxifan You can merge if everything is good. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

That is a valid solution in my opinion.

@@ -55,7 +55,7 @@ write_fabric_verilog --file ./SRC --explicit_port_mapping --include_timing --pri
# - Enable top-level testbench which is a full verification including programming circuit and core logic of FPGA
# - Enable pre-configured top-level testbench which is a fast verification skipping programming phase
# - Simulation ini file is optional and is needed only when you need to interface different HDL simulators using openfpga flow-run scripts
write_verilog_testbench --file ./SRC --reference_benchmark_file_path ${REFERENCE_VERILOG_TESTBENCH} --print_top_testbench --print_preconfig_top_testbench --print_simulation_ini ./SimulationDeck/simulation_deck.ini --include_signal_init --support_icarus_simulator --explicit_port_mapping
write_verilog_testbench --file ./SRC --reference_benchmark_file_path ${REFERENCE_VERILOG_TESTBENCH} --print_top_testbench --print_preconfig_top_testbench --print_simulation_ini ./simulation_deck.ini --include_signal_init --explicit_port_mapping
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you avoid to modifying existing script templates? We can create a new template script with this feature.

@nachiket
Copy link
Contributor Author

nachiket commented Feb 20, 2021 via email

@tangxifan
Copy link
Collaborator

I agree. Is there a way to use run_modelsim.py with this script unmodified? Id rather not have to delete the simulationdeck folder path at all but can’t see how else to use the Modelsim flow.

On Feb 19, 2021, at 10:44 PM, tangxifan @.***> wrote:  @tangxifan requested changes on this pull request. In openfpga_flow/openfpga_shell_scripts/fix_device_route_chan_width_example_script.openfpga: > @@ -55,7 +55,7 @@ write_fabric_verilog --file ./SRC --explicit_port_mapping --include_timing --pri # - Enable top-level testbench which is a full verification including programming circuit and core logic of FPGA # - Enable pre-configured top-level testbench which is a fast verification skipping programming phase # - Simulation ini file is optional and is needed only when you need to interface different HDL simulators using openfpga flow-run scripts -write_verilog_testbench --file ./SRC --reference_benchmark_file_path ${REFERENCE_VERILOG_TESTBENCH} --print_top_testbench --print_preconfig_top_testbench --print_simulation_ini ./SimulationDeck/simulation_deck.ini --include_signal_init --support_icarus_simulator --explicit_port_mapping +write_verilog_testbench --file ./SRC --reference_benchmark_file_path ${REFERENCE_VERILOG_TESTBENCH} --print_top_testbench --print_preconfig_top_testbench --print_simulation_ini ./simulation_deck.ini --include_signal_init --explicit_port_mapping Can you avoid to modifying existing script templates? We can create a new template script with this feature. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

@nachiket You can define variables in the openfpga shell script and then define the value of the variables in task configuration file. For example, you can define ${OPENFPGA_SIMULATION_DECK_INI_FILE} in the openfpga shell script.
We have an example here.

@nachiket
Copy link
Contributor Author

nachiket commented Feb 22, 2021 via email

@tangxifan
Copy link
Collaborator

@nachiket The example shows the use of variable ${OPENFPGA_VPR_ROUTE_CHAN_WIDTH}. You can create new variable for your own usage.

In the openfpga shell script, you can define a variable

vpr ${VPR_ARCH_FILE} ${VPR_TESTBENCH_BLIF} --clock_modeling route --route_chan_width ${OPENFPGA_VPR_ROUTE_CHAN_WIDTH} --device ${OPENFPGA_VPR_DEVICE_LAYOUT}

In the task configuration file, you can use it by

According to what I know, the variable is not visible to the run_modelsim.py script.

I am a bit confused by what you need exactly here. If you can provide more details, I can help.

@nachiket
Copy link
Contributor Author

nachiket commented Feb 22, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants