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.

Sign up for GitHub

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

Jump to bottom

Fix Random() for big-endian systems #987

Merged
merged 2 commits into from
May 28, 2024
Merged

Fix Random() for big-endian systems #987

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2718cdd
Fix Random() for big-endian systems
th-otto Mar 27, 2024
38b4e2d
Merge branch 'vanilla' into PR-40
giulianobelinassi May 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions common/irandom.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include <stdlib.h>
#include <time.h>
#include "irandom.h"
#include "endianness.h"

unsigned int RandNumb = 0x12349876;

Expand Down Expand Up @@ -59,6 +60,18 @@ unsigned char Random()
** This treats the number as bytes so setting it on bit endian machines needs to byte swap.
*/
unsigned char* bytes = reinterpret_cast<unsigned char*>(&RandNumb);
#ifdef __BIG_ENDIAN__
unsigned char cf1 = (bytes[3] >> 1) & 1;
unsigned char tmp_a = bytes[3] >> 2;
unsigned char cf2 = (bytes[1] >> 7) & 1;
bytes[1] = (bytes[1] << 1) | cf1;
cf1 = (bytes[2] >> 7) & 1;
bytes[2] = (bytes[2] << 1) | cf2;
cf2 = (tmp_a - (RandNumb + (cf1 != 1))) & 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this on the second iteration change RandNumb's value and then change cf2 Can you please elaborate why it works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code treats the unsigned int RandNumb as a 4 byte array. So the endianness has to be taken into account (the comment already warns about this). The new code essentially does the same as the original code for big-endian systems.

Since this is supposed to generate random numbers, it does not make much difference at runtime, but the tests/irandom.cpp only works on little-endian systems.

bytes[3] = (bytes[3] >> 1) | (cf2 << 7);

return bytes[2] ^ bytes[3];
#else
unsigned char cf1 = (bytes[0] >> 1) & 1;
unsigned char tmp_a = bytes[0] >> 2;
unsigned char cf2 = (bytes[2] >> 7) & 1;
Expand All @@ -69,6 +82,7 @@ unsigned char Random()
bytes[0] = (bytes[0] >> 1) | (cf2 << 7);

return bytes[1] ^ bytes[0];
#endif
}

int Get_Random_Mask(int maxval)
Expand Down