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

Add infinite-width bit extract operator #869

Closed

Conversation

albert-magyar
Copy link
Contributor

Related issue: #857
Type of change: other enhancement
Impact: API addition (no impact on existing code)
Development Phase: hypothetical proposal

@jackkoenig @azidar @ducky64 This is an example of the hypothetical "infinite extract" operator. Not that I'm saying this is the right solution, but it is one easy solution.

Note: i is a terrible name that I just made up as an example.

@ducky64
Copy link
Contributor

ducky64 commented Aug 1, 2018

Can we do something more compositional rather than introducing a special-case extract operation? I'd prefer something like (5.U.pad)(4, 2), though we should think about that an unbounded pad would do (or whether it's even useful) in other cases.

@azidar
Copy link
Contributor

azidar commented Sep 7, 2018

I'm not sure what the final word on this was, but I personally am in favor of not breaking backwards compatibility for this, and just adding a safe extract operator. I don't like i as the API though....

@ducky64
Copy link
Contributor

ducky64 commented Oct 12, 2018

So the issue is that the backwards compatible solution means inconsistent behavior depending on whether your bit extraction is handled in Chisel frontend on a literal (in which case it implicitly pads the number to infinity) or in FIRRTL (where it would error out). And I'd argue that the former case is probably more of a bug than anything else, since it was a result of trying to special case literals in the chisel frontend but failing to maintain consistent behavior with FIRRTL.

We could also introduce a runtime deprecation phase, where the inconsistency will continue to work but emit a runtime deprecation warning, before removing it outright.

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

Successfully merging this pull request may close these issues.

3 participants