verific: Add bottom and top bound properties to wire #4538
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
RCoeurjoly
force-pushed
the
verific_bounds
branch
from
45c09bd to
1af9fa6
Compare
povik
reviewed
Aug 14, 2024
povik
reviewed
Aug 14, 2024
widlarizer
changed the title
nakengelhardt
previously requested changes
Aug 16, 2024
There was a problem hiding this comment.
Please make your formatting conform to the surrounding code/the yosys coding style.
The tests do not seem to test anything, what is their intent?
Formatting done. The tests were meant to exercise the |
RCoeurjoly
force-pushed
the
verific_bounds
branch
from
849a092 to
d4de358
Compare
|
Coverted to draft because most likely we want this PR to be merged first so that |
RCoeurjoly
force-pushed
the
verific_bounds
branch
from
d4de358 to
7eff1a0
Compare
RCoeurjoly
dismissed
nakengelhardt’s stale review
Requested changes are done, but no way to resolve the conversation
RCoeurjoly
changed the title
RCoeurjoly
changed the title
for signed attributes Co-authored-by: N. Engelhardt <[email protected]> Co-authored-by: Roland Coeurjoly <[email protected]>
RCoeurjoly
force-pushed
the
verific_bounds
branch
from
4bde830 to
bdc43c6
Compare
povik
approved these changes
Sep 10, 2024
nakengelhardt
approved these changes
Sep 12, 2024
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.
What are the reasons/motivation for this change?
Having bound properties for wires would help detect underflow and overflow in cases where the user is more restrictive than the RTLIL representation.
For example, the following VHDL
signal a : INTEGER range 0 to 6;is represented in RTLIL as:
wire width 3 input 1 \abecause to represent decimal 6 you need at least 3 bits.However, decimal 7 can also be represented, which would be an overflow as per the user specified range, and that user constraint would get lost without this attribute.
This patch helps ensure that those user constraints are maintained and accessible in the RTLIL representation.
Explain how this is achieved.
With the Verific API, we get the bounds information and create attributes (named
bottom_boundandtop_bound).Those attributes are of the same bit width as the wire they belong to.
If at least one the bound is negative, both constants are flagged as signed.
Supporting changes
printattrs.cc: Updated the logic to print attributes for signed constants.
RTLIL::Const: The constructor was modified to use
long longinstead ofint. This is necessary because to support VHDL 2019 integers with bit widths up to 64, the Verific API’sTypeRange::GetScalarRangeLeftBound()method returns along long.Discarded options
We decided that bound attributes for enumeration types will not be added in this PR.