-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
add 'swapEndian' intrinsics to bitops & endians module . #5898
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
Conversation
b48be21 to
8512599
Compare
|
I wouldn't mind losing the proc swapEndian*[T:SomeInteger](x: T): T? |
|
I think that the Int suffix was to prevent name confusion (no name collision) with |
|
Any progress? |
this optimizsation can lead to undefined behaviour since strict aliasing rule is broken.
8512599 to
10d4555
Compare
|
issue #7587 as well as any sign extension bug should now be fixed. |
| template toUint32[T](x: T): uint32 = | ||
| when sizeof(x) == 1: cast[uint8](x).uint32 | ||
| elif sizeof(x) == 2: cast[uint16](x).uint32 | ||
| else: cast[uint32](x) |
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.
I think this should be elif sizeof(x) == 4: cast[uint32](x) else: {.error: "invalid size".} and likewise for the other parts where this pattern is used.
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.
not relevant as of 1.3.5 278b458
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.
oups, didn't realize PR was closed but not merged
|
we already have https://github.com/nim-lang/Nim/blob/devel/lib/pure/endians.nim |
EDIT: The failing test is caused by differences in type conversion between the different platforms & the VM.
Will go back to this later, once this issue is solved.
EDIT: needs integer cast support in VM (PR #5908 waiting for review)