RFR: 8257815: Replace global log2 functions with efficient implementations [v6]

Stefan Karlsson stefank at openjdk.java.net
Wed Dec 9 12:10:38 UTC 2020


On Wed, 9 Dec 2020 11:15:32 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> src/hotspot/share/compiler/compilerDefinitions.cpp line 120:
>> 
>>> 118:   // one bit shorter then the value of the notification frequency. Set
>>> 119:   // max_freq_bits accordingly.
>>> 120:   int max_freq_bits = InvocationCounter::number_of_count_bits + 1;
>> 
>> Why was the type changed?
>
> Presumably the type change is so MIN2 can be used on line 127.  And there's no obvious reason why the bit counts here should be intx.

The reason is that ilog2 returns int. I incorrectly reasoned that ilog2 would use the same return type as the input argument.

>> src/hotspot/share/gc/g1/g1FreeIdSet.cpp line 47:
>> 
>>> 45:   // 2^shift must be greater than size. Equal is not permitted, because
>>> 46:   // size is the "end of list" value, and can be the index part of _head.
>>> 47:   uint shift = ilog2(size) + 1;
>> 
>> It's not obvious that it's OK to change the type here. Could you change back so that we don't have to figure that out in this review?
>
> The type of `size` is manifestly `uint`.  The cast doesn't do anything useful, and I have no idea why it's there, even though it looks like I wrote this code.  It certainly seems out of character for me.  And I wonder why I didn't use `log2_uint`?  Maybe that was added later?

I agree. This seems fine.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1663


More information about the hotspot-dev mailing list