Hi Pengfei, On 29/11/2019 03:56, Pengfei Li (Arm Technology China) wrote:
> Please help review this small fix for 64-bit client build. > > Webrev: http://cr.openjdk.java.net/~pli/rfr/8234791/webrev.00/ > JBS: https://bugs.openjdk.java.net/browse/JDK-8234791 > > Current 64-bit client VM build fails because errors occurred in dumping > the CDS archive. In JDK 12, we enabled "Default CDS Archives"[1] which > runs "java -Xshare:dump" after linking the JDK image. But for Client VM > build on 64-bit platforms, the ergonomic flag UseCompressedOops is not > set.[2] This leads to VM exits in checking the flags for dumping the > shared archive.[3] > > This change removes the "#if defined" macro to make shared archive dump > successful in 64-bit client build. By tracking the history of the macro, > I found it is initially added as "#ifndef COMPILER1"[4] 10 years ago > when C1 did not have a good support of compressed oops and modified to > current shape[5] in the implementation of tiered compilation. It should > be safe to be removed today. > > This patch also fixes another client build issue on AArch64. > > [1] http://openjdk.java.net/jeps/341 > [2] > http://hg.openjdk.java.net/jdk/jdk/file/981a55672786/src/hotspot/share/runtime/arguments.cpp#l1694 > [3] > http://hg.openjdk.java.net/jdk/jdk/file/981a55672786/src/hotspot/share/runtime/arguments.cpp#l3551 > [4] http://hg.openjdk.java.net/jdk8/jdk8/hotspot/rev/323bd24c6520#l11.7 > [5] http://hg.openjdk.java.net/jdk8/jdk8/hotspot/rev/d5d065957597#l86.56 Your explanation sounds correct and the change to arguments.cpp looks good. Can you explain why you have modified sharedRuntime_aarch64.cpp to include nativeInst_aarch64.hpp? I don't see any other change in the source file that would make this necessary. 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
