Skip to content
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

[Proposal] Remove implicit truncations. #165

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

darthscsi
Copy link
Collaborator

Update the connect algorithm to eliminate type equivalence between smaller to larger conversions.

The type equivalence sections sometimes talks about types being equivalent and sometimes talks about them being connectable. Since the types are not equivalent, they are convertable, update all wording to only talk about connectability.

Update the connect algorithm to eliminate type equivalence between smaller to larger conversions.

The type equivalence sections sometimes talks about types being equivalent and sometimes talks about them being connectable.  Since the types are not equivalent, they are convertable, update all wording to only talk about connectability.
@mmaloney-sf
Copy link
Collaborator

While we're at it, would it be worthwhile to forbid implicit extensions as well?

It's more principled and would let us (almost) remove this section entirely.

@jackkoenig
Copy link
Collaborator

jackkoenig commented Jan 23, 2024

While we're at it, would it be worthwhile to forbid implicit extensions as well?

No. How would you connect a number with a known width to a number with an unknown width that may end up with a larger width due to a later conditional connection? I think this would require explicit width parameters and operations for manipulating them (a good idea, but not something FIRRTL has yet).

It's also just not a good design point for arithmetic types, but I will grant if we had parameters and sufficient operations defined on them then maybe frontend languages like Chisel can just emit the appropriate manipulations.

I think it would be good to add a new "Bits" type that does not have implicit extension, but it too would be hard to use without the mentioned width manipulation operations.

An unsigned integer type is always equivalent to another unsigned integer type regardless of bit width, and is not equivalent to any other type.
Similarly, a signed integer type is always equivalent to another signed integer type regardless of bit width, and is not equivalent to any other type.
An unsigned integer type may be connected to another unsigned integer type of equal or longer width or with an uninferred width.
Similarly, a signed integer type may be connect to another signed integer type of equal or longer width or with an uninferred width.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Similarly, a signed integer type may be connect to another signed integer type of equal or longer width or with an uninferred width.
Similarly, a signed integer type may be connected to another signed integer type of equal or longer width or with an uninferred width.


Two bundle types are equivalent if they have the same number of fields, and both the bundles' i'th fields have matching names and orientations, as well as equivalent types.
Two bundle types may be connected if they have the same number of fields, and both the bundles' i'th fields have matching names and orientations, as well as connectable types.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Two bundle types may be connected if they have the same number of fields, and both the bundles' i'th fields have matching names and orientations, as well as connectable types.
Two bundle types may be connected if they have the same number of fields, and both the bundles' i'th fields have matching names and orientations, and are connectable types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants