8074457: Remove the non-Zero CPP Interpreter

[Coleen Phillimore]

On 12/18/15 3:44 PM, Coleen Phillimore wrote:
>
> Hi Goetz,  Thank you for reviewing this so quickly and testing on 
> PPC.  Does Zero build on PPC too?
>
> On 12/18/15 10:36 AM, Lindenmaier, Goetz wrote:
>> Hi Coleen,
>>
>> I had a look at the shared and ppc files.  I also tested it on ppc, 
>> works
>> fine.
>>
>> Some smaller things:
>> http://cr.openjdk.java.net/~coleenp/8074457/src/share/vm/interpreter/abstractInterpreter.hpp.udiff.html 
>>
>> + #if defined(TARGET_ARCH_ppc) || defined(TARGET_ARCH_sparc)
>> I's more common to use
>> #if defined(PPC) || defined(AARCH64)
>
> I made this change but PPC can be defined with ZERO, so these lines 
> have to exclude ZERO.
>
>>
>> bytecodeInterpreter.hpp:
>> Maybe you want to clean up the #includes at the bottom as
>> you do in the other files?
>
> Yes, I missed this one.
>>
>> cppInterpreterGenerator.hpp
>> #ifdef TARGET_ARCH_zero  --> #ifdef ZERO
>> templateInterpreter.hpp
>> #ifdef TARGET_ARCH_ppc --> #ifdef PPC
>> ... there are more of these.
>
> These ones I changed too.  I didn't change the one in:
>
> http://cr.openjdk.java.net/~coleenp/8074457/src/share/vm/interpreter/templateInterpreter.cpp.udiff.html 
>
>
> because I didn't add it and don't know what tests use it.

I am testing out changing this to #ifdef IA32 (as well as the 
declarations in templateInterpreterGenerator.hpp file).
>
>>
>> templateInterpreter.hpp
>> I think the ppc method math_entry_available() can just be 'inlined'
>> at its callsite, so that the platform special case goes away.
>
> This seemed tricky since it has some semantics you wouldn't want to 
> duplicate for using both in TemplateInterpreter and 
> TemplateInterpreterGenerator functions.  I'd like to just leave this 
> conditional.
>
>>
>> templateInterpreterGenerator.hpp
>> lock_method():
>> I would just add the ppc parameters with default values to the 
>> existing definition
>> of lock_method().
>> unlock_method(): I would place this near lock_method().
>> generate_fixed_frame():
>> generate_stack_overflow_check()
>> Here also I would add the parameters to the existing function with 
>> default
>> values.
>
> I thought of doing this, but recently got burned by default parameters 
> so I don't like them anymore.   And I didn't want to change the other 
> platforms to include the default parameters that aren't used in the 
> definition of the functions.   I thought the small set of conditionals 
> was a small price to pay so I'd like to leave these too.
>
>
> See new webrev at:
>
Oops, here's webrev:

open webrev at http://cr.openjdk.java.net/~coleenp/8074457.02/

Coleen
>
> Thanks,
> Coleen
>
>>
>> Best regards,
>>    Goetz.
>>
>>
>>
>>
>>
>>
>>
>>> -----Original Message-----
>>> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On
>>> Behalf Of Coleen Phillimore
>>> Sent: Freitag, 18. Dezember 2015 14:49
>>> To: hotspot-dev developers <hotspot-dev at openjdk.java.net>
>>> Subject: RFR: 8074457: Remove the non-Zero CPP Interpreter
>>>
>>> Summary: Remove cppInterpreter assembly files and reorganize
>>> InterpreterGenerator includes
>>>
>>> This change is mostly removal and removing the InterpreterGenerator
>>> class and making class Interpreter a typedef.  I removed conditional
>>> includes from interpreter header files in favor of small sections with
>>> ifdefs.   Many interpreter functions are still in the wrong cpp files
>>> but I want to leave that for a follow on, to not overwhelm reviewers.
>>> This is Large but not difficult to review.  There is also more purging
>>> that can be done with Zero, but I also want to leave that as a 
>>> follow on
>>> RFE.
>>>
>>> This has been tested with RBT (most of runtime tests on x86 and sparc),
>>> JPRT and builds zero with debug/fastdebug and product.
>>>
>>> There are changes and deletions to ppc and aarch64.   Please let me 
>>> know
>>> if you want to test with this patch or leave for follow on fixes.
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/8074457/
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8074457
>>>
>>> Thanks,
>>> Coleen
>