Hi Matthias, your changes look good to me. Please note, however, that I'm not a Reviewer.
One minor thing: do you really want to keep the old code (as comment) in sharedRuntime_ppc.cpp:2853? Thanks for putting this straight. Regards, Lutz On 29.10.19, 14:32, "hotspot-dev on behalf of Baesken, Matthias" <[email protected] on behalf of [email protected]> wrote: Thanks . May I have a second review please ? Best regards, Matthias From: Doerr, Martin <[email protected]> Sent: Dienstag, 29. Oktober 2019 13:48 To: Baesken, Matthias <[email protected]>; '[email protected]' <[email protected]> Cc: '[email protected]' <[email protected]> Subject: RE: RFR: 8233078 : fix minimal VM build on Linux ppc64(le) Hi Matthias, > Not sure if there are any plans to support OptimizeFill on ppc64 ? This question is not related to this issue. Commenting out parts of it is not a good style. Thank you for your update. The new webrev looks good to me. Best regards, Martin From: Baesken, Matthias <[email protected]<mailto:[email protected]>> Sent: Dienstag, 29. Oktober 2019 13:25 To: Doerr, Martin <[email protected]<mailto:[email protected]>>; '[email protected]' <[email protected]<mailto:[email protected]>> Cc: '[email protected]' <[email protected]<mailto:[email protected]>> Subject: RE: RFR: 8233078 : fix minimal VM build on Linux ppc64(le) Hi Martin, thanks for the input . I did the adjustments you suggested; new webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8233078.1/ Regarding : stubGenerator_ppc.cpp: "Code should better be protected by #ifdef COMPILER2 than commenting out." Currently the if (OptimizeFill) { ... } coding is dead on ppc . See : c2_globals.hpp ------------------------ 234 /* OptimizeFill not yet supported on PowerPC. */ \ 235 product(bool, OptimizeFill, true PPC64_ONLY(&& false), \ c2_init_ppc.cpp ------------------------ 53 if (OptimizeFill) { 54 warning("OptimizeFill is not supported on this CPU."); 55 FLAG_SET_DEFAULT(OptimizeFill, false); Not sure if there are any plans to support OptimizeFill on ppc64 ? Best regards, Matthias Hi Matthias, thanks for fixing it. I have a few requests: disassembler_ppc.cpp: Please remove includes completely if no longer needed (instead of commenting out). sharedRuntime_ppc.cpp: I think it's better to remove the 2 align(InteriorEntryAlignment). Succeeding code is not performance critical. stubGenerator_ppc.cpp: Code should better be protected by #ifdef COMPILER2 than commenting out. Otherwise, looks good to me. Thanks, Martin From: Baesken, Matthias <[email protected]<mailto:[email protected]>> Sent: Dienstag, 29. Oktober 2019 12:42 To: '[email protected]' <[email protected]<mailto:[email protected]>> Cc: '[email protected]' <[email protected]<mailto:[email protected]>>; Doerr, Martin <[email protected]<mailto:[email protected]>> Subject: RFR: 8233078 : fix minimal VM build on Linux ppc64(le) Hello, please review the following fix . I recently experimented a bit with the minimal VM build on linux x86_64 (--with-jvm-features=minimal --with-jvm-variants=minimal) . This worked fine . However when I tried the minimal vm build on linux ppc64 / ppc64le , I noticed that it fails because of a few wrong dependencies . Thanks to Martin for the advice regarding Register ic = as_Register(Matcher::inline_cache_reg_encode()); Replacement with Register ic = R19_inline_cache_reg; In http://cr.openjdk.java.net/~mbaesken/webrevs/8233078.0/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp.frames.html Bug/webrev : https://bugs.openjdk.java.net/browse/JDK-8233078 http://cr.openjdk.java.net/~mbaesken/webrevs/8233078.0/ Thanks, Matthias
