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

[Vladimir Kozlov]

It is good. I need to prepare closed part and I will push after its reviews.

Thanks,
Vladimir

On 9/15/13 5:46 AM, Lindenmaier, Goetz wrote:
> 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.
>>>>