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

Fix unaligned cast in impl_uint_from_fill #20

Merged
merged 1 commit into from
Oct 31, 2017

Conversation

pitdicker
Copy link

It seems to me doing an unsafe cast from [u8; 4] to u32 is not right, because they have different minimum alignments. But this code was used by both OsRng and ReadRng, and did not seems to cause problems. No idea why.

@dhardy
Copy link
Owner

dhardy commented Oct 23, 2017

I would guess the byte array was sufficiently aligned anyway.

Does this change affect benchmarks?

@pitdicker
Copy link
Author

I think LLVM optimizes out a memcopy here and writes fill_bytes directly to the u32 in the parent stack frame. So in practice it is was already aligned well. So no real problem here, but it seems neater to me :-).

I will run a benchmark, but even if it compiles to different instructions I doubt it is something we can measure.

@pitdicker
Copy link
Author

There is no measurable change in benchmarks.

@dhardy dhardy merged commit 9a4e2e7 into dhardy:master Oct 31, 2017
@pitdicker pitdicker deleted the fill_from_next branch October 31, 2017 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants