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

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