Skip to content

[libc] Improve the implementation of the rand() function #66131

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

Merged
merged 2 commits into from
Sep 12, 2023
Merged

[libc] Improve the implementation of the rand() function #66131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8a674da
[libc] Improve the implementation of the rand() function
jhuber6 Sep 12, 2023
8ab90e1
fix comment
jhuber6 Sep 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libc/include/llvm-libc-macros/stdlib-macros.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@
#define EXIT_SUCCESS 0
#define EXIT_FAILURE 1

#define RAND_MAX 32767
#define RAND_MAX 2147483647
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we define it as INT_MAX instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure if that would always be available when this header is included.

Copy link
Contributor

Choose a reason for hiding this comment

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

given that the implementation shifts right by 32, I think it's likely safe to have this be written out.


#endif // __LLVM_LIBC_MACROS_STDLIB_MACROS_H
12 changes: 7 additions & 5 deletions libc/src/stdlib/rand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@

namespace __llvm_libc {

// This rand function is the example implementation from the C standard. It is
// not cryptographically secure.
LLVM_LIBC_FUNCTION(int, rand, (void)) { // RAND_MAX is assumed to be 32767
rand_next = rand_next * 1103515245 + 12345;
return static_cast<unsigned int>((rand_next / 65536) % 32768);
// An implementation of the xorshift64star pseudo random number generator. This
// is a good general purpose generator for most non-cryptographics applications.
LLVM_LIBC_FUNCTION(int, rand, (void)) {
rand_next ^= rand_next >> 12;
rand_next ^= rand_next << 25;
rand_next ^= rand_next >> 27;
return static_cast<int>((rand_next * 0x2545F4914F6CDD1Dul) >> 32) & RAND_MAX;
}

} // namespace __llvm_libc
4 changes: 3 additions & 1 deletion libc/src/stdlib/rand_util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace __llvm_libc {

LIBC_THREAD_LOCAL unsigned long rand_next;
// C standard 7.10p2: If 'rand' is called before 'srand' it is to proceed as if
// the 'srand' function was called with a value of '1'.
LIBC_THREAD_LOCAL unsigned long rand_next = 1;

} // namespace __llvm_libc
10 changes: 10 additions & 0 deletions libc/test/src/stdlib/rand_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,21 @@
#include <stdlib.h>

TEST(LlvmLibcRandTest, UnsetSeed) {
static int vals[1000];
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need static here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a larger array, so putting static there places it in the .bss section instead of the stack which makes it less likely to explode when run on targets with a smaller stack (AKA the GPU).


for (size_t i = 0; i < 1000; ++i) {
int val = __llvm_libc::rand();
ASSERT_GE(val, 0);
ASSERT_LE(val, RAND_MAX);
vals[i] = val;
}

// The C standard specifies that if 'srand' is never called it should behave
// as if 'srand' was called with a value of 1. If we seed the value with 1 we
// should get the same sequence as the unseeded version.
__llvm_libc::srand(1);
for (size_t i = 0; i < 1000; ++i)
ASSERT_EQ(__llvm_libc::rand(), vals[i]);
}

TEST(LlvmLibcRandTest, SetSeed) {
Expand Down