[VTA] add doc to tsim-example driver and update verilator env variable #3302
Conversation
vta/hardware/chisel/Makefile
Outdated
| @@ -15,6 +15,8 @@ | |||
| # specific language governing permissions and limitations | |||
| # under the License. | |||
|
|
|||
| export VERILATOR_INC_DIR = /usr/local/share/verilator/include | |||
There was a problem hiding this comment.
This should be external to the Makefile IMO; and set by the user. See: https://docs.tvm.ai/install/from_source.html#tvm-package
I think we should make it clear in the instructions that this variable needs to be set by the user, when they install verilator.
There was a problem hiding this comment.
maybe just wrap the export in an ifndef
There was a problem hiding this comment.
I see. I guess we could try to check if the path exists in the makefile then?.
I think having to setup more environment variables is a pain for the user.
There was a problem hiding this comment.
I think that hard coded paths will fail at some point depending on os, or package manager. Given that the user will need to install verilator themselves, this is part of the installation process. We just need to add a line saying to export blah.
There was a problem hiding this comment.
I just added something that test if path exist otherwise it won't build. We will add this to the documentation.
There was a problem hiding this comment.
we should have a cascade of checks: check that the variable exists, if not, set to default path; check that the binary exists, else error out
There was a problem hiding this comment.
Alright, I just added the cascade of checks for binary first then for the path.
There was a problem hiding this comment.
Thanks, it looks good except that we shouldn't use export in a Makefile. Instead can we use a ?= to set if not set?
apache#3302) * add documentation and check for extension * add env variable for verilator include * fix typo * this will test if path exist otherwise it won't buid * check if verilator path and binary is set properly * add ? * remove export * no longer needed
apache#3302) * add documentation and check for extension * add env variable for verilator include * fix typo * this will test if path exist otherwise it won't buid * check if verilator path and binary is set properly * add ? * remove export * no longer needed
apache#3302) * add documentation and check for extension * add env variable for verilator include * fix typo * this will test if path exist otherwise it won't buid * check if verilator path and binary is set properly * add ? * remove export * no longer needed
No description provided.