[FIRRTL][HW] Replace ModuleNamespace with InnerSymbol one. #5819
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
Combine identical "ModuleNamespaces" into a new common one for anything with inner symbols. First step towards dropping use of a "Namespace" for generating inner symbols entirely, once other pieces are in place. Also sets the stage for moving the utility functions that help create inner symbols and inner refs out of FIRRTL. NFCI.
dtzSiFive
force-pushed
the
feature/innersym-namespace
branch
from
b1352e5
to
af25cc8
Compare
seldridge
approved these changes
Aug 10, 2023
There was a problem hiding this comment.
Nice cleanup!
With the removal of the original ModuleNamespace this is clearly moving in a direction of inner symbols are unique, but names within a module are not unique in FIRRTL dialect. This is fine and reasonable. This will have some consequences as you've seen before with port uniqueness.
Comment on lines
+116
to
+117
| using GetNamespaceCallback = | ||
| llvm::function_ref<hw::InnerSymbolNamespace &(FModuleLike mod)>; |
There was a problem hiding this comment.
Good use of an alias here to keep the function signatures tighter. 👍
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Combine identical "ModuleNamespaces" into a new common one
for anything with inner symbols.
First step towards dropping use of a "Namespace" for generating
inner symbols entirely, once other pieces are in place.
Also sets the stage for moving the utility functions that help
create inner symbols and inner refs out of FIRRTL.
NFCI.