Skip to content

Templates for scripting#120

Merged
micprog merged 18 commits into
masterfrom
tpl_script
Jan 12, 2024
Merged

Templates for scripting#120
micprog merged 18 commits into
masterfrom
tpl_script

Conversation

@micprog

@micprog micprog commented Mar 23, 2023

Copy link
Copy Markdown
Member

Modify script output to use custom templates (tera package). Allows use of custom script template files.

Fixes #124

@micprog micprog force-pushed the tpl_script branch 2 times, most recently from a27abdc to 8b7b8db Compare April 6, 2023 14:28
@micprog micprog marked this pull request as ready for review April 6, 2023 14:48

@bluewww bluewww left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

I tested flist (it also fixes #124) and vsim and vcs scripts output in carfield's safety island.

@micprog

micprog commented Apr 6, 2023

Copy link
Copy Markdown
Member Author

Review status for all script types:

  • flist (bluewww)
  • vsim (bluewww)
  • vcs (bluewww)
  • verilator
  • synopsys
  • formality
  • riviera
  • genus
  • vivado
  • vivado-sim
  • precision
  • custom template

Other checks (for all script types):

  • separate/common compilation mode
  • no-abort-on-error
  • VHDL sanity check

@micprog micprog requested a review from meggiman April 6, 2023 14:55
@jwnrt

jwnrt commented Apr 13, 2023

Copy link
Copy Markdown
Contributor

Apologies for what feels like a really obnoxious comment, but I was wondering what lead you to choose tera for the templating?

My motivation for asking is that it feels quite heavy with 68 transitive dependencies and adding ~50% to the build time. Maybe this isn't a metric you're measuring or optimizing for, which is totally fair!

From a quick survey of templating engines, tinytemplate is quite widely used and hopefully lightweight, but there are others too. I've only skimmed this PR's template usage, so maybe this doesn't have all the features you need, though.

This is a great feature regardless, thanks for all the work you're doing on it! I don't want to dismiss any of this work or suggest you redo anything!

@micprog

micprog commented Apr 13, 2023

Copy link
Copy Markdown
Member Author

Apologies for what feels like a really obnoxious comment, but I was wondering what lead you to choose tera for the templating?

My motivation for asking is that it feels quite heavy with 68 transitive dependencies and adding ~50% to the build time. Maybe this isn't a metric you're measuring or optimizing for, which is totally fair!

From a quick survey of templating engines, tinytemplate is quite widely used and hopefully lightweight, but there are others too. I've only skimmed this PR's template usage, so maybe this doesn't have all the features you need, though.

This is a great feature regardless, thanks for all the work you're doing on it! I don't want to dismiss any of this work or suggest you redo anything!

Hi @jwnrt,

The main reason I opted for tera was its similarity to other popular templating libraries, lowering the barrier of entry for those not familiar with Rust. The main motivation with this change is to make bender easier to use without having to figure out the intricacies within bender. It was also a good opportunity to streamline the script generation flow for all the different available tools.

I understand that the build time increases, but this is mainly a constraint on development - for deployment, there are easily available binary releases with a custom script for downloading. We have even recently added a custom github CI action to install bender within github actions. Build time of the tool is not a concern.

Regarding other template libraries, I have heard great things about mako, but this is limited to Python. In an effort to allow easier extension to other tools without needing to be familiar with Rust, I opted for a more advanced templating library than tinytemplate to allow more advanced features, in case they are necessary (I use replacements and conditions on loop state, which I don't think is possible with tinytemplate). Arguably, some things could be handled within the software, not the template, but the aim was to also provide the option for a user input template without needing to modify the Rust code. Also, not all libraries have equal active development.

Either way, thank you for your question, I hope this clarifies it somewhat. Let me know if you have feedback on the generated scripts with the new template!

@jwnrt

jwnrt commented Apr 14, 2023

Copy link
Copy Markdown
Contributor

Yep, that answers my question, thank you for the very detailed response!

@micprog micprog force-pushed the tpl_script branch 2 times, most recently from f0e4544 to 99e2b70 Compare July 6, 2023 16:19
Comment thread src/cmd/script.rs Outdated
@CyrilKoe

CyrilKoe commented Jan 9, 2024

Copy link
Copy Markdown

Tested vivado script, see Carfield pipeline.
Visual inspection of vivado-sim ok, only difference with before is that it now repeats certain defines:

set_property verilog_define [list \
    CLUSTER_ALIAS \
    CLUSTER_ALIAS \
    FEATURE_ICACHE_STAT \
    FEATURE_ICACHE_STAT \
    GEN_NO_HYPERBUS=1 \
    HIERARCHY_ICACHE_32BIT \
    HIERARCHY_ICACHE_32BIT \
    ICAHE_USE_FF \
    ICAHE_USE_FF \
    PRIVATE_ICACHE \
    PRIVATE_ICACHE \
    PULP_FPGA_EMUL \
    TARGET_BSCANE \

(Not the case before)

@fischeti

fischeti commented Jan 9, 2024

Copy link
Copy Markdown
Contributor

Synopsys seems to work fine, I didn't see any issues pop during an elaboration step.

Add flist-plus script for files and plusargs
@CyrilKoe

CyrilKoe commented Jan 9, 2024

Copy link
Copy Markdown

Uniquify works for vivado/vivado-sim, now the script is the same as for previous bender version (just with with some re-ordering)

@fischeti

fischeti commented Jan 9, 2024

Copy link
Copy Markdown
Contributor

scripts generated for verilator also seem to run through now

@micprog micprog merged commit 436377f into master Jan 12, 2024
@micprog micprog deleted the tpl_script branch January 12, 2024 10:10
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.

Defines and Includes in flist

6 participants