RFR: 8179004: Add an efficient implementation of the "count trailing zeros" operation

Kim Barrett kim.barrett at oracle.com
Fri Apr 21 17:41:14 UTC 2017


> On Apr 21, 2017, at 1:54 AM, Chris Plummer <chris.plummer at oracle.com> wrote:
> 
> On 4/19/17 11:17 PM, Kim Barrett wrote:
>> Please review this addition of the count_trailing_zeros function.
>> 
>> […]
>> As part of reviewing this change, please feel free to suggest
>> alternative ways to structure the code. I'm not completely happy with
>> the way I've done it, but alternatives I've tried seemed worse.
> Are you talking about the #if/#elseif structure and the ////// delimiters. I'd suggest replacing the ////// with a block comment that stands out. Something like:
> 
> /**************
> * GCC Support
> **************/

Sure, that makes things stand out a bit more.

Updated webrev (only comment changes in count_trailing_zeros.hpp)
http://cr.openjdk.java.net/~kbarrett/8179004/hotspot.01/

What I'm looking for is whether this TARGET_COMPILER dispatch with the
corresponding code is okay, or whether some other structure would be
preferred.

Some alternatives I considered were:

* Always #include OS_CPU_INCLUDE(count_trailing_zeros). But that
proliforates files which will be duplicates (possibly duplicates
by #include of some shared toolchain-related include).

* Dispatch to #include "utilities/count_trailing_zeros_<tool>.hpp"
similarly to what globalDefinitions.hpp does.  This adds 4 small
files, which seems a little excessive.

* Put these toolchain-specific definitions in the corresponding
globalDefintions_<tool>.hpp. But the #include of those files by
globalDefinitions.hpp happens very early in that file, before things
like the uintx type are defined. Also, presently debug.hpp can't
be #include'd by globalDefinitions.hpp, so the latter does similar
things "by hand". (I think that's something that ought to be fixed.)

> Otherwise it looks fine to me, but I'm no expert in this area. The testing looks satisfactory and the bitmap results you are getting are reason enough to add this functionality.

Thanks.  Do you want to be counted as a reviewer?



More information about the hotspot-dev mailing list