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

[jhuber6]

Summary:
This patch improves the implementation of the standard rand() function
by implementing it in terms of the xorshift64star pRNG as described in
https://en.wikipedia.org/wiki/Xorshift#xorshift*. This is a good,
general purpose random number generator that is sufficient for most
applications that do not require an extremely long period. This patch
also correctly initializes the seed to be 1 as described by the
standard. We also increase the RAND_MAX value to be INT_MAX as the
standard only specifies that it can be larger than 32768.

[llvmbot]

@llvm/pr-subscribers-libc

Changes

Summary:
This patch improves the implementation of the standard rand() function
by implementing it in terms of the xorshift64star pRNG as described in
https://en.wikipedia.org/wiki/Xorshift#xorshift*. This is a good,
general purpose random number generator that is sufficient for most
applications that do not require an extremely long period. This patch
also correctly initializes the seed to be 1 as described by the
standard. We also increase the RAND_MAX value to be INT_MAX as the
standard only specifies that it can be larger than 32768.

--
Full diff: https://github.com/llvm/llvm-project/pull/66131.diff

4 Files Affected:

• (modified) libc/include/llvm-libc-macros/stdlib-macros.h (+1-1)
• (modified) libc/src/stdlib/rand.cpp (+7-5)
• (modified) libc/src/stdlib/rand_util.cpp (+3-1)
• (modified) libc/test/src/stdlib/rand_test.cpp (+7)

diff --git a/libc/include/llvm-libc-macros/stdlib-macros.h b/libc/include/llvm-libc-macros/stdlib-macros.h
index 1c66a4359a6876d..a7625aa187c910f 100644
--- a/libc/include/llvm-libc-macros/stdlib-macros.h
+++ b/libc/include/llvm-libc-macros/stdlib-macros.h
@@ -17,6 +17,6 @@
 #define EXIT_SUCCESS 0
 #define EXIT_FAILURE 1
 
-#define RAND_MAX 32767
+#define RAND_MAX 2147483647
 
 #endif // __LLVM_LIBC_MACROS_STDLIB_MACROS_H
diff --git a/libc/src/stdlib/rand.cpp b/libc/src/stdlib/rand.cpp
index ef6a7211ab097e2..5c5bc78a65e4945 100644
--- a/libc/src/stdlib/rand.cpp
+++ b/libc/src/stdlib/rand.cpp
@@ -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((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 ((rand_next * 0x2545F4914F6CDD1Dull) >> 32) & RAND_MAX;
 }
 
 } // namespace __llvm_libc
diff --git a/libc/src/stdlib/rand_util.cpp b/libc/src/stdlib/rand_util.cpp
index 9c29eb880df1b26..dac8dca2804e1c2 100644
--- a/libc/src/stdlib/rand_util.cpp
+++ b/libc/src/stdlib/rand_util.cpp
@@ -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
diff --git a/libc/test/src/stdlib/rand_test.cpp b/libc/test/src/stdlib/rand_test.cpp
index fcd693cda743b13..34054eb583fc948 100644
--- a/libc/test/src/stdlib/rand_test.cpp
+++ b/libc/test/src/stdlib/rand_test.cpp
@@ -14,11 +14,18 @@
 #include 
 
 TEST(LlvmLibcRandTest, UnsetSeed) {
+  static int vals[1000];
+
   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;
   }
+
+  __llvm_libc::srand(1);
+  for (size_t i = 0; i < 1000; ++i)
+    ASSERT_EQ(__llvm_libc::rand(), vals[i]);
 }
 
 TEST(LlvmLibcRandTest, SetSeed) {

[lntue]

Shall we define it as INT_MAX instead?

[jhuber6]

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

[michaelrj-google]

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

[lntue]

why do we need static here?

[jhuber6]

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).

[michaelrj-google]

Overall looks good

[michaelrj-google]

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

[michaelrj-google]

LGTM