[aarch64-port-dev ] RFR(S) 8248660: AArch64: JDK-Windows: Make _clear_cache and _nop portable

[Andrew Haley]

On 8/3/20 11:29 PM, Monica Beckwith wrote:
> Hi all, could I please get a review for the following webrev? I had previously sent this out for feedback and Kim had provided feedback on the usual way to handle such changes: https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-July/009272.html 
> 
> I have made the needful changes and ran them through JTREG for linux-aarch64, windows-aarch64, windows-x86-64, and linux-x86-64.
> 
> Webrev: http://cr.openjdk.java.net/~mbeckwit/8248660/webrev.01/ 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8248660   
> 
> I wanted to highlight a small change that I made: 
> 
> - For void __builtin___clear_cache (void *begin, void *end); the `begin` is inclusive, and `end` is exclusive [1].
> - Hence for icache_linux_aarch64.hpp’s invalidate_word(), I changed the end to `addr + 4` which previously was `addr + 3`. So it now reads: 
> __builtin___clear_cache((char *)addr, (char *)(addr + 4));
> 
> Although there is forced instruction alignment on Arm64, I still felt I should correct the "victimless crime" (as one of our kernel experts puts it. ��)

This is shared code, and I don't know if it works on every target:

--- a/src/hotspot/share/utilities/globalDefinitions_gcc.hpp
+++ b/src/hotspot/share/utilities/globalDefinitions_gcc.hpp
@@ -164,4 +164,7 @@
 //
 #define ATTRIBUTE_ALIGNED(x) __attribute__((aligned(x)))

+// __nop needs volatile so that compiler doesn't optimize it away
+#define NOP() asm volatile ("nop");
+
 #endif // SHARE_UTILITIES_GLOBALDEFINITIONS_GCC_HPP

I suggest you do this in one of the AArch64 headers instead:

#ifdef GCC
#define NOP() asm volatile ("nop");
#else MSVC etc.

Otherwise OK.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671