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

Use yaml for the config file and add solution specific settings #23

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

Conversation

lukasngl
Copy link

Hi, glad this tool exists!
I needed more solution specific settings, ie. setting directives not in the source but in the directives file and different subsets of the source files across solutions.
So I extended the functionality by

  • use yaml instead of python file for config
  • add global cflags for testbench
  • add (optional) per file cflags
  • select solution by name instead of number
  • set a subset of settings per solution
  • set directives and include tcl scripts per solution

Changes to the codebase:

  • pep8 formatting
  • refactor common funtionality
  • declarative config parser

I use vivado hls 2018.1 so maybesome functionality is broken.
I tested all features on my machine and did not run into errors.

@benjmarshall
Copy link
Owner

Hi Lukas,

Thanks for submitting this! I'm very impressed, it looks like you have done a lot of work here. I'm also glad to hear that there are people out there using this. I'm keen to see this tool improve and progress if people find it useful and so would really like to get this update merged.

I really like the idea of moving to YAML for configuration and I think if I had more knowledge at the time this is what i would have originally done, so a massive thanks for rolling that in. I also like the idea of the extended flexibility in terms of solution specific settings.

You can probably tell from the lack of updates to this repo that I haven't done much with HLSCLT in the last couple of years. This is primarily because I haven't been using HLS as much for the last couple of years, either for work or personal projects. The reason I'm adding this note is really to say that it might take me a little longer than I'd like to get this PR reviewed and merged. I will try and get my Centos VM booted and pull your version for testing but can't guarantee how quickly I can do that. If I can get the code pulled and tested I might have a few questions to come back with.

I noticed you pushed some extra updates after submitting the PR yesterday, do you consider the current status pretty stable? Or are you continuing to develop this further in the next few days?

Can you let me know some more details about the system you use the tool on? I assume it's Linux based? You already mentioned Vivado version so that's perfect. Have you done any testing on other platforms? I had a previous issue discussing Windows 10 compatibility which I couldn't help with as I don't have a Windows 10 system to develop on.

@benjmarshall benjmarshall self-requested a review June 11, 2021 06:34
@benjmarshall benjmarshall self-assigned this Jun 11, 2021
@lukasngl
Copy link
Author

lukasngl commented Sep 30, 2021 via email

@lukasngl
Copy link
Author

lukasngl commented Oct 6, 2021

Hi, i just implemented said changes and polished the commit history.

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

Successfully merging this pull request may close these issues.

None yet

2 participants