Vladimir, thank you for the review!

Note that based on Stefan’s comments I have removed the decryptSuffix variable 
in 
src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java
 in the upcoming webrev.

Cheers,
Mikael

> On May 4, 2020, at 12:01 PM, Vladimir Kozlov <vladimir.koz...@oracle.com> 
> wrote:
> 
> JIT, AOT, JVMCI and Graal changes seem fine to me.
> 
> It would be interesting to see shared code execution coverage change.  There 
> are places where we use flags and setting instead of #ifdef SPARC which may 
> not be executed now or executed partially. We may simplify such code too.
> 
> Thanks,
> Vladimir
> 
> On 5/3/20 10:12 PM, Mikael Vidstedt wrote:
>> Please review this change which implements part of JEP 381:
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8244224
>> webrev: 
>> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/hotspot/open/webrev/
>> JEP: https://bugs.openjdk.java.net/browse/JDK-8241787
>> Note: When reviewing this, please be aware that this exercise was 
>> *extremely* mind-numbing, so I appreciate your help reviewing all the 
>> individual changes carefully. You may want to get that coffee cup filled up 
>> (or whatever keeps you awake)!
>> Background:
>> Because of the size of the total patch and wide range of areas touched, this 
>> patch is one out of in total six partial patches which together make up the 
>> necessary changes to remove the Solaris and SPARC ports. The other patches 
>> are being sent out for review to mailing lists appropriate for the 
>> respective areas the touch. An email will be sent to jdk-dev summarizing all 
>> the patches/reviews. To be clear: this patch is *not* in itself complete and 
>> stand-alone - all of the (six) patches are needed to form a complete patch. 
>> Some changes in this patch may look wrong or incomplete unless also looking 
>> at the corresponding changes in other areas.
>> For convenience, I’m including a link below[1] to the full webrev, but in 
>> case you have comments on changes in other areas, outside of the files 
>> included in this thread, please provide those comments directly in the 
>> thread on the appropriate mailing list for that area if possible.
>> In case it helps, the changes were effectively produced by searching for and 
>> updating any code mentioning “solaris", “sparc”, “solstudio”, “sunos”, etc. 
>> More information about the areas impacted can be found in the JEP itself.
>> A big thank you to Igor Ignatyev for helping make the changes to the hotspot 
>> tests!
>> Also, I have a short list of follow-ups which I’m going to look at 
>> separately from this JEP/patch, mainly related to command line options/flags 
>> which are no longer relevant and should be deprecated/obsoleted/removed.
>> Testing:
>> A slightly earlier version of this change successfully passed tier1-8, as 
>> well as client tier1-2. Additional testing will be done after the first 
>> round of reviews has been completed.
>> Cheers,
>> Mikael
>> [1] 
>> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/all/open/webrev/

Reply via email to