[PATCH] D34089: [llvm-stress] Ensure that the C++11 random device respects its min/max values (PR32585)

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 12 11:08:51 PDT 2017


Simon Pilgrim via Phabricator <reviews at reviews.llvm.org> writes:
> RKSimon created this revision.
>
> As noted on PR32585, the change in
> https://reviews.llvm.org/D29780/https://reviews.llvm.org/rL295325
> resulted in calls to Rand32() (values 0 -> 0xFFFFFFFF) but the
> min()/max() operators indicated it would be (0 -> 0x7FFFF).
>
> This patch changes the random operator to call Rand() instead which
> does respect the 0 -> 0x7FFFF range.

We should probably just delete this whole class and update llvm-stress
to use std::uniform_int_distribution instead, but sure, this change is
simple and correct. LGTM.

> Not sure the best way to test this - would an assert in the operator be enough?

Given that there aren't actually any tests that run llvm-stress at all,
the assert would only fire if someone hit the bug anyway. Seems a bit
redundant.

> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D34089
>
> Files:
>   tools/llvm-stress/llvm-stress.cpp
>
> Index: tools/llvm-stress/llvm-stress.cpp
> ===================================================================
> --- tools/llvm-stress/llvm-stress.cpp
> +++ tools/llvm-stress/llvm-stress.cpp
> @@ -1,4 +1,4 @@
> -//===-- llvm-stress.cpp - Generate random LL files to stress-test LLVM ----===//
> +//===-- llvm-stress.cpp - Generate random LL files to stress-test LLVM ----===//

I can't actually tell what changed here, but it looks strange...

>  //
>  //                     The LLVM Compiler Infrastructure
>  //
> @@ -116,7 +116,7 @@
>  
>    /// Make this like a C++11 random device
>    typedef uint32_t result_type;
> -  uint32_t operator()() { return Rand32(); }
> +  uint32_t operator()() { return Rand(); }
>    static constexpr result_type min() { return 0; }
>    static constexpr result_type max() { return 0x7ffff; }
>    
>


More information about the llvm-commits mailing list