RFR: 8234331: Add robust and optimized utility for rounding up to next power of two+

[Claes Redestad]

Hi Erik,

thanks for looking at this.

On 2019-12-05 18:49, Erik Österlund wrote:
> Hi Claes,
> 
> I wonder what issue you ran into with std::numeric_limit on Solaris. I 
> would personally like to understand that
> before dismissing its use and rolling our own function instead.
> Note that std::numeric_limit does not accept CV qualified types - 
> perhaps that is what you ran into.

slowdebug failed to build with the following:

[2019-12-05T01:34:22,502Z] Undefined            first referenced
[2019-12-05T01:34:22,502Z]  symbol                  in file
[2019-12-05T01:34:22,502Z] unsigned
std::_Integer_limits<unsigned,0U,4294967295U,-1,true>::max()

> 
> Perhaps if you pass in RemoveCv<T>::type instead of T to that trait, 
> things will work properly.
> If you really want to note use numeric_limit, then you might have to 
> stick in some more sanity checks in
> that functions, such as checking that the input is integral, etc. And 
> dynamically calculating the limit
> does seem unfortunate. In that case I'd rather we rolled our own numeric 
> limit trait, but I would be
> surprised if that is indeed necessary.

I can try the RemoveCv<T> trick. This "private" max_value utility is not
performance critical since it's only used for an assert and in tests,
but yes it'd be much preferable if we could use std::numeric_limits<..>

> 
> Other than that, I would just like to say I approve of having this stuff 
> in a separate file, as it includes
> a whole bunch of metaprogramming utilities, which I would not like to be 
> added from the #include "allTheRandomStuff"
> header of our JVM, that is globalDefinitions.hpp.

Yes, in this particular case wedging these utilities into
globalDefinitions is tricky. In fact impossible unless we also throw in
count_leading_zeros.hpp there too. I do agree with others that some
functions could be consolidated into bigger lumps.

> 
> Other than that, I think this looks good. Oh - not sure but I have a gut 
> feeling you can remove a few includes
> for zUtils that were only previously used to get to the power of two 
> utility, which is no longer necessary.

Good point, I'll check.

Thanks!

/Claes