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

[Lindenmaier, Goetz]

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.

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?
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.
>>