Skip to content

Conversation

@seldridge
Copy link
Member

Change FIRRTL's inject-dut-hier pass to move all annotations onto the
wrapper 1 when in moveDut=true mode. Previously, this would only move
the MarkDUTAnnotation. This fixes an internal issue where Object Model
paths were not being updated when using this pass in moveDut=true mode.

Other NFC chagnes:

  1. Refactor the InjectDUTHierarchy pass to not introduce the notion of a
    newDUT variable. This only confuses things. Instead setup the DUT and its
    internal wrapper immediately and then reuse these terms in all places.
  2. Rename a test which is now the main test of non-local annotation (NLA)
    behavior inside FIRRTL's inject-dut-hier pass. Previously this was the "new"
    test which duplicated a now deleted test.
  3. Remove a test of non-local annotation (NLA) handling in the inject-dut-hier
    FIRRTL pass. This test started out as the original test, then a copy of it
    was created that used a different NLA format, then the test became a true
    duplicate.

Footnotes

  1. This is using the terminology of the pass which views this as taking
    all the logic inside the original DUT and shoving it into a wrapper
    instantiated inside the DUT. When in moveDut=true mode, it is more
    natural to think of the pass as creating a wrapper around the original
    DUT. However, to adopt this alternative terminology would be modal.
    I.e., "wrapper" would mean something different when in movedDut=true
    mode which would be confusing.

@seldridge seldridge force-pushed the dev/seldridge/firrtl-inject-dut-hier-move-annotations branch from 676c048 to 2b1c2a9 Compare June 24, 2025 02:23
@seldridge seldridge marked this pull request as ready for review June 24, 2025 02:31
@seldridge seldridge requested a review from darthscsi as a code owner June 24, 2025 02:31
@seldridge seldridge force-pushed the dev/seldridge/firrtl-inject-dut-hier-move-annotations branch 3 times, most recently from dcd5214 to 89d7f8a Compare June 25, 2025 17:21
@seldridge seldridge requested a review from youngar June 27, 2025 15:54
Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

Refactor the InjectDUTHierarchy pass to not introduce the notion of a
`newDUT` variable.  This only confuses things.  Instead setup the DUT and
its internal wrapper immediately and then reuse these terms in all places.

Signed-off-by: Schuyler Eldridge <[email protected]>
Change FIRRTL's inject-dut-hier pass to move all annotations onto the
wrapper [^1] when in `moveDut=true` mode.  Additionally, when in this
mode, the WRAPPER will be named using the `name` field of this pass'
controlling annotation.  Previously, this would only move the
`MarkDUTAnnotation` when operating in this mode.  This fixes an internal
issue where Object Model paths were not being updated when using this pass
in `moveDut=true` mode.

[^1]: This is using the terminology of the pass which views this as taking
all the logic inside the original DUT and shoving it into a wrapper
instantiated inside the DUT.  When in `moveDut=true` mode, it is more
natural to think of the pass as creating a wrapper around the original
DUT.  However, to adopt this alternative terminology would be modal.
I.e., "wrapper" would mean something different when in `movedDut=true`
mode which would be confusing.

Signed-off-by: Schuyler Eldridge <[email protected]>
@seldridge seldridge force-pushed the dev/seldridge/firrtl-inject-dut-hier-move-annotations branch from 89d7f8a to 4f348d2 Compare June 27, 2025 16:20
@seldridge seldridge merged commit 4f348d2 into main Jun 27, 2025
3 checks passed
@seldridge seldridge deleted the dev/seldridge/firrtl-inject-dut-hier-move-annotations branch June 27, 2025 16:21
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.

3 participants