Skip to content

[FIRRTL] Remove dead annotations relating to taps and blackboxes. #9146

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
Merged
dtzSiFive merged 1 commit into from
Oct 29, 2025
Merged

[FIRRTL] Remove dead annotations relating to taps and blackboxes. #9146

dtzSiFive merged 1 commit into from
Oct 29, 2025

Conversation

@dtzSiFive
Copy link
Contributor

@dtzSiFive dtzSiFive commented Oct 28, 2025

Most of these were internal-only and removed with previous changes years ago.

fabianschuiki
fabianschuiki approved these changes Oct 28, 2025
View reviewed changes
Copy link
Contributor

@fabianschuiki fabianschuiki left a comment

Choose a reason for hiding this comment

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

🥳 Fantastic to see tech debt disappear 😏

seldridge
seldridge approved these changes Oct 28, 2025
View reviewed changes
Copy link
Member

@seldridge seldridge left a comment

Choose a reason for hiding this comment

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

LGTM

Are these dead internally?

Anything to update in docs about this?

@dtzSiFive dtzSiFive force-pushed the feature/drop-unused-tap-bits branch from 9a16618 to e49813f Compare October 28, 2025 22:43
@dtzSiFive
Copy link
Contributor Author

dtzSiFive commented Oct 28, 2025

I need to triple-check the MemTap-based annotations aren't reachable internally, thanks folks!

DataTaps will be removed in follow-on PR, after also removing them internally 👍. Docs removed there, the annotations removed here were not documented or tested in CIRCT (at least not anymore/FWIW).

@dtzSiFive
Copy link
Contributor Author

dtzSiFive commented Oct 29, 2025

I believe we're good!

source/sink/port are members of the MemTap annotation but not used as the class -- that's something that we did within CIRCT: 539e148 .
That is to say -- these annotation classes were never from the input.

🏅 for the wonderful explanatory commit message, made the source code archaeology go a lot easier!

The Deleted/Literal annotation classes were removed internally Feb 2023.

@dtzSiFive dtzSiFive merged commit 433833c into main Oct 29, 2025
7 checks passed
@dtzSiFive dtzSiFive deleted the feature/drop-unused-tap-bits branch October 29, 2025 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabianschuiki fabianschuiki fabianschuiki approved these changes

@seldridge seldridge seldridge approved these changes

@darthscsi darthscsi Awaiting requested review from darthscsi darthscsi is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dtzSiFive @fabianschuiki @seldridge