RFR: 8224974: Implement JEP 352

[Andrew Dinn]

Hi Boris,

On 31/07/2019 13:01, Boris Ulasevich wrote:
> I did a quick check of the change across our platforms. Arm32 and x86_64
> built successfully. But I see it fails due to minor issues on aarch64
> and x86_32 with webrev.09.
> Can you please have a look at this?
> 
>> src/hotspot/cpu/aarch64/aarch64.ad:2202:1: error: expected ‘;’ before
> ‘}’ token
>> src/hotspot/cpu/x86/macroAssembler_x86.cpp:9925: undefined reference
> to `Assembler::clflush(Address)'
The AArch64 error was simply a missing semi-colon. With that corrected
AArch64 now builds and runs as expected (i.e. it fails the PMem support
test with an UnsupportedOperationException).

The second error is happening because the calling method
MacroAssembler::cache_wb has not been guarded with #ifdef _LP64 (the
same applies for MacroAssembler::cache_wbsync). Note that cache line
writeback via Unsafe.writeBackMemory is only expected to work on Intel
x86_64 so these two methods only get called from x86_64-specific code
(x86_64.ad and stuGenerator_x86_64.cpp).

So, the solution to this specific problem is to add #ifdef _LP64 around
the declaration and implementation of these two methods. At the same
time it would be helpful to remove the redundant #ifdef _LP64/#endif
that I for some strange reason inserted around the definitions, but not
the declarations, of clflushopt and clwb (that didn't help when I was
trying to work out what was going wrong).

However, a related problem also needs fixing. The Java code for method
Unsafe.writebackMemory only proceeds when the data cache line writeback
unit size (value of field UnsafeConstants.DATA_CACHE_LINE_FLUSH_SIZE) is
non-zero. Otherwise it throws an exception. On x86_32 that field /must/
be zero. The native methods which Unsafe calls out to and the intrinsics
which replace the native calls are not implemented on x86_32.

The field from which the value of the Java constant is derived is
currently initialised using CPU-specific information in
vm_version_x86.cpp as follows

   if (os::supports_map_sync()) {
     // publish data cache line flush size to generic field, otherwise
     // let if default to zero thereby disabling writeback
     _data_cache_line_flush_size =
_cpuid_info.std_cpuid1_ebx.bits.clflush_size * 8;
   }

i.e. writeback is enabled on x86 when the operating is known to be
capable of supporting MAP_SYNC. os_linux.cpp returns true for that call,
irrespective of whether this is 32 or 64 bit linux. The rationale is
that any Linux is capable of supporting map_sync (by contrast Windows,
Solaris, AIX etc currently return false). So, the above assignment also
needs to be guarded by #ifdef _LP64 in order to ensure that writeback is
never attempted on x86_32.

Thank you for spotting these errors. I will add the relevant fixes to
the next patch and add you as a reviewer.

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander