Kim, thank you for the review! Comments inline..
> On May 4, 2020, at 3:47 AM, Kim Barrett <kim.barr...@oracle.com> wrote: > >> On May 4, 2020, at 1:12 AM, Mikael Vidstedt <mikael.vidst...@oracle.com> >> 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 > > I've only looked at the src/hotspot changes so far. I've not > duplicated comments already made by Stefan. > > Looks good, other than a few very minor issues, some of which might > already be covered by planned followup RFEs. > > ------------------------------------------------------------------------------ > > I think with sparc removal, c1's pack64/unpack64 stuff is no longer > used. So I think that can be removed from c1_LIR.[ch]pp too. Good catch. Fixed. > ------------------------------------------------------------------------------ > src/hotspot/share/opto/generateOptoStub.cpp > 225 // Clear last_Java_pc and (optionally)_flags > > The sparc-specific clearing of "flags" is gone. Fixed. > ------------------------------------------------------------------------------ > src/hotspot/share/runtime/deoptimization.cpp > 1086 *((jlong *) check_alignment_get_addr(obj, index, 8)) = (jlong) > *((jlong *) &val); > > [pre-existing] > The rhs cast to jlong is unnecessary, since it's dereferencing a jlong*. When I first updated the code I actually remove the cast, but it just ended up looking asymmetrical so I chose to leave it there. Let me know if you feel strongly that it should go. (I don’t like these casts in general). > ------------------------------------------------------------------------------ > src/hotspot/share/runtime/flags/jvmFlagConstraintsCompiler.cpp > 236 JVMFlag::Error CompilerThreadPriorityConstraintFunc(intx value, bool > verbose) { > 237 return JVMFlag::SUCCESS; > 238 } > > After SOLARIS code removal we no longer need this constraint function. Fixed. (I had that on my follow-up list, but included it in the upcoming webrev.) > ------------------------------------------------------------------------------ > src/hotspot/share/runtime/globals.hpp > 2392 experimental(size_t, ArrayAllocatorMallocLimit, > \ > 2393 (size_t)-1, > \ > > Combine these lines. Fixed. > ------------------------------------------------------------------------------ > src/hotspot/share/utilities/dtrace.hpp > > Shuold just eliminate all traces of HS_DTRACE_WORKAROUND_TAIL_CALL_BUG. Fixed - more code removed! Cheers, Mikael
