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,

could I please get a second review?
I also please need a sponsor.

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