RFR: 8179004: Add an efficient implementation of the "count trailing zeros" operation
kim.barrett at oracle.com
Fri Apr 21 22:41:35 UTC 2017
> On Apr 21, 2017, at 2:21 PM, Chris Plummer <chris.plummer at oracle.com> wrote:
> On 4/21/17 10:41 AM, Kim Barrett wrote:
>> * 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.)
> I also had this thought before reading your comment above. If your code was all toolchain independent, would you still consider putting it in globalDefintions.hpp? If not, I don't necessarily think that it makes sense to put it there just to piggyback on globalDefinitions.hpp the <tool> logic. I skimmed through globalDefintions.hpp and couldn't get enough a good feel for whether or not your API belongs there.
globalDefinitions.hpp contains a lot of little utility functions, and
this one (and some related ones that I'm not (yet) proposing) fall
into that category. So I think it fits. OTOH, I pretty strongly
dislike files like globalDefinitions.hpp. If it were up to me, that
file would get some significant refactoring, with several new files
being pulled out of it.
But the noted structural issues make this function not fit in there
very nicely at present.
>>> 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?
> Sure. FYI I'm not a (R)eviewer.
OK. Thanks for the review.
More information about the hotspot-dev