RFR 8144534: Refactor templateInterpreter and templateInterpreterGenerator functions

[Christian Thalinger]

> On Dec 4, 2015, at 7:48 AM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
> 
>> 
>> On Dec 3, 2015, at 1:51 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
>> 
>> 
>> Hi Goetz,
>> 
>> Thank you for looking at my change.  I made the ppc changes and hg added templateInterpreter_ppc/aarch64.cpp back, so they should be in the patch.
>> 
>> Now webrev thinks I've deleted 7785 lines, but I didn't really.
>> 
>> http://cr.openjdk.java.net/~coleenp/8144534.02/
>> 
>> The most interesting part of this change is in the x86 code:
>> 
>> http://cr.openjdk.java.net/~coleenp/8144534.02/src/cpu/x86/vm/templateInterpreterGenerator_x86.cpp.udiff.html
>> 
>> This is where the 32 and 64 bit template interpreter code is merged.
> 
> +   Register rarg = NOT_LP64(rax) LP64_ONLY(c_rarg1);
> +   Register rarg2 = NOT_LP64(rbx) LP64_ONLY(c_rarg2);
> 
> Aren’t these always the same and could also be defined globally?  Maybe keep rarg1.
> 
> !   // compute beginning of parameters (rdi/r14)
> !   __ lea(rlocals, Address(rsp, rcx, Interpreter::stackElementScale(), -wordSize));
> 
> You can remove the rdi/r14 part.
> 
> !   // restore rsi/r13 to have legal interpreter frame, i.e., bci == 0 <=>
>    // r13 == code_base()
> 
>    // compute beginning of parameters (r14)
> 
> Same for these.
> 
>      ExternalAddress unsatisfied(SharedRuntime::native_method_throw_unsatisfied_link_error_entry());
> !     __ movptr(rscratch2, unsatisfied.addr());
>      __ cmpptr(rax, rscratch2);
> !     __ cmpptr(rax, unsatisfied.addr());
> 
> I think this is wrong.  On 64-bit cmp can’t take a 64-bit immediate.  That’s why we have the move.

Maybe not.  I should have looked at cmpptr first.  Seems like it’s handling all this.  Truly a macro instruction….

> 
> Otherwise this looks good but I’ve only looked at this one file.
> 
>> 
>> Thanks,
>> Coleen
>> 
>> On 12/3/15 8:14 AM, Lindenmaier, Goetz wrote:
>>> Hi Coleen,
>>> 
>>> I've been looking at this change. A cleanup in this area is really
>>> useful.
>>> I missed templateInterpreter_ppc.cpp in your patch, I guess you forgot
>>> "hg add" for it.
>>> In addition, I had to fix some code around math_entry_available(), see this patch:
>>> http://cr.openjdk.java.net/~goetz/webrevs/8144534-interp/8144534-fixes_for_ppc.patch
>>> 
>>> In general, I think it's strange that AbstractInterpreterGenerator
>>> implementations are in interpreter_<cpu>.cpp, as mentioned In the bug.
>>> Why not move them to templateInterpreterGenerator_<cpu>.cpp? Or will
>>> this be subject to a later change that removes the funny common
>>> subclasses Interpreter/InterpreterGenerator?
>>> 
>>> Should I make a change removing the CC_INTERP support from ppc?
>>> 
>>> Best regards,
>>>  Goetz.
>>> 
>>>> -----Original Message-----
>>>> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On
>>>> Behalf Of Coleen Phillimore
>>>> Sent: Donnerstag, 3. Dezember 2015 02:58
>>>> To: hotspot-dev developers <hotspot-dev at openjdk.java.net>
>>>> Subject: RFR 8144534: Refactor templateInterpreter and
>>>> templateInterpreterGenerator functions
>>>> 
>>>> Summary: merged templateInterpreter_x86_32/64 into
>>>> templateInterpreterGenerator_x86.cpp (some 32/64 functions remain for
>>>> the hand coded crc functions).
>>>> 
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8144534.01/
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8144534
>>>> 
>>>> I tried to make this reviewable by hg renaming files.   I basically
>>>> renamed templateInterpreter_ppc.cpp and removed the
>>>> AbstractInterpreter
>>>> functions and put them back into templateInterpreter_ppc.cpp.  I didn't
>>>> really delete 4000 lines of code, more like 1500.
>>>> 
>>>> Also, can someone with the PPC and AARCH port look at and test out these
>>>> changes? I tried to reduce the #includes in the new
>>>> templateInterpreter_ppc/aarch64.cpp files which worked for sparc.
>>>> 
>>>> Tested with JPRT on all platforms, also ran jtreg and
>>>> collocated/non-collocated tonga tests for linux x64 and i386 since
>>>> that's were the real changes are.
>>>> 
>>>> See the bug for more details and my partial vision of how these
>>>> functions will be reorganized when I remove the broken c++ interpreter.
>>>> 
>>>> Also tested that Zero still builds.
>>>> 
>>>> Thanks!
>>>> Coleen