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

[Volker Simonis]

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