-
Notifications
You must be signed in to change notification settings - Fork 154
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
Netlist track usage #2230
Merged
Merged
Netlist track usage #2230
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
alex-mckenna
force-pushed
the
netlist-track-usage
branch
4 times, most recently
from
June 1, 2022 09:09
cb665f9
to
5f90376
Compare
3 tasks
leonschoorl
reviewed
Jun 13, 2022
leonschoorl
reviewed
Jun 13, 2022
leonschoorl
reviewed
Jun 13, 2022
leonschoorl
reviewed
Jun 13, 2022
leonschoorl
reviewed
Jun 13, 2022
leonschoorl
reviewed
Jun 13, 2022
leonschoorl
reviewed
Jun 13, 2022
leonschoorl
reviewed
Jun 13, 2022
leonschoorl
reviewed
Jun 13, 2022
leonschoorl
reviewed
Jun 13, 2022
leonschoorl
reviewed
Jun 13, 2022
leonschoorl
reviewed
Jun 13, 2022
leonschoorl
requested changes
Jun 13, 2022
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.
See ^
alex-mckenna
force-pushed
the
netlist-track-usage
branch
from
June 14, 2022 09:12
5f90376
to
6d7cead
Compare
alex-mckenna
force-pushed
the
netlist-track-usage
branch
2 times, most recently
from
June 21, 2022 07:59
fc779a8
to
e98e567
Compare
martijnbastiaan
approved these changes
Jun 22, 2022
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.
LGTM, but it'd be great if @christiaanb could give this a glance too.
alex-mckenna
force-pushed
the
netlist-track-usage
branch
2 times, most recently
from
June 22, 2022 12:55
ec9b281
to
72b5ab3
Compare
leonschoorl
approved these changes
Jun 24, 2022
In NetDecl', the type of the decl was either a HWType or raw text. Since only the HWType branch was ever used in Clash, this should be simplified.
The function `id2identifier` is an unsafe version of `fromCoreId` in the netlist identifier module. It makes more sense for it to be defined there, so callers can see it is obviously unsafe.
When producing sequential and concurrent code, there are restrictions for how the same declared signal can be assigned which depend on the HDL being targeted. In short, these are VHDL: A `signal` can be assigned continuously, or procedurally with non-blocking assignment. A `variable` can only be assigned procedurally with blocking assignment. Verilog: A `wire` can only be assigned continuously. A `reg` can only be assigned procedurally, but can be assigned both non-blocking and blocking. When the backend produces HDL, it needs to know the type of a declaration to render it. With the `WireOrReg` type, this was easy for VHDL, but does not help differentiate between if a signal is intended to be a VHDL signal or variable. A more general type is now provided which provides the extra information to make this choice: ```haskell data Blocking = Blocking | NonBlocking data Usage = Cont | Proc Blocking ``` Since a declaration can accept more than one type of assignment, the usage is not tracked with the declaration like `WireOrReg` was, but tracked in the assignments and a usage map is created for every component (tracking declared signals to their most restrictive use). When producing netlist, the current use of a signal should be checked to ensure that generated code only produces assignments that are valid for the backend being targeted.
alex-mckenna
force-pushed
the
netlist-track-usage
branch
from
June 24, 2022 14:47
72b5ab3
to
93fd5b4
Compare
2 tasks
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.
When producing sequential and concurrent code, there are restrictions for how the same declared signal can be assigned which depend on the HDL being targeted. In short, these are
signal
can be assigned continuously, or procedurally with non-blocking assignment. Avariable
can only be assigned procedurally with blocking assignment.wire
can only be assigned continuously. Areg
can only be assigned procedurally, but can be assigned both non-blocking and blocking.When the backend produces HDL, it needs to know the type of a declaration to render it. With the
WireOrReg
type, this was easy for VHDL, but does not help differentiate between if a signal is intended to be a VHDL signal or variable. A more general type is now provided which provides the extra information to make this choice:Since a declaration can accept more than one type of assignment, the usage is not tracked with the declaration like
WireOrReg
was, but tracked in the assignments and a usage map is created for every component (tracking declared signals to their most restrictive use). When producing netlist, the current use of a signal should be checked to ensure that generated code only produces assignments that are valid for the backend being targeted.Still TODO: