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

Speed up, harmonize BitReduction #2244

Merged
merged 1 commit into from
Jun 20, 2022
Merged

Speed up, harmonize BitReduction #2244

merged 1 commit into from
Jun 20, 2022

Conversation

DigitalBrains1
Copy link
Member

@DigitalBrains1 DigitalBrains1 commented Jun 17, 2022

The cases for reduceAnd and reduceOr where the mask is non-zero can
also be sped up just like the cases with a zero mask.

reduceAnd and reduceOr always produce a Bit, but possibly with a
non-zero mask (i.e., an undefined value). reduceXor should also
produce such a value, which is different from throwing an XException
as it did.

Still TODO:

  • Write a changelog entry (see changelog/README.md)
  • Check copyright notices are up to date in edited files

@DigitalBrains1
Copy link
Member Author

Ooops, silly me. Hold on.

@DigitalBrains1
Copy link
Member Author

Okay, now without dumb gaffes.

@DigitalBrains1 DigitalBrains1 marked this pull request as ready for review June 17, 2022 09:37
Copy link
Member

@martijnbastiaan martijnbastiaan left a comment

Choose a reason for hiding this comment

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

Speed ups and harmonization LGTM.

With regards to the XException change: I think what you're doing here is correct. We've had reports before where people expected an undefined bitvector to have mask ~ maxBound instead of being a bottom. To me this makes sense, as the latter will break unjustifiably whenever an operation is applied to them. My opinion on it isn't very strong though.

clash-prelude/src/Clash/Sized/Internal/BitVector.hs Outdated Show resolved Hide resolved
The cases for `reduceAnd` and `reduceOr` where the mask is non-zero can
also be sped up just like the cases with a zero mask.

`reduceAnd` and `reduceOr` always produce a `Bit`, but possibly with a
non-zero mask (i.e., an undefined value). `reduceXor` should also
produce such a value, which is different from throwing an `XException`
as it did.
@DigitalBrains1 DigitalBrains1 merged commit 99824af into master Jun 20, 2022
@DigitalBrains1 DigitalBrains1 deleted the reducefuns branch June 20, 2022 15:42
mergify bot pushed a commit that referenced this pull request Jun 20, 2022
The cases for `reduceAnd` and `reduceOr` where the mask is non-zero can
also be sped up just like the cases with a zero mask.

`reduceAnd` and `reduceOr` always produce a `Bit`, but possibly with a
non-zero mask (i.e., an undefined value). `reduceXor` should also
produce such a value, which is different from throwing an `XException`
as it did.

(cherry picked from commit 99824af)
DigitalBrains1 added a commit that referenced this pull request Jun 20, 2022
The cases for `reduceAnd` and `reduceOr` where the mask is non-zero can
also be sped up just like the cases with a zero mask.

`reduceAnd` and `reduceOr` always produce a `Bit`, but possibly with a
non-zero mask (i.e., an undefined value). `reduceXor` should also
produce such a value, which is different from throwing an `XException`
as it did.

(cherry picked from commit 99824af)

Co-authored-by: Peter Lebbing <peter@digitalbrains.com>
vmchale pushed a commit that referenced this pull request Jun 23, 2022
The cases for `reduceAnd` and `reduceOr` where the mask is non-zero can
also be sped up just like the cases with a zero mask.

`reduceAnd` and `reduceOr` always produce a `Bit`, but possibly with a
non-zero mask (i.e., an undefined value). `reduceXor` should also
produce such a value, which is different from throwing an `XException`
as it did.
vmchale pushed a commit that referenced this pull request Jun 27, 2022
The cases for `reduceAnd` and `reduceOr` where the mask is non-zero can
also be sped up just like the cases with a zero mask.

`reduceAnd` and `reduceOr` always produce a `Bit`, but possibly with a
non-zero mask (i.e., an undefined value). `reduceXor` should also
produce such a value, which is different from throwing an `XException`
as it did.
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.

None yet

2 participants