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 reverseBV to clash prelude #2725

Open
rowanG077 opened this issue May 21, 2024 · 2 comments
Open

Add reverseBV to clash prelude #2725

rowanG077 opened this issue May 21, 2024 · 2 comments

Comments

@rowanG077
Copy link
Member

rowanG077 commented May 21, 2024

Slow in simulation implementation:

-- | Reverse bitvector
reverseBV :: KnownNat n => BitVector n -> BitVector n
reverseBV = v2bv . reverse . bv2v

Originally mentioned by @christiaanb in #2694 (comment)

  1. add this function to clash-prelude and
  2. make something with better time complexity such as https://graphics.stanford.edu/~seander/bithacks.html#ReverseParallel

Originally posted by @DigitalBrains1 in #2694 (comment)

Hah I had noted this too, but didn't comment on it. I think I didn't even write it down for myself, which was dumb. Anyway...... the current implementation is fine in hardware, and a quick implementation would probably need a primitive. Unless you use clashSimulation from Clash.Magic to use the quick implementation in simulation but use v2bv . reverse . bv2v when generating HDL. That will write your primitive for you (but like a usual primitive, it's still up to you to make sure simulation and HDL match in behaviour).

[edit]
The quick implementation should probably deconstruct the bitvector and construct it again at the end because logical operations on bitvector are needlessly expensive for this application, in this PR I achieved a roughly threefold speedup of logical operations that way.
[/edit]

[edit 2]
I read this wrong: I read this as that Rowan was going to do the implementation. Once the issue is raised, I'll copy-paste the essence of the above to that issue.
[/edit 2]

@rowanG077
Copy link
Member Author

rowanG077 commented May 21, 2024

Once implemented remove reverseBV from:

-- | Reverse bitvector
reverseBV :: KnownNat n => BitVector n -> BitVector n
reverseBV = v2bv . reverse . bv2v

@jvnknvlgl
Copy link
Collaborator

Chiming in here to mention that I'm using the same function here in Clash.Cores.Sgmii.Common:

-- | Reverse the bits of a 'BitVector'
reverseBV :: (KnownNat n) => BitVector n -> BitVector n
reverseBV = v2bv . reverse . bv2v

So if something new is added this has to be replaced as well.

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

No branches or pull requests

3 participants