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.

Sign up for GitHub

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

Jump to bottom

[FIRRTL] Allow local targets to be multiply-instantiated. #7613

Merged
merged 3 commits into from
Sep 21, 2024
Merged

[FIRRTL] Allow local targets to be multiply-instantiated. #7613

merged 3 commits into from
Sep 21, 2024

Commits on Sep 20, 2024

  1. [FIRRTL] Allow local targets to be multiply-instantiated. 

    The existing path support was built up based on the assumption that
every target is unique. That is true for FIRRTL produced by standard
Chisel code, which elaborates unique modules for each instance. We
definitely don't want to limit ourselves to this world, and we should
support targeting things that are multiply instantiated when it is not
ambiguous what we refer to.

This patch relaxes the single-instantiation constraints for local
targets, which refer to a module or something inside a module,
regardless of how many times or at what paths that particular module
was instantiated.

This required a couple changes through the pipeline:

ResolvePaths already had an early exit for the local path case, but
this needed to come before the single-instantiation check.

LowerClasses needed a couple small changes to not enforce the
single-instantiation check in the local path case, and to build a
hierpath that just has a single element.

While this is not a new requirement, we can still get ambiguous local
targets, for instance from nested module prefixing. The error message
in LowerClasses for this case was made a little more clear.
    @mikeurbach
    mikeurbach committed Sep 20, 2024
    Configuration menu
    Copy the full SHA
    103090a View commit details
    Browse the repository at this point in the history

  2. Update lib/Dialect/FIRRTL/Transforms/ResolvePaths.cpp 

    Co-authored-by: Hideto Ueno <[email protected]>
    @mikeurbach @uenoku
    mikeurbach and uenoku authored Sep 20, 2024
    Configuration menu
    Copy the full SHA
    71d7792 View commit details
    Browse the repository at this point in the history

  3. Update comment about ambiguity.

    @mikeurbach
    mikeurbach committed Sep 20, 2024
    Configuration menu
    Copy the full SHA
    4d28fb7 View commit details
    Browse the repository at this point in the history