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

[Lindenmaier, Goetz]

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?

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

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