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 11.07.2016 11:42, Lindenmaier, Goetz wrote:
> Hi Volker, 
> 
> thanks for looking at this. 
> I can't add the bug because I don't have access to it, it's closed.
> Tobias, can you access it and change it to open?

Unfortunately, I cannot change it to open because the bug description and comments contain confidential information. I still linked the bug.

> And I can't add the description because jbs doesn't let me log in ...
> I'll keep trying.

I saw that you now managed to add a comment. I think that's sufficient for other people to understand the limitations of your fix. 

Best regards,
Tobias

> 
> Best regards,
>   Goetz.
> 
>> -----Original Message-----
>> From: Volker Simonis [mailto:volker.simonis at gmail.com]
>> Sent: Montag, 11. Juli 2016 11:13
>> To: Tobias Hartmann <tobias.hartmann at oracle.com>
>> Cc: 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,
>>
>> your change looks good!
>>
>> Can you please also link 8148353 to this bug in JBS. Maybe you can
>> also add your description from the initial mail to JBS. I think it
>> could be useful if somebody will look at this again (maybe because he
>> wants to implement the widening issue).
>>
>> @Tobias: thanks for sponsoring.
>>
>> Regards,
>> Volker
>>
>>
>>
>> On Mon, Jul 11, 2016 at 10:40 AM, Tobias Hartmann
>> <tobias.hartmann at oracle.com> wrote:
>>> 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.
>>>>>>>>