[PATCH] D29780: Remove uses of `std::random_shuffle` in the llvm code base

[Justin Bogner via llvm-commits]

Marshall Clow via Phabricator <reviews at reviews.llvm.org> writes:
> mclow.lists created this revision.
>
> `std::random_shuffle` has been deprecated since C++14, and will be
> removed in C++17.  The replacement is `std::shuffle`.  Change all of
> the remaining uses (outside of libc++) to use std::shuffle.

LGTM.

> The rationale for doing this in the standard is here:
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4190
>
> I'd appreciate some feedback on the changes in ListReducer.h; I
> matched what was there, but was not 100% sure of the intent.

This code is just heuristically trying to avoid worst-case behaviour of
the search by jumbling up the list. These changes should accomplish (or
fail to accomplish) that just as well as the old code did.

> https://reviews.llvm.org/D29780
>
> Files:
>   tools/bugpoint/FindBugs.cpp
>   tools/bugpoint/ListReducer.h
>   tools/llvm-stress/llvm-stress.cpp
>   unittests/ADT/TinyPtrVectorTest.cpp
>
> Index: unittests/ADT/TinyPtrVectorTest.cpp
> ===================================================================
> --- unittests/ADT/TinyPtrVectorTest.cpp
> +++ unittests/ADT/TinyPtrVectorTest.cpp
> @@ -17,6 +17,7 @@
>  #include "llvm/Support/type_traits.h"
>  #include "gtest/gtest.h"
>  #include <algorithm>
> +#include <random>
>  #include <vector>
>  
>  using namespace llvm;
> @@ -23,13 +24,6 @@
>  
>  namespace {
>  
> -// The world's worst RNG, but it is deterministic and makes it easy to get
> -// *some* shuffling of elements.
> -static ptrdiff_t test_shuffle_rng(ptrdiff_t i) {
> -  return (i + i * 33) % i;
> -}
> -static ptrdiff_t (*test_shuffle_rng_p)(ptrdiff_t) = &test_shuffle_rng;
> -
>  template <typename VectorT>
>  class TinyPtrVectorTest : public testing::Test {
>  protected:
> @@ -46,7 +40,7 @@
>      for (size_t i = 0, e = array_lengthof(TestValues); i != e; ++i)
>        TestPtrs.push_back(&TestValues[i]);
>  
> -    std::random_shuffle(TestPtrs.begin(), TestPtrs.end(), test_shuffle_rng_p);
> +    std::shuffle(TestPtrs.begin(), TestPtrs.end(), std::mt19937{});
>    }
>  
>    ArrayRef<PtrT> testArray(size_t N) {
> Index: tools/llvm-stress/llvm-stress.cpp
> ===================================================================
> --- tools/llvm-stress/llvm-stress.cpp
> +++ tools/llvm-stress/llvm-stress.cpp
> @@ -28,6 +28,7 @@
>  #include "llvm/Support/PrettyStackTrace.h"
>  #include "llvm/Support/ToolOutputFile.h"
>  #include <algorithm>
> +#include <random>
>  #include <vector>
>  
>  namespace llvm {
> @@ -112,7 +113,13 @@
>    ptrdiff_t operator()(ptrdiff_t y) {
>      return  Rand64() % y;
>    }
> -
> +  
> +// Make this like a C++11 random device
> +  typedef uint32_t result_type;
> +  uint32_t operator()() { return Rand32(); }
> +  static constexpr result_type min() { return 0; }
> +  static constexpr result_type max() { return 0x7ffff; }
> +  
>  private:
>    unsigned Seed;
>  };
> @@ -662,7 +669,7 @@
>        BoolInst.push_back(&Instr);
>    }
>  
> -  std::random_shuffle(BoolInst.begin(), BoolInst.end(), R);
> +  std::shuffle(BoolInst.begin(), BoolInst.end(), R);
>  
>    for (auto *Instr : BoolInst) {
>      BasicBlock *Curr = Instr->getParent();
> Index: tools/bugpoint/ListReducer.h
> ===================================================================
> --- tools/bugpoint/ListReducer.h
> +++ tools/bugpoint/ListReducer.h
> @@ -19,6 +19,7 @@
>  #include "llvm/Support/raw_ostream.h"
>  #include <algorithm>
>  #include <cstdlib>
> +#include <random>
>  #include <vector>
>  
>  namespace llvm {
> @@ -46,7 +47,7 @@
>    /// that bugpoint does.
>    Expected<bool> reduceList(std::vector<ElTy> &TheList) {
>      std::vector<ElTy> empty;
> -    std::srand(0x6e5ea738); // Seed the random number generator
> +    std::mt19937 randomness(0x6e5ea738);  // Seed the random number generator
>      Expected<TestResult> Result = doTest(TheList, empty);
>      if (Error E = Result.takeError())
>        return std::move(E);
> @@ -92,7 +93,7 @@
>        // distribution (improving the speed of convergence).
>        if (ShufflingEnabled && NumOfIterationsWithoutProgress > MaxIterations) {
>          std::vector<ElTy> ShuffledList(TheList);
> -        std::random_shuffle(ShuffledList.begin(), ShuffledList.end());
> +        std::shuffle(ShuffledList.begin(), ShuffledList.end(), randomness);
>          errs() << "\n\n*** Testing shuffled set...\n\n";
>          // Check that random shuffle doesn't lose the bug
>          Expected<TestResult> Result = doTest(ShuffledList, empty);
> Index: tools/bugpoint/FindBugs.cpp
> ===================================================================
> --- tools/bugpoint/FindBugs.cpp
> +++ tools/bugpoint/FindBugs.cpp
> @@ -21,6 +21,7 @@
>  #include "llvm/Support/raw_ostream.h"
>  #include <algorithm>
>  #include <ctime>
> +#include <random>
>  using namespace llvm;
>  
>  Error
> @@ -39,14 +40,13 @@
>        return E;
>    }
>  
> -  srand(time(nullptr));
> -
> +  std::mt19937 randomness(std::random_device{}());
>    unsigned num = 1;
>    while (1) {
>      //
>      // Step 1: Randomize the order of the optimizer passes.
>      //
> -    std::random_shuffle(PassesToRun.begin(), PassesToRun.end());
> +    std::shuffle(PassesToRun.begin(), PassesToRun.end(), randomness);
>  
>      //
>      // Step 2: Run optimizer passes on the program and check for success.