RFR: 8074457: Remove the non-Zero CPP Interpreter

[Coleen Phillimore]

On 12/22/15 8:48 AM, Bertrand Delsart wrote:
> Hi Coleen,
>
> Overall looks good. Thanks for the cleanup.
>
> A few minor issues. No showstoppers. Feel free to ignore them or 
> handle them later.
>
> First issue in 
> src/share/vm/interpreter/templateInterpreterGenerator.hpp, for 
> instance for these lines :
>
>  #ifdef TARGET_ARCH_aarch64
> - # include "templateInterpreterGenerator_aarch64.hpp"
> - #endif
> + void bang_stack_shadow_pages(bool native_call);
> + void generate_transcendental_entry(AbstractInterpreter::MethodKind 
> kind, int fpargs);
> + #endif // TARGET_ARCH_aarch64
>
> Is there a reason to remove a CPU dependent file, which could be 
> customized by any porting group, and instead directly add 
> port-dependent methods in a shared file ?
>
> Similar problem for AbstractInterpreter::expr_offset_in_bytes, which 
> was defined in CPU dependent files but is now defined in 
> abstractInterpreter.hpp, requiring a PPC and SPARC ifdef. Should we 
> instead define new abstractInterpreter_<cpu>.hpp files, including the 
> CPU specific part that was in interpreter_<cpu>.hpp ?

Hi Bertrand,  I was hoping for some comments on these changes in 
particular and I should have explained my reasoning in the RFR.  I 
decided that a couple of ifdefs in the shared code were better than the 
cpu dependent inclusion of header files.   Mostly because the cpu 
dependent header files had a lot of duplicated content and it was 
difficult to find exactly what the differences were between the cpu 
implementations.  Also, it would make the interpreter easier to work on 
if the interfaces were similar (you can change the same named function 
in all the cpu files)  and this is a mechanism for doing so.

The example with expr_offset_in_bytes is the annoying difference in 
sparc, and now PPC, where the TOS points one past Lesp that we filed a 
cleanup bug for a long time ago.  It would be nice not to have this 
difference!

Lastly, I also dislike #include in the declaration of a class and so do 
many IDEs.

Thank you for reviewing the code.
Coleen


>
> Regards,
>
> Bertrand.
>
>
>
> On 18/12/2015 14:49, Coleen Phillimore wrote:
>> 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
>>
>
>