[WIP] #497 Bit slicing

[anthonyabeo]

This PR addresses the implementation of the proposed bit-slicing operation per #497 .

This module accepts as arguments, a begin_index and end_index, to specify the range of the slice, and outputs the value of the specified range from the input.

I have included an example in the example/tutorial directory that uses this implementation and the expected output file. @sampsyo, I need some assistance on how to write automated tests for this implementation.

I'll appreciate all feedback on improving this implementation.

PS: For now I am calling the module std_bit_slice. Does that sound good? Suggestions are welcome

Thank you.

[rachitnigam]

Not sure if this is the right interface. How about something like: START_IDX and END_IDX instead? This will also simplify the implementation to be just:

assign out = in[START_IDX:END_IDX]

[anthonyabeo]

I didn't realize the parameters can be used that way. Thanks @rachitnigam .

I have pushed the changes.

[sampsyo]

I agree with @rachitnigam here that START_IDX and END_IDX are the right parameters to include here… but unfortunately, some limitations in the Calyx primitive system mean that we need one more parameter too: OUT_WIDTH, which must be exactly equal to the difference between START_IDX and END_IDX.

Here is some more detail on what I mean: this slicing primitive should have a different input width from its output width. When you slice bits 4:3 out of a 32-bit value, for example, you want the result to be a 2-bit value.

In an ideal world, the 3 parameters currently present (input width, start index, end index) would suffice, and we would know that the output would have a width equal to (start index - end index + 1) or whatever. But alas, Calyx primitive parameters (unlike Verilog parameters) cannot be automatically calculated based on other parameters. Every single parameter has to be specified by the instantiating code, even when it is redundant. So to make this work, we will have to include that 4th parameter and require people to specify it correctly.

Does this make sense?

[anthonyabeo]

Yes, that makes sense @sampsyo. I will implement the suggested changes and revert.

[sampsyo]

It's great to have a test for this, and basing the test on the language-tutorial-* programs is also fine, but this is probably not the right place for it: examples/tutorial/ is used specifically for that language tutorial. This probably belongs in tests/, perhaps under tests/correctness/?

We can also delete those ANCHOR comments, which are just used to produce our documentation.

[anthonyabeo]

I've installed runt == 0.4.0 and jq. When I run the tests with runt -i core, Most of the tests are failing. See uploaded image.

What am I missing?

PS: System Information

Platform: MacOS Sonoma.
Rust Version: 1.75.0
runt Version: 0.4.0
jq Version: jq-1.7.1
Calyx Version: v0.6.1

[rachitnigam]

Can you use the runt -d to display the diffs for the failing tests?

[anthonyabeo]

@rachitnigam a bunch of these.

[rachitnigam]

That's really odd. You can use runt -i <test path> -n to extract the exact command that's being run so that you can debug it better. This shouldn't really affect any other tests but weird that you're having failures.

[anthonyabeo]

you mean something like this?

[rachitnigam]

Yup! Seems like the head command is causing the problem so worth running just the part of the command within $(...) and seeing what it prints out?

[anthonyabeo]

so it turns out the issue was due to some missing installations. For instance, the fud e command used to run some of the tests used --through verilog, since I didn't have verilator installed, those tests failed. After I changed it to --through icarus-verilog, those tests passed.

Then I encountered another dependency error for tvm. So I figured it was a dependencies issue, so I decided to go with the Docker approach. After running the container, the tests under runt -i core passed but the others were failing, some due to timeout error.

[rachitnigam]

That sounds okay to me! Also, in case it's helpful, we've created a public zulip channel for Calyx that you can join if you have questions and we can try to answer them more quickly!

[sampsyo]

I agree with @rachitnigam here that START_IDX and END_IDX are the right parameters to include here… but unfortunately, some limitations in the Calyx primitive system mean that we need one more parameter too: OUT_WIDTH, which must be exactly equal to the difference between START_IDX and END_IDX.

Here is some more detail on what I mean: this slicing primitive should have a different input width from its output width. When you slice bits 4:3 out of a 32-bit value, for example, you want the result to be a 2-bit value.

In an ideal world, the 3 parameters currently present (input width, start index, end index) would suffice, and we would know that the output would have a width equal to (start index - end index + 1) or whatever. But alas, Calyx primitive parameters (unlike Verilog parameters) cannot be automatically calculated based on other parameters. Every single parameter has to be specified by the instantiating code, even when it is redundant. So to make this work, we will have to include that 4th parameter and require people to specify it correctly.

Does this make sense?

[sampsyo]

Thanks for getting this started!! I have a couple of granular technical points within.

It would also be awesome to add this to the documentation in this PR. Could you please add an entry to this docs page?
https://github.com/calyxir/calyx/blob/main/docs/libraries/core.md

[rachitnigam]

The output shouldn't be the same width as the input right? It needs to be END_IDX-START_IDX. But again, as @sampsyo said, we need that to be explicitly specified in as a parameter so it should instead be: OUT_WIDTH along with an assertion that checks that the OUT_WIDTH has the correct value.

[rachitnigam]

@anthonyabeo anything blocking the merge? If you have questions, feel free to bump the thread in Zulip.

[rachitnigam]

Closing this in favor of #1906