-
Notifications
You must be signed in to change notification settings - Fork 611
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
[ChiselSim] Add Inline Layer Control #4555
Merged
Merged
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
seldridge
changed the title
Dev/seldridge/chiselsim inline layers
[ChiselSim] Add Inline Layer Control
Dec 10, 2024
This also should be updated to work with |
seldridge
force-pushed
the
dev/seldridge/chiselsim-inline-layers
branch
from
December 10, 2024 20:59
caf364b
to
362d104
Compare
jackkoenig
reviewed
Dec 10, 2024
jackkoenig
approved these changes
Dec 11, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM, a few nits and a question/point about whether certain APIs should be public or private.
Add a function that returns that sequence of all layers from the current layer up to the root. This is part of forthcoming support for different layer convention ABIs which need to use this information in their ABI utility functions. Signed-off-by: Schuyler Eldridge <[email protected]>
Add an enumeration and functions that can be used to help generate the information for the two standard layer convention ABIs. Signed-off-by: Schuyler Eldridge <[email protected]>
Add a field to the `DesignAnnotation` that stores all layers that were generated in a run of Chisel. This is intended to be used by testing APIs that need to know what layers exist in order to turn them all on or to error if a user tries to enable a layer that does not exist. Signed-off-by: Schuyler Eldridge <[email protected]>
Add a member function `preprocessorDefines` to `LayerControl.Type` which will return any preprocessor defines that should be set to enable the requested layers. Add an abstract member function, `getLayerSubset`, which layer control concrete types implement to make the preprocessor define function work. Signed-off-by: Schuyler Eldridge <[email protected]>
Extend ChiselSim with the ability to control inline layers. This requires untangling some of the way that compilation settings are passed into a `Simulator` because these options need to be added to after Chisel elaboration. E.g., to enable all inline layers, this requires knowing all the layers that were created. This didn't come up previously with extract layers as all layer files could be blindly included for this case. In susbequent commits, I will cleanup the way that extract layers work to use the same code that is introduced here. Signed-off-by: Schuyler Eldridge <[email protected]>
seldridge
force-pushed
the
dev/seldridge/chiselsim-inline-layers
branch
from
December 11, 2024 04:13
c61fb86
to
bc063c0
Compare
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.
Add support for ChiselSim to turn on inline layers. This requires a different approach than how this worked for extract layers in order to continue to support the ability to "enable all" layers. For extract layers, this was straightforward: all files which looked like layer files could be included. However, for inline layers, to enable all layers requires knowledge of the layers that exist because Verilog preprocessor defines must be hand-crafted.
The first part of this patch adds support for layer ABIs. After that, the
DesignAnnotation
is widened to include information about what layers were elaborated. Finally, ChiselSim and svsim classes/functions are untangled to make it possible to add information to the compiler options after elaborating the circuit.There are stubs in here to add support for extract layers to work the same way. I'd like to land this patch first and then clean that up following.
Commits should be reviewed individually.
Release Notes
Fix ChiselSim so that inline layers can be enabled using the existing
layerControl
argument to simulators.