This patch contains a Makefile change: http://cr.openjdk.java.net/~rkennke/zerojdk8/webrev.03/make/Makefile.udiff.html
-- Chris Begin forwarded message: > From: Roman Kennke <rken...@redhat.com> > Subject: Re: RFR: Make Zero build and run with JDK8 > Date: October 17, 2012 4:09:13 PM PDT > To: Christian Thalinger <christian.thalin...@oracle.com> > Cc: hotspot-...@openjdk.java.net > > Am Mittwoch, den 17.10.2012, 15:34 -0700 schrieb Christian Thalinger: >> On Oct 17, 2012, at 3:05 PM, Roman Kennke <rken...@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 <rken...@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 > >
