olofk / fusesoc

Package manager and build abstraction tool for FPGA/ASIC development
BSD 2-Clause "Simplified" License
1.16k stars 242 forks source link

Strings are not quoted for Xilinx ISE/xst "-generics" option #175

Closed ThomasHornschuh closed 6 years ago

ThomasHornschuh commented 6 years ago

Hi, I encountered a problem when using the [parameters] section of fusesoc together with ISE:

fussoc parameters of type "file" or "str" passed to the -generics option of xst must be quoted with ".

The generated tcl file should for example look like: project set "Generics, Parameters" "RamFileName=\"compiled_code/monitor.hex\"" -process "Synthesize - XST" Currently the \" are missing, as temporary workaround I have added them to the parameter itself [parameter RamFileName] datatype=file default=\"compiled_code/monitor.hex\" description=Initial boot RAM contents (in hex) paramtype=vlogparam scope=public But this will not work with isim which expects the parameter without quotes :-(

In addition I'm not sure about expected semantics of "file" parameters: I would assume that the path should be expanded to a path relative to the build root when passed to the tools, like it is done for the project files.

olofk commented 6 years ago

Thanks for reporting. Those string parameters have given me a lot of headache. I need to check that and ideally write some unit tests.

Regarding the file parameter expansion, it will expand environment variables, ~ and then convert to an absolute path before it is passed to the EDA tool if you pass it on the command line.

If you set default values in the .core file, it will be relative to the work root (e.g. build/system_name/sim-isim). Currently, I've been using hacks like setting the value to ../src/system_name/path/to/file as the default value. This is of course a horrible solution and I have been planning to make it easier to refer to files in the exported source directories.

But, funny thing is that I'm just in this moment working on another feature that will likely be of great help here. I'm implementing a new parameter, copyto, that you can set on files in filesets to copy them to a location relative the work root. In your case that would mean that you specify your file with files = compiled_code/monitor.hex[copyto=monitor.hex] to have it appear directly under work root. With that, you can just set default value to monitor.hex. Hope to have this done in the next few days

olofk commented 6 years ago

copyto is implemented now. You can find it on the copyto branch. Need some doc and tests before I push it to master

olofk commented 6 years ago

...and pushed to master. Will investigate the quoting problems soon

ThomasHornschuh commented 6 years ago

Thanks, I will check this out in the next days. I had already the idea of a similar concept of copyto. What is the semantics of copyto in case of the no-export option? Theoretically it would be better to execute copyto also in case of no-export. I started to run in problems with my .hex file when trying to make my project work with and without no-export. To my surprise I discovered in the meantime that the Xilinx tools search for a file not only relative to their build working directory, they also search relative to the location of the hdl source file opening the file. This at least applies to xst and isim when using vhdl file_open. The no-export option is very usufull when using fusesoc for interactive development work. What I would wish is a „-start-eda-tool“ option. Currently with ISE I use the -setup option, edit the tcl file (remove the last line) then run the tcl in tclsh and finally load the generated project into ISE. It would be helpful to automate this step. Of course this is specific to ISE, I have not tested yet with Vivado. Finally I will work with both, because my current project targets SPARTAN-6 and Artix devices.

But generally FuseSoc is great, it really helps to target hdl code for different tools and also publish a project in a reproducable manner to e.g. Github. Many thanks for your work

olofk commented 6 years ago

First of all. Thanks for letting me know that you find the tool helpful!

copyto works also for no-export mode (that was one of the reasons I chose copyto instead of moveto). That is also useful for example when you use Vivado xci files since they tend to leave a lot of temp files in the directory where the xci lives, so it can pollute source repositores.

Hmm.. I haven't checked how file_open behaves. Do you mean that it search both in the working directory and the source file directory, or only in the latter? If it's the latter, then the copyto functionality still isn't helping out all that much I guess, but you can still add a pre_build_script to copy the file manually before building.

Regarding the start-eda-tool option, I see what you mean. That would be useful also for debugging with simulators like modelsim. I will start working towards this. As a first step I think that we could generate two tcl files. One that contains everything but the last line, and then another file that just sources the first file and start running. That way you have an easier option to run the first file yourself and open the tool. Need to dig into that a bit as I haven't used ISE tcl mode very much.

olofk commented 6 years ago

Could you please check if the patch applied to the isequotes branch works for you? That should remove the need to manually escape strings for ISE

olofk commented 6 years ago

If this works for you, please close the issue and we can track your request for interactive mode in #178

ThomasHornschuh commented 6 years ago

Thanks, I will hopefully have time to check it at the weekend.

ThomasHornschuh commented 6 years ago

Works ! Many thanks.