RFR 8144534: Refactor templateInterpreter and templateInterpreterGenerator functions

[Christian Thalinger]

> 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.

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
>>> 
>