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

[Volker Simonis]

Hi Kim,

thanks for implementing the AIX part of the change!
I've compiled and tested it and it works fine.
Although we currently have no 32-bit port on AIX I think you could
leave the 32-bit variant in for reference (and to help the brave
developer who starts the 32-bit version of the AIX port :)

Thank you and best regards,
Volker


On Fri, Apr 21, 2017 at 7:41 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>> 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?
>