merge vm_version_x86_{32,64}.hpp

Paul Hohensee Paul.Hohensee at Sun.COM
Mon Feb 23 04:29:02 PST 2009


Imo, the #ifdef's are ok in this case because they're quite limited.

Vladimir set prefetch distances based on experiment.  The difference
between 320 and 384 is a single cache line.  It seems to me that 32
and 64-bit would be the same because it's the same hardware, except
that it may be that 64-bit code is faster than 32-bit code, thus would
require a slightly greater prefetch distance.

32-bit doesn't use the Prefetchxxx switches because there's no common
prefetch instruction for 32-bit that can be used in C++ code.  #ifdef is
ok here.

Paul

Christian Thalinger wrote:
> On Mon, 2009-02-23 at 08:49 +0100, Christian Thalinger wrote:
>   
>> On Mon, 2009-02-23 at 10:10 +1000, David Holmes - Sun Microsystems
>> wrote:
>>     
>>> Hi Vladimir,
>>>
>>> Christian's proposal was to take the existing four files:
>>>
>>>  > vm_version_x86_32.cpp, vm_version_x86_32.hpp,
>>>  > vm_version_x86_64.cpp, vm_version_x86_64.hpp
>>>
>>> and to factor out the common code into:
>>>
>>>  > vm_version_x86.cpp and vm_version_x86.hpp
>>>
>>> the question asked was then whether to deal with the platform specific 
>>> code by:
>>>
>>> a) keeping the existing 4 files in addition to the 2 new ones (assuming 
>>> all four are still needed); or
>>>
>>> b) use ifdef's in the new files and discard the 4 old ones
>>>       
>
> Okay, there are way too less differences to keep the old specific files
> around.  There are only two differences I'm not sure about:
>
> @@ -418,31 +355,32 @@
>  
>    if( AllocatePrefetchStyle == 2 && is_intel() &&
>        cpu_family() == 6 && supports_sse3() ) { // watermark prefetching on Core
> -    AllocatePrefetchDistance = 320;
> +    AllocatePrefetchDistance = 384;
>    }
>    assert(AllocatePrefetchDistance % AllocatePrefetchStepSize == 0, "invalid value");
>  
> +  // Prefetch settings
> +  PrefetchCopyIntervalInBytes = prefetch_copy_interval_in_bytes();
> +  PrefetchScanIntervalInBytes = prefetch_scan_interval_in_bytes();
> +  PrefetchFieldsAhead         = prefetch_fields_ahead();
> +
>
> For the first one, I'm not sure which value to choose or if they should
> be different on 32 and 64-bit?
>
> And the second, I'm not sure I can set them on 32-bit.  Per default
> these are -1.
>
> Besides that, there's only one real difference:
>
> +  // OS should support SSE for x64 and hardware should support at least SSE2.
> +  if (!VM_Version::supports_sse2()) {
> +    vm_exit_during_initialization("Unknown x64 processor: SSE2 not supported");
>    }
>
> Is that check really necessary on x64?  If so, I simply put it inside an
> #ifdef.
>
> -- Christian
>
>   



More information about the hotspot-dev mailing list