RFR(S): 8056950: Compiled code (64-bit) on SPARC should sign extend INT parameters passed on registers to runtime or native methods.

[Tobias Hartmann]

Hi Goetz,

On 08.07.2016 15:32, Lindenmaier, Goetz wrote:
> Hi,
> 
> could I please get a second review?
> I also please need a sponsor.

I can do the sponsoring once you got a second review.

Best regards,
Tobias

> 
> Best regards,
>   Goetz.
> 
>> -----Original Message-----
>> From: Tobias Hartmann [mailto:tobias.hartmann at oracle.com]
>> Sent: Mittwoch, 6. Juli 2016 15:37
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-compiler-
>> dev at openjdk.java.net
>> Subject: Re: RFR(S): 8056950: Compiled code (64-bit) on SPARC should sign
>> extend INT parameters passed on registers to runtime or native methods.
>>
>> Hi Goetz,
>>
>> On 05.07.2016 12:09, Lindenmaier, Goetz wrote:
>>> Hi Tobias,
>>>
>>> thanks for your comments!
>>>
>>> For the widening issue -- it's not straight forward to implement.
>>> Each intrinsic has it's own section generating the type and call node.
>>> If these are fixed, it tends to be forgotten if new ones are added.
>>>
>>> A generic approach would add the widening further down when
>>> generating the call, but there e.g. the type can not be changed any
>>> more because types are constant and it's passed down by reference.
>>
>> Right, I agree that this is not easy to fix, so let's leave this for now.
>>
>> Best regards,
>> Tobias
>>
>>>
>>> Best regards,
>>>   Goetz.
>>>
>>>> -----Original Message-----
>>>> From: Tobias Hartmann [mailto:tobias.hartmann at oracle.com]
>>>> Sent: Dienstag, 5. Juli 2016 09:16
>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-
>> compiler-
>>>> dev at openjdk.java.net
>>>> Subject: Re: RFR(S): 8056950: Compiled code (64-bit) on SPARC should sign
>>>> extend INT parameters passed on registers to runtime or native methods.
>>>>
>>>> Hi Goetz,
>>>>
>>>> thanks for taking care of this bug!
>>>>
>>>> I agree that we should think about fixing the C stubs by widening ints to
>> longs
>>>> in C2 but as this is currently not an issue, I'm fine with your fix.
>>>>
>>>> Best regards,
>>>> Tobias
>>>>
>>>> On 04.07.2016 16:35, Lindenmaier, Goetz wrote:
>>>>> Hi,
>>>>>
>>>>>
>>>>>
>>>>> I had a look at "8056950: Compiled code (64-bit) on SPARC should sign
>>>> extend INT parameters passed on registers to runtime or native
>> methods."
>>>> https://bugs.openjdk.java.net/browse/JDK-8056950  We fixed the same
>>>> issue for ppc.
>>>>>
>>>>>
>>>>>
>>>>> There are three parts to this:
>>>>>
>>>>>
>>>>>
>>>>> 1.) generate_native_wrapper(). The native wrapper should do the sign
>>>> extensions for the smaller ints.
>>>>>
>>>>> This was missing when 8056950 was opened, but fixed by "8148353:
>> [linux-
>>>> sparc] Crash in libawt.so on Linux SPARC"
>>>>>
>>>>> in the meantime.
>>>> http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/1f4f4866aee0
>>>>>
>>>>>
>>>>>
>>>>> 2.)  Stubs called directly like updateBytesCRC32C. If the C2 compiler
>> passes
>>>> an int to these it is not
>>>>>
>>>>> correctly sign extended. updateBytesCRC32C and most others are
>>>> implemented in assembly, so one can
>>>>>
>>>>>  assure proper 32-bit instructions are used.
>>>>>
>>>>> A problem are stubs implemented in C, as montgomery_square() on
>> ppc.
>>>> On ppc we solved this with
>>>>>
>>>>>  a trick in the platform implementations, see
>> sharedRuntime_ppc.cpp:3445,
>>>> "len = len & 0x7fffFFFF;"
>>>>>
>>>>> Montgomery is not implemented on sparc, though. I did not find any
>> other
>>>> similar on sparc.
>>>>>
>>>>>
>>>>>
>>>>> 3.) The optoStubs generated by the C2 compiler. The C2 compiler will
>> insert
>>>> proper casts here if
>>>>>
>>>>>   CCallingConventionRequiresIntsAsLongs is set. So this needs to be set
>> on
>>>> sparc.
>>>>>
>>>>>
>>>>>
>>>>> I propose the folowing simple, straight forward webrev:
>>>>>
>>>>> http://cr.openjdk.java.net/~goetz/wr16/8056950-
>>>> SparcIntToLong/webrev.01/
>>>>>
>>>>>
>>>>>
>>>>> For 2.)/montgtomery, one could think about implementing widening the
>>>> type of 'len' to
>>>>>
>>>>> long in C2, but it's currently not an issue.
>>>>>
>>>>>
>>>>>
>>>>> Best regards,
>>>>>
>>>>>   Goetz.
>>>>>