Request for reviews (M): 7079329: Adjust allocation prefetching for T4

[Christian Thalinger]

On Aug 16, 2011, at 5:01 PM, Vladimir Kozlov wrote:

> On 8/16/11 2:29 AM, Christian Thalinger wrote:
>> 
>> On Aug 16, 2011, at 3:12 AM, Vladimir Kozlov wrote:
>> 
>>> http://cr.openjdk.java.net/~kvn/7079329/webrev
>>> 
>>> 7079329: Adjust allocation prefetching for T4
>>> 
>>> L2 cache line size is 32 bytes on T4 instead of 64 bytes on T series before. As result BIS instruction prefetches only 32 bytes. Jbb2005 runs show that prefetching 64 bytes is still better on T4 so 2 BIS instructions should be issued.
>>> 
>>> BIS can't be use for general prefetching since it may fault. New PrefetchAllocation node was added for allocation prefetching.
>>> 
>>> Changed prefetchAlloc_bis parameter from memory to regP.
>>> 
>>> Use AllocatePrefetchInstr on Sparc to allow specify what instruction to use for allocation prefetching (0: prefetch write, 1: BIS).
>>> 
>>> Added new instructions on Sparc cacheLineAdrX to reduce number of instructions generated for finding next cache line address.
>>> 
>>> Added new flag AllocateInstPrefetchLines to specify number of lines to prefetch for instance allocation.
>>> 
>>> L1_data_cache_line_size() renamed to prefetch_data_size().
>> 
>> src/cpu/x86/vm/x86_32.ad:
>> src/cpu/x86/vm/x86_64.ad:
>> 
>> Can you use MacroAssembler instructions to emit the code for the new instructs?
> 
> OK.
> 
>> 
>> src/cpu/sparc/vm/vm_version_sparc.cpp:
>> 
>> +       if (is_T4()) {
>> +         // Double number of prefetched cache lines on T4
>> +         // since L2 cache line size is smaller (32 bytes).
>> +         if (FLAG_IS_DEFAULT(AllocatePrefetchLines)) {
>> +           FLAG_SET_DEFAULT(AllocatePrefetchLines, 6);
>> +         }
>> +         if (FLAG_IS_DEFAULT(AllocateInstPrefetchLines)) {
>> +           FLAG_SET_DEFAULT(AllocateInstPrefetchLines, 2);
>> +         }
>> +       }
>> 
>> Maybe you should use *2 here.
> 
> Something like this?:
> 
> +         if (FLAG_IS_DEFAULT(AllocatePrefetchLines)) {
> +           FLAG_SET_DEFAULT(AllocatePrefetchLines, AllocatePrefetchLines*2);
> +         }

Yes, that makes more sense to me.

-- Christian

> 
> Vladimir
> 
>> 
>> Otherwise this looks good.
>> 
>> -- Christian