RFR: Make Zero build and run with JDK8

[Roman Kennke]

Am Mittwoch, den 17.10.2012, 15:34 -0700 schrieb Christian Thalinger:
> On Oct 17, 2012, at 3:05 PM, Roman Kennke <rkennke at redhat.com> wrote:
> 
> > Am Mittwoch, den 17.10.2012, 14:40 -0700 schrieb Christian Thalinger:
> >> On Oct 16, 2012, at 8:01 AM, Roman Kennke <rkennke at redhat.com> wrote:
> >> 
> >>> Hi Christian,
> >>> 
> >>>>>>>> In the recent weeks I worked on the Zero interpreter, to get it to build
> >>>>>>>> and run with JDK8, and in particular with the latest changes that came
> >>>>>>>> from mlvm (meth-lazy). The following webrev applies to hsx/hotspot-main:
> >>>>>>>> 
> >>>>>>>> http://cr.openjdk.java.net/~rkennke/zerojdk8/webrev.00/
> >>>>>> 
> >>>>>> src/share/vm/asm/codeBuffer.cpp:
> >>>>>> 
> >>>>>> -  if (dest->blob() == NULL) {
> >>>>>> +  if (dest->blob() == NULL && dest_filled != 0x0) {
> >>>>>> 
> >>>>>> Do we really need this when you have this:
> >>>>> 
> >>>>> The above is needed, because the loop above it that initializes
> >>>>> dest_filled is never executed. However, I will change the test to
> >>>>> dest_filled != NULL :-)
> >>>>> 
> >>>>>> static void pd_fill_to_bytes(void* to, size_t count, jubyte value) {
> >>>>>> 
> >>>>>> -  memset(to, value, count);
> >>>>>> +  if ( count > 0 ) memset(to, value, count);
> >>>>>> 
> >>>>>> }
> >>>>> 
> >>>>> Turns out that this is 1. not related to the other change above and 2.
> >>>>> not needed. I'll remove it.
> >>>>> 
> >>>>>> src/share/vm/interpreter/interpreter.cpp:
> >>>>>> 
> >>>>>> -  assert(_entry_table[kind] == _entry_table[abstract], "previous value must be AME entry");
> >>>>>> +  assert(_entry_table[kind] == NULL || _entry_table[kind] == _entry_table[abstract], "previous value must be AME entry or NULL");
> >>>>>> 
> >>>>>> Why did you need that change?
> >>>>> 
> >>>>> Apparently, before the whole table was initialized, and the methodhandle
> >>>>> related entries left as abstract. Now the methodhandle entries are
> >>>>> simply left to NULL. I suppose we could change the assert to 
> >>>>> 
> >>>>> assert(_entry_table[kind] == NULL, "previous value must be NULL");
> >>>>> 
> >>>>> Alternatively, we could fix the code that puts the other entries to also
> >>>>> set the methodhandle entries to AME and go back to the original assert.
> >>>>> What do you think?
> >>>> 
> >>>> TemplateInterpreterGenerator::generate_all sets all MH entries to AME:
> >>>> 
> >>>> // method handle entry kinds are generated later in MethodHandlesAdapterGenerator::generate:                                                                  
> >>>> for (int i = Interpreter::method_handle_invoke_FIRST; i <= Interpreter::method_handle_invoke_LAST; i++) {                                                      
> >>>>   Interpreter::MethodKind kind = (Interpreter::MethodKind) i;                                                                                                  
> >>>>   Interpreter::_entry_table[kind] = Interpreter::_entry_table[Interpreter::abstract];                                                                          
> >>>> }                                                                                                                                                              
> >>>> 
> >>>> You need similar code in Zero.
> >>> 
> >>> Alright, I extracted this piece of code into a separate protected method
> >>> void AbstractInterpreterGenerator::initializeMethodHandleEntries() and
> >>> call it both from templateInterpreter and cppInterpreter to initialize
> >>> the method handle entries to AME.
> >>> 
> >>> This new webrev also reverts this:
> >>> 
> >>>>> static void pd_fill_to_bytes(void* to, size_t count, jubyte value) {
> >>>>>> 
> >>>>>> -  memset(to, value, count);
> >>>>>> +  if ( count > 0 ) memset(to, value, count);
> >>>>>> 
> >>>>>> }
> >>>> 
> >>> 
> >>> .. and changes the 0x0 to NULL here:
> >>> 
> >>> 
> >>>>> src/share/vm/asm/codeBuffer.cpp:
> >>>>>> 
> >>>>>> -  if (dest->blob() == NULL) {
> >>>>>> +  if (dest->blob() == NULL && dest_filled != 0x0) {
> >>>>>> 
> >>>> 
> >>> 
> >>> I tried JRuby a little more and verified that it's actually using +indy,
> >>> and it works all well.
> >>> 
> >>> http://cr.openjdk.java.net/~rkennke/zerojdk8/webrev.01/
> >> 
> >> It seems this webrev contains the old changes.
> > 
> > Arg! I did the changes in my hotspot-comp tree then made the webrev in
> > my hotspot-main :-) Here's the correct one:
> > 
> > http://cr.openjdk.java.net/~rkennke/zerojdk8/webrev.02/
> 
> That looks better.  The only thing is we don't use camel-case for methods:
> 
> +  void initializeMethodHandleEntries();
> 
> Could you change it to:
> 
> +  void initialize_method_handle_entries();
> 
> I would do it myself but I cannot verify that I didn't break Zero.  I really should set up a build environment for Zero...

Here it comes:

http://cr.openjdk.java.net/~rkennke/zerojdk8/webrev.03/

I built both Zero and normal debug_build successfully.

Cheers,
Roman