Add Xcelium script format and --incdir flag#219
Add Xcelium script format and --incdir flag#219DanielKellerM wants to merge 2 commits intopulp-platform:masterfrom
Conversation
97b4aa0 to
5d19a8a
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the Xcelium simulator to the bende tool. The main purpose is to enable script generation for Xcelium by sanitizing library names to be compatible with xrun requirements.
- Added Xcelium as a new format option for script generation
- Implemented library name sanitization by replacing hyphens with underscores in makelib blocks
- Added support for user-provided include directories via the
--incdircommand line argument
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/script_fmt/xcelium_f.tera | New template file for generating Xcelium-compatible script files with sanitized library names |
| src/cmd/script.rs | Added Xcelium format support, include directory argument handling, and per-library template context |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@micprog I am jumping in just to mention that I worked as a beta tester for this and seems to be working pretty well. I am currently testing it also in Cheshire. |
micprog
left a comment
There was a problem hiding this comment.
This PR seems to add multiple things:
- New arg for custom global include directory
- xcelium script format
- new additional structure for passing grouped file information to template
Some things I am noticing:
- The custom incdir is only applied to the all_incdir directive. For existing script formats, this means that it only applies when using a single compile context scripts, not when compiling individual groups. My expectation would be that an additional incdir is applied globally. Please add the custom incdir to the individual file groups for the existing format.
- The new info you are adding to the template struct looks very repetitive to me, as it is almost the same as what is already available with the
srcskey in the template. What looks to be missing is thenamekey. Is there other information I am missing that would prevent you from re-using this existing structure? I see that the code implementation could benefit from some clean-up, but is there a fundamental reason to use a different structure? - If I am not mistaken, the xcelium script format applies incdirs and defines multiple times, once applying all globally and then again individually for the libs. Is this intended?
|
@DanielKellerM do you have bandwidth to look into this? |
|
@yvantor I already started fixing this locally, hopefully next week I update the PR |
ecfb205 to
5296e76
Compare
|
Hi @micprog, I've addressed all your review comments:
The branch has been rebased onto current Bender 0.30.0 |
Add `bender script xcelium` command that generates a flat .f file for Cadence Xcelium/xrun. Per-group incdirs, defines, and source files are emitted directly — consistent with the vsim and vcs templates. Also adds a global `--incdir` / `-I` flag to inject user-provided include directories into both the global incdir list and each per-source-group's incdirs. Other changes: - Add `name` field to TplSrcStruct (from src.package) - Add `root_package` to template context - Support `--relative-path` for $ROOT-relative output - Sanitize library names (hyphens to underscores) in group names
5296e76 to
245e303
Compare
Remove `root_package` and `name` fields from template context that were not used by any .tera template but changed the template_json output, causing cli_template_json regression test to fail.
This PR adds support for the Cadence Xcelium simulator to Bender.
Changes
New command:
bender script xcelium.ffile with per-group incdirs, defines, and source filesvsimandvcstemplates work (no-makelib/-endliblibrary scoping)--relative-pathfor$ROOT-relative outputsource_annotationsmetadata comments (opt-out with--no-source-annotations)New global flag:
--incdir/-IStruct changes
name: Option<String>toTplSrcStruct(populated fromsrc.package)root_packageto template contextTesting
Verified end-to-end with a 5,107-module Snitch Cluster design: