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

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

Closed
ThomasHornschuh opened this Issue Oct 8, 2017 · 9 comments

Comments

Projects
None yet
2 participants
@ThomasHornschuh
Contributor

ThomasHornschuh commented Oct 8, 2017

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

This comment has been minimized.

Show comment
Hide comment
@olofk

olofk Oct 8, 2017

Owner

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

Owner

olofk commented Oct 8, 2017

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

This comment has been minimized.

Show comment
Hide comment
@olofk

olofk Oct 8, 2017

Owner

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

Owner

olofk commented Oct 8, 2017

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

@olofk

This comment has been minimized.

Show comment
Hide comment
@olofk

olofk Oct 9, 2017

Owner

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

Owner

olofk commented Oct 9, 2017

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

@ThomasHornschuh

This comment has been minimized.

Show comment
Hide comment
@ThomasHornschuh

ThomasHornschuh Oct 9, 2017

Contributor

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

Contributor

ThomasHornschuh commented Oct 9, 2017

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

This comment has been minimized.

Show comment
Hide comment
@olofk

olofk Oct 10, 2017

Owner

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.

Owner

olofk commented Oct 10, 2017

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

This comment has been minimized.

Show comment
Hide comment
@olofk

olofk Oct 10, 2017

Owner

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

Owner

olofk commented Oct 10, 2017

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

This comment has been minimized.

Show comment
Hide comment
@olofk

olofk Oct 10, 2017

Owner

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

Owner

olofk commented Oct 10, 2017

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

@ThomasHornschuh

This comment has been minimized.

Show comment
Hide comment
@ThomasHornschuh

ThomasHornschuh Oct 12, 2017

Contributor

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

Contributor

ThomasHornschuh commented Oct 12, 2017

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

@ThomasHornschuh

This comment has been minimized.

Show comment
Hide comment
@ThomasHornschuh

ThomasHornschuh Oct 15, 2017

Contributor

Works !
Many thanks.

Contributor

ThomasHornschuh commented Oct 15, 2017

Works !
Many thanks.

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