RFR(M): 8042737 : Introduce umbrella header prefetch.inline.hpp

[Christian Thalinger]

Alright.  I will push this to hs-comp.

On May 19, 2014, at 8:18 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:

> On 2014-05-19 16:43, Lindenmaier, Goetz wrote:
>> Hi Stefan,
>> 
>> thanks for the comments!
>> I fixed compactibelFreeListSpace.cpp and heapRegion.cpp.
>> I also saw another usage of Prefetch in klass.hpp.  I removed
>> the macro altogether as it's unused.
>> 
>> I kept the include in precompiled.hpp though, as for other
>> files it's listed there, too.  If you insist, and
>> there are no other objections, I'll remove it.
> 
> OK.
> 
>> 
>> The new webrev is at
>> http://cr.openjdk.java.net/~goetz/webrevs/8042737-prefIncl/webrev.01
> 
> Looks good.
> 
> thanks,
> StefanK
>> 
>> Best regards,
>>   Goetz.
>> 
>> -----Original Message-----
>> From: Stefan Karlsson [mailto:stefan.karlsson at oracle.com]
>> Sent: Montag, 19. Mai 2014 13:21
>> To: Lindenmaier, Goetz; Christian Thalinger
>> Cc: hotspot-dev at openjdk.java.net
>> Subject: Re: RFR(M): 8042737 : Introduce umbrella header prefetch.inline.hpp
>> 
>> Hi Goetz,
>> 
>> On 2014-05-19 12:58, Lindenmaier, Goetz wrote:
>>> Hi Christian,
>>> 
>>> thanks for the review!
>>> 
>>> Although prefetch is not used that often, I think it's good
>>> that all headers in the os_cpu directories are handled
>>> similarly now, i.e., they are only included in the respective
>>> shared file.  (For some it's the file in the /cpu/ dir, like
>>> copy_linux_x86.inline.hpp.)
>>> 
>>> Could you sponsor the change?  Do I need a second reviewer?
>> Thanks for cleaning this up.
>> 
>> Some minor comments:
>> 
>> http://cr.openjdk.java.net/~goetz/webrevs/8042737-prefIncl/webrev.00/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp.frames.html
>> 
>> You shouldn't have to include prefetch.inline.hpp from this file.
>> 
>> 
>> http://cr.openjdk.java.net/~goetz/webrevs/8042737-prefIncl/webrev.00/src/share/vm/memory/space.inline.hpp.frames.html
>> 
>> heapRegion.cpp needs to include space.inline.hpp since it uses the
>> SCAN_AND_FORWARD macro.
>> 
>> 
>> http://cr.openjdk.java.net/~goetz/webrevs/8042737-prefIncl/webrev.00/src/share/vm/precompiled/precompiled.hpp.frames.html
>> 
>> I don't think its necessary to add the prefetch.inline.hpp to
>> precompiled.hpp.
>> 
>> 
>> Otherwise, this looks good to me.
>> 
>> thanks,
>> StefanK
>> 
>>> Best regards,
>>>    Goetz.
>>> 
>>> 
>>> 
>>> -----Original Message-----
>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>> Sent: Donnerstag, 15. Mai 2014 17:14
>>> To: Lindenmaier, Goetz
>>> Cc: hotspot-dev at openjdk.java.net
>>> Subject: Re: RFR(M): 8042737 : Introduce umbrella header prefetch.inline.hpp
>>> 
>>> Looks good to me.  I thought that prefetch is used more often but apparently not.
>>> 
>>> On May 15, 2014, at 1:16 AM, Lindenmaier, Goetz <goetz.lindenmaier at sap.com> wrote:
>>> 
>>>> Hi,
>>>> 
>>>> This change introduces an umbrella header for prefetch_<os>_<cpu>.inline.hpp
>>>> files: runtime/prefetch.inline.hpp as proposed in
>>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-April/013702.html
>>>> This follows the scheme applied to atomic.inline.hpp.
>>>> 
>>>> Further this change adds includes of prefetch.inline.hpp in all .cpp
>>>> and .inline.hpp files where a method of Prefetch declared 'inline' is
>>>> called.
>>>> 
>>>> Finally it moves macros calling inline methods of Prefetch from
>>>> space.hpp to the corresponding .inline.hpp file and adds the necessary
>>>> includes in files using the moved macros.
>>>> 
>>>> Please review and test this change.  I please need a sponsor.
>>>> http://cr.openjdk.java.net/~goetz/webrevs/8042737-prefIncl/webrev.00/
>>>> 
>>>> Thanks and best regards,
>>>>   Goetz.
>