[PATCH] Fixed Noop insertion

Stephen Crane sjcrane at uci.edu
Mon Jan 26 16:02:40 PST 2015


================
Comment at: lib/Support/RandomNumberGenerator.cpp:69
@@ +68,2 @@
+  return RandomNumber % Max;
+}
----------------
jfb wrote:
> Do you think you could create a utility that effectively meets the `uniform_int_distribution` requirements of the standard, while offering an implementation that doesn't change from one standard library to another? This would make it easier to convince readers of the code that the RNG+distribution is actually doing what they expect.
> 
> The obvious fix is to simply include [[ http://llvm.org/svn/llvm-project/libcxx/trunk/include/algorithm | libc++'s implementation ]] since the license is the same:
> ```
> template<class _IntType>
> template<class _URNG>
> typename uniform_int_distribution<_IntType>::result_type
> uniform_int_distribution<_IntType>::operator()(_URNG& __g, const param_type& __p)
> {
>     typedef typename conditional<sizeof(result_type) <= sizeof(uint32_t),
>                                             uint32_t, uint64_t>::type _UIntType;
>     const _UIntType _Rp = __p.b() - __p.a() + _UIntType(1);
>     if (_Rp == 1)
>         return __p.a();
>     const size_t _Dt = numeric_limits<_UIntType>::digits;
>     typedef __independent_bits_engine<_URNG, _UIntType> _Eng;
>     if (_Rp == 0)
>         return static_cast<result_type>(_Eng(__g, _Dt)());
>     size_t __w = _Dt - __clz(_Rp) - 1;
>     if ((_Rp & (_UIntType(~0) >> (_Dt - __w))) != 0)
>         ++__w;
>     _Eng __e(__g, __w);
>     _UIntType __u;
>     do
>     {
>         __u = __e();
>     } while (__u >= _Rp);
>     return static_cast<result_type>(__u + __p.a());
> }
> ```
> The duplication is unfortunate, but generating the same sequence independently of stdlib implementation kind of makes duplication a requirement.
Sorry, I missed your comment. Somehow it slipped by my email.

Yes, I think we could use libc++'s implementation (from the spec it seems that independent_bits_engine should be stable). However, I think the only thing we really gain with this is efficiency. The libc++ implementation is faster, at the expense of readability and code size.

I'm happy to integrate this optimized implementation into the current RNG class, but I'd be adverse to creating a completely duplicate uniform_int_distribution utility class. That seems like a lot of code duplication for a simple feature.

http://reviews.llvm.org/D6983

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list