Fwd: Re: RFR (XL): 8029940: PPC64 (part 122): C2 compiler port

[Vladimir Kozlov]

On 12/11/13 2:59 AM, Lindenmaier, Goetz wrote:
> Hi Vladimir,
>
> yes, I missed all relevant files -- It was just too late, sorry.
> Now it's in there:
> http://cr.openjdk.java.net/~goetz/webrevs/8029940-0-c2/
>
> I fixed the isel assembler routines.

Thanks.

> I know the CC_INTERP stuff is not complete if you
> switch it off.  That's because my colleagues are working
> on the template interpreter port, and I don't want to add
> that yet.  It's still changing too much.  But in some places
> I already added the defines, especially where they changed
> code used in both cases.
> I would appreciate if I could leave it that way.

Yes, it is fine. I thought it is already final changes :)

>
> The atomic files have whitespace fixes.  The comments were
> no more aligned after the ppc_ prefix removal.

Okay. We (Oracle) should fix webrev to show such changes. I had the same problem recently.

>
> We added _32 includes wherever we use a _64 include.
> And we  distinguished the two versions wherever it's done
> so on x86/sparc.

Okay.

>
> Thanks for pushing the other two changes!  Once this one
> is in, the compiler works quiet stable and with good performance.  I have
> two minor compiler fixes in the queue, but I want to test
> first to see whether they are still needed.

Why you need ppc_64.ad? Will you add code to it later?

I looked through ppc.ad file briefly and did not find anything bad. I will not be able to review it in details (12K
lines) so I will leave it to your judgement.

c2_globals_ppc.hpp - from what you pushed I don't see changes for your Tiered compilation implementation. Do you use
different flag? I am asking because I see your ReservedCodeCacheSize is big by default and for Tiered we increase it (in
arguments.cpp):

     FLAG_SET_DEFAULT(ReservedCodeCacheSize, ReservedCodeCacheSize * 5);

Is it okay with your tiered code?


In general changes are good and you can push it since it does not have shared changes (nothing to test for JPRT).


Thanks,
Vladimir

>
> Best regards,
>    Goetz
>
>
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Wednesday, December 11, 2013 1:01 AM
> To: Lindenmaier, Goetz; 'hotspot-dev at openjdk.java.net'; 'ppc-aix-port-dev at openjdk.java.net'
> Subject: Re: RFR (XL): 8029940: PPC64 (part 122): C2 compiler port
>
> I don't see .ad file. Did you do 'hg add'?
>
> assembler_ppc.inline.hpp:  coding style in Assembler::isel() and
> Assembler::isel_0(). Put if's body on separate line and use {}.
> Variables definitions and assignments should be on separate lines.
>
> frame_ppc.cpp:  three variables are defined under #ifdef CC_INTERP but
> are used outside it.
> sharedRuntime_ppc.cpp: has the same problem.
>
> interpreter_ppc.cpp:  please, confirm that you need #ifdef CC_INTERP in
> InterpreterGenerator::generate_abstract_entry() - you removed it in
> other places in this file (except in
> AbstractInterpreterGenerator::generate_slow_signature_handler()).
>
> register_definitions_ppc.cpp: why you need interp_masm_ppc_32.hpp? Just
> curious.
>
> atomic_linux_ppc.inline.hpp and orderAccess_linux_ppc.inline.hpp: empty
> diffs. Is it only spacing changes?
>
> Thanks,
> Vladimir
>
> On 12/10/13 3:21 PM, Lindenmaier, Goetz wrote:
>> Hi,
>>
>> this change contains the ppc c2 compiler port.  The major
>> contribution is obviously the .ad file.
>> In addition, it contains a row of improvements in ppc
>> code already used, and fixups left over after the ppc_-prefix
>> removal.
>> http://cr.openjdk.java.net/~goetz/webrevs/8029940-0-c2/
>>
>> The change touches only files of the ppc port.
>>
>> Please review and test this change.
>>
>> Please only push it after 219, as that's required to build
>> this change.
>>
>> (You might like the rules we use for matching StrIndexOf)
>>
>> Best regards,
>>     Goetz.
>>