-
Notifications
You must be signed in to change notification settings - Fork 53
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
std-bit-slice implementation #1906
Conversation
Okay, there are failures in the interpreter and the compiler. @anthonyabeo can you take a shot at fixing the compiler errors? With
If the output looks correct, you can save the updated file using the |
Zulip message about how to implement a new primitive in Cider: https://calyx.zulipchat.com/#narrow/stream/423600-development/topic/please-review/near/420297504 |
@EclecticGriffin thanks for the assist. I was able to resolve the interpreter's issue. @rachitnigam noted. My main issue appears to be creating the local environment so I can get the tests to pass. On Mac, even the core tests are failing. I tried using Ubuntu 22.04 and cannot run |
You need to install the |
@rachitnigam @EclecticGriffin I have been able to install most of the dependencies. see screenshot However, some of the tests are still failing and I am not sure how to go about debugging them. See the screenshot below for the output of |
That seems to be because your branch is out of date from |
@rachitnigam I managed to resolve the failing tests. Not sure why but I still get failing tests locally when I merged |
|
||
wires { | ||
group bit_slice { | ||
slice.in = 32'd206256; |
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.
Can we write this number in binary so its more obvious what the slicing is doing?
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.
The syntax in Calyx is 32'b0101011
(note the b
instead of d
)
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.
sure
@@ -0,0 +1,10 @@ | |||
{ | |||
"mem": { | |||
"data": [10], |
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.
This value is never used because we simply write to the memory right? If that is the case, lets make this 0 instead.
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.
sure.
Ah, I can really help debug those without the outputs but this PR seems to be passing all tests! There is one comment about updating the test but after that we can merge! |
sure. Will do |
* add docs entry for std_bit_slice module * add implementation of std_bit_slice module * add test case for std_bit_slice module * add test data for testing std_bit_slice module * fix backward ordering of range extract parameters * add 'std_bit_slice' implementation to interpreter * change the Value.slice method to accept &self * Update std-bit-slice test to use the new 'comb_mem_d1' * Fix failing test * Add 'std_bit_slice' definition to 'tests/import/a.expect' * update std-bit-slice test
This PR addresses the implementation of the proposed bit-slicing operation per #497 .