-
Notifications
You must be signed in to change notification settings - Fork 162
Fix numeric prims #2689
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
Merged
Merged
Fix numeric prims #2689
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
martijnbastiaan
requested changes
Mar 4, 2024
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.
Nice work! I haven't thoroughly checked the changes, but the tests are pretty convincing I'd say. Minor changes requested.
christiaanb
requested changes
Mar 5, 2024
clash-lib/prims/vhdl/Clash_Sized_Internal_Signed.primitives.yaml
Outdated
Show resolved
Hide resolved
I'll squash/fixup the new commits later. |
christiaanb
approved these changes
Mar 6, 2024
martijnbastiaan
approved these changes
Mar 6, 2024
They're used to implement countLeadingZeros and countTrailingZeros on Int* and Word*
With GHC 9.4 the unboxed version of Int64 changed from Int# to Int64#. And similar for Word64. This caused the evalutor fail to evalute many primitives on Word64 and Int64.
Now we don't truncate them to 16 bits any more. Also deduplicate core Literal to netlist Literal conversion code.
In VHDL integers are only guarenteed to include [-2^31-1..2^31-1] inclusive. For -2^31 we now fallback to path for wider numbers, that doesn't use VHDL integers. In (System)Verilog we didn't do any limiting, which verilator didn't like. Now we use the same algorithm as VHDL.
Which implements testBit @integer on ghc >= 9.0
This used to be implented as resize(~ARG[0] * ~ARG[1],~SIZE[~TYPO]) But resize on signed numbers in VHDL preserves the sign bit. But in Haskell and the other HDLs we just truncate the result, and that has different behaviour on over/underflow.
fd29fd9
to
035c64d
Compare
Make it do the same thing as the VHDL version. When expanding, it now duplicates the sign bit, preserving the value, that's new in this commit. When shrinking it does a bit slice, implementing the wrap-around behavior that the Haskell simulation does.
035c64d
to
d4c5588
Compare
martijnbastiaan
added a commit
that referenced
this pull request
Mar 8, 2024
Fix numeric prims (copy #2689)
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes various issues with primitives on various number types.
Some of it caused by changes in GHC, like the change from integer-gmp to ghc-bignum and the addition of sized versions of unboxed Int and unboxed Word.
And some that were just never tested properly.
For more details see the individual commits.
Still TODO: