RFR(M): 8024342: PPC64 (part 111): Support for C calling conventions that require 64-bit ints.

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Sun Sep 15 05:46:48 PDT 2013


Hi,

I am now using a constant as proposed by you, Vladimir.
I like it much better this way.  Here the new webrev:
http://cr.openjdk.java.net/~goetz/webrevs/8024342-intArg-3

Thanks for the review!
Best regards,
  Goetz.


-----Original Message-----
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
Sent: Saturday, September 14, 2013 12:21 AM
To: Lindenmaier, Goetz
Cc: 'ppc-aix-port-dev at openjdk.java.net'; 'hotspot-dev at openjdk.java.net'
Subject: Re: RFR(M): 8024342: PPC64 (part 111): Support for C calling conventions that require 64-bit ints.

On 9/13/13 1:33 PM, Lindenmaier, Goetz wrote:
> Hi,
>
>>> Why only intrinsify_fill changed? What about arraycopy and other stubs
>>> which use int parameter?
> I figured why we need the adaption of the signature for intrinsify_fill.
> This stub call is done with a CallLeafNoFPNode, which is child of
> CallRuntimeNode. Therefore the matcher expects c_calling_conventions.
> Arraycopy stubs are called with CallStaticJavaNode, thus they are fine. Other stubs
> just use 64-bit types.
> Also there is a guarantee() in the ppc c_calling_convention() checking
> the signature, so that we are sure we did not miss a case.

Okay.

>
> I adapted the type test and moved the functions to the ppc file :(
> What I don't like is that c_calling_convention_requires_ints_as_longs() can
> not be inlined by the C Compiler.  That's because sharedRuntime has
> no platform header.  Do you have an idea how I could improve this?

Yes, if it was inlined we could keep those methods in shared code but 
their bodies will be guarded by 
c_calling_convention_requires_ints_as_longs() check and C will optimize 
them out on platforms we don't need them.

May be replace c_calling_convention_requires_ints_as_longs() method with 
constant in globalDefinitions_<arch>.hpp ?

Vladimir

> http://cr.openjdk.java.net/~goetz/webrevs/8024342-intArg-2
>
> Also, could I please get a second review?
>
> Thanks & best regards,
>    Goetz.
>
>
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Thursday, September 12, 2013 10:02 PM
> To: Lindenmaier, Goetz
> Cc: ppc-aix-port-dev at openjdk.java.net; hotspot-dev at openjdk.java.net
> Subject: Re: RFR(M): 8024342: PPC64 (part 111): Support for C calling conventions that require 64-bit ints.
>
>
>
> On 9/12/13 4:42 AM, Lindenmaier, Goetz wrote:
>> Hi Vladimir,
>>
>>> Can you move convert_ints_to_longints*() methods from sharedRuntime.cpp
>>> to sharedRuntime_ppc.cpp? They are static and not used in any other places.
>> I really would like to keep this code in the shared file because:
>>    - The code is completely plarform independent.
>>    - The feature needs several shared changes, so I think it's natural to keep all
>>      it's platform independent parts in the shared files.
>>    - Internally, we use it also for zArch, so for us it would reduce code duplication
>>       to have it in the shared file.
>
> My concern is about VM size on platforms which do not need this.
> I wanted to suggest to put it under #ifdef but it will be ugly.
>
>>
>>> Why only intrinsify_fill changed? What about arraycopy and other stubs
>>> which use int parameter?
>> Yes, that looks strange.  For stub calls that should not be needed, as
>> we also implement the callee.  I'll investigate this in our VM, and try
>> to remove the change to runtime.cpp and loopTransform.cpp.
>> I'll come up with a new webrev then.  Thanks for pointing me to this!
>>
>>> loopTransform.cpp: I don't think ConvI2L will work for t==T_FLOAT. Also
>>> you can use (t == T_INT || is_subword_type(t)) check instead for int types.
>> Above, a MoveF2I is issued, therefore ConvI2L works.  Probably conversion
>> is pointless, but the type is correct then.
>
> I missed MoveF2I. Okay then.
>
> Thanks,
> Vladimir
>
>>
>> Thanks for the review and for adapting the closed stuff to 0112.
>>
>> Best regards,
>>     Goetz.
>>
>>
>> On 9/9/13 1:42 AM, Lindenmaier, Goetz wrote:
>>> Hi,
>>>
>>> This is the first of about 10 changes taking us to a rudimentary c2
>>> compiler on ppc64.
>>>
>>> Some platforms, as ppc and s390x/zArch require that 32-bit ints are
>>> passed as 64-bit values to C functions. This change adds support to
>>> adapt the signature and to issue proper casts to c2-compiled stubs. The
>>> functions are used in generate_native_wrapper(). We also need to adapt
>>> the signature in PhaseIdealLoop::intrinsify_fill().
>>>
>>> We add a function  c_calling_convention_requires_ints_as_longs()to the platform files of sharedRuntime, with
>>>
>>> enables this feature on ppc.  All other shared changes depend on this function.  The code should not affect the existing
>>>
>>> platforms.  The usage of the code is already visible in the sharedRuntime_ppc64 file in the staging repository (protected by
>>>
>>> ifdef COMPILER2).  Seehttp://hg.openjdk.java.net/ppc-aix-port/stage/hotspot/file/bdd155477289/src/cpu/ppc/vm/sharedRuntime_ppc.cpp
>>>
>>> line 1752 ff.
>>>
>>>
>>>
>>> http://cr.openjdk.java.net/~goetz/webrevs/8024342-intArg/
>>>
>>> Please review and test this change.
>>>
>>> Best regards,
>>>
>>>      Goetz.
>>>


More information about the ppc-aix-port-dev mailing list