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

[SV] Cleanup HWExportModuleHierarchyPass #5485

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

seldridge
Copy link
Member

Cleanup the HWExportModuleHierarchyPass pass in the following ways:

  1. Do not emit JSON directly, but rely use sv.verbatim operations with substitutions.

  2. Move HWExportModuleHierarchyPass before ExportVerilog (again). Given that (1) is happening, this must happen. Both (1) and (2) effectively undo the work in [HWExportModuleHierarchy] Directly emit JSON to a file instead of an sv.verbatim op. #1931.

  3. Use a hw::ModuleNamespace to ensure unique symbols are created for substitutions. This motivated some refactoring of the HWExportModuleHierarchyPass from a struct to a class since it now has private members. (This aligns with LLVM coding guidelines.)

  4. Update existing tests to expect the new sv.verbatim JSON.

  5. Add a new firtool end-to-end test baed on [FIRRTL][SV] Module Hierarchy Files Not Using VerilogName #5478.

@seldridge seldridge changed the title Dev/seldridge/export hierarchy cleanup [SV] Cleanup HWExportModuleHierarchyPass Jun 27, 2023
@seldridge seldridge force-pushed the dev/seldridge/export-hierarchy-cleanup branch from ad0dc10 to 653112f Compare June 27, 2023 15:55
Cleanup the `HWExportModuleHierarchyPass` pass in the following ways:

  1. Do not emit JSON directly, but rely use `sv.verbatim` operations with
  substitutions.

  2. Move `HWExportModuleHierarchyPass` before `ExportVerilog` (again).
  Given that (1) is happening, this must happen.  Both (1) and (2)
  effectively undo the work in #1931.

  3. Use a `hw::ModuleNamespace` to ensure unique symbols are created for
  substitutions.  This motivated some refactoring of the
  `HWExportModuleHierarchyPass` from a `struct` to a `class` since it now
  has private members. (This aligns with LLVM coding guidelines.)

  4. Update existing tests to expect the new `sv.verbatim` JSON.

  5. Add a new `firtool` end-to-end test baed on #5478.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
@seldridge seldridge force-pushed the dev/seldridge/export-hierarchy-cleanup branch from 653112f to 4740516 Compare June 27, 2023 21:40
@seldridge seldridge merged commit 4740516 into main Jun 27, 2023
5 checks passed
@seldridge seldridge deleted the dev/seldridge/export-hierarchy-cleanup branch June 27, 2023 22:16
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

1 participant