RFR: 8231649: PPC64: Intrinsics for Math.ceil, floor, rint on Power

[Doerr, Martin]

Hi Michihiro,

please move the implementation into the "instruct" and remove the declarations from macroAssembler_ppc.hpp.
If you prefer having the implementation outside of the "instruct", please put it into new "enc_class".

Thanks,
Martin


From: Michihiro Horie <HORIE at jp.ibm.com>
Sent: Montag, 7. Oktober 2019 15:32
To: Doerr, Martin <martin.doerr at sap.com>
Cc: hotspot compiler <hotspot-compiler-dev at openjdk.java.net>; ppc-aix-port-dev at openjdk.java.net; Vladimir Ivanov <vladimir.x.ivanov at oracle.com>
Subject: RE: RFR: 8231649: PPC64: Intrinsics for Math.ceil, floor, rint on Power


Hi Martin, Vladimir,
Thank you for your feedback.

Fixed webrev is as follows.
http://cr.openjdk.java.net/~mhorie/8231649/webrev.03/

Best regards,
Michihiro

[Inactive hide details for "Doerr, Martin" ---2019/10/07 22:09:15---> I suggest to put the enum in RoundDoubleModeNode (convertn]"Doerr, Martin" ---2019/10/07 22:09:15---> I suggest to put the enum in RoundDoubleModeNode (convertnode.hpp) > and lift instruction selectio

From: "Doerr, Martin" <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>
To: Vladimir Ivanov <vladimir.x.ivanov at oracle.com<mailto:vladimir.x.ivanov at oracle.com>>, Michihiro Horie <HORIE at jp.ibm.com<mailto:HORIE at jp.ibm.com>>
Cc: hotspot compiler <hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>>, "ppc-aix-port-dev at openjdk.java.net<mailto:ppc-aix-port-dev at openjdk.java.net>" <ppc-aix-port-dev at openjdk.java.net<mailto:ppc-aix-port-dev at openjdk.java.net>>
Date: 2019/10/07 22:09
Subject: [EXTERNAL] RE: RFR: 8231649: PPC64: Intrinsics for Math.ceil, floor, rint on Power

________________________________



> I suggest to put the enum in RoundDoubleModeNode (convertnode.hpp)
> and lift instruction selection logic from macroAssembler_ppc.cpp into ppc.ad.
That sounds good.

Thanks,
Martin


> -----Original Message-----
> From: Vladimir Ivanov <vladimir.x.ivanov at oracle.com<mailto:vladimir.x.ivanov at oracle.com>>
> Sent: Montag, 7. Oktober 2019 12:55
> To: Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>; Michihiro Horie
> <HORIE at jp.ibm.com<mailto:HORIE at jp.ibm.com>>
> Cc: hotspot compiler <hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>>; ppc-aix-
> port-dev at openjdk.java.net<mailto:port-dev at openjdk.java.net>
> Subject: Re: RFR: 8231649: PPC64: Intrinsics for Math.ceil, floor, rint on Power
>
> > I think sharedRuntime is not a good place for the C2 enum.
> Agree.
>
> I suggest to put the enum in RoundDoubleModeNode (convertnode.hpp)
> and
> lift instruction selection logic from macroAssembler_ppc.cpp into ppc.ad.
>
> Best regards,
> Vladimir Ivanov
>
> >
> > Maybe intrinsicnode.hpp would be a better pace for it. But then, the
> > code in macroAssembler should be guarded by #ifdef COMPILER2.
> >
> > Best regards,
> >
> > Martin
> >
> > *From:*Michihiro Horie <HORIE at jp.ibm.com<mailto:HORIE at jp.ibm.com>>
> > *Sent:* Montag, 7. Oktober 2019 09:14
> > *To:* Vladimir Ivanov <vladimir.x.ivanov at oracle.com<mailto:vladimir.x.ivanov at oracle.com>>
> > *Cc:* hotspot compiler <hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>>;
> > ppc-aix-port-dev at openjdk.java.net<mailto:ppc-aix-port-dev at openjdk.java.net>; Doerr, Martin
> <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>
> > *Subject:* RE: RFR: 8231649: PPC64: Intrinsics for Math.ceil, floor,
> > rint on Power
> >
> > Hi Vladimir,
> >
> > Thanks a lot for your nice suggestion. Yes, I prefer enum use.
> > I'm wondering if the enum constants can be declared in
> sharedRuntime.hpp.
> > Webrev: http://cr.openjdk.java.net/~mhorie/8231649/webrev.02/
> >
> > Best regards,
> > Michihiro
> >
> > Inactive hide details for Vladimir Ivanov ---2019/10/04 23:05:48---Hi
> > Michihiro, src/hotspot/cpu/ppc/macroAssembler_ppc.cpp:Vladimir Ivanov
> > ---2019/10/04 23:05:48---Hi Michihiro,
> > src/hotspot/cpu/ppc/macroAssembler_ppc.cpp:
> >
> > From: Vladimir Ivanov <vladimir.x.ivanov at oracle.com
<mailto:vladimir.x.ivanov at oracle.com%0b>> > <mailto:vladimir.x.ivanov at oracle.com>>
> > To: Michihiro Horie <HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>,
> > ppc-aix-port-dev at openjdk.java.net<mailto:ppc-aix-port-dev at openjdk.java.net>
> > <mailto:ppc-aix-port-dev at openjdk.java.net>, hotspot compiler
> > <hotspot-compiler-dev at openjdk.java.net
<mailto:hotspot-compiler-dev at openjdk.java.net%0b>> > <mailto:hotspot-compiler-dev at openjdk.java.net>>
> > Date: 2019/10/04 23:05
> > Subject: [EXTERNAL] Re: RFR: 8231649: PPC64: Intrinsics for Math.ceil,
> > floor, rint on Power
> >
> > ------------------------------------------------------------------------
> >
> >
> >
> >
> > Hi Michihiro,
> >
> > src/hotspot/cpu/ppc/macroAssembler_ppc.cpp:
> > +  switch (rmode) {
> > +    case 0: // rint
> > +      frin(t, b);
> > +      break;
> > +    case 1: // floor
> > +      frim(t, b);
> > +      break;
> > +    case 2: // ceil
> > +      frip(t, b);
> > +      break;
> > +    default:
> > +      ShouldNotReachHere();
> > +  }
> >
> > What do you think about introducing enum constants instead of using
> > hard-coded 0/1/2?
> >
> > src/hotspot/share/opto/library_call.cpp:
> >
> >    case vmIntrinsics::_ceil:   n = new RoundDoubleModeNode(arg,
> > makecon(TypeInt::make(2))); break;
> >    case vmIntrinsics::_floor:  n = new RoundDoubleModeNode(arg,
> > makecon(TypeInt::make(1))); break;
> >    case vmIntrinsics::_rint:   n = new RoundDoubleModeNode(arg,
> > makecon(TypeInt::make(0))); break;
> >
> > The downside is you have to move the switch from macroAssembler
> because
> > enum should be visible from both places. (Is there a better place for
> > that than roundD_regNode::emit() in AD file?)
> >
> > Best regards,
> > Vladimir Ivanov
> >
> > On 02/10/2019 07:27, Michihiro Horie wrote:
> >>
> >> Dear all,
> >>
> >> Would you please review the following change?
> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8231649
> >> Webrev: http://cr.openjdk.java.net/~mhorie/8231649/webrev.00
> >>
> >> This change adds intrinsics for Math's ceil, floor, and rint for PPC64, on
> >> top of 8226721: Missing intrinsics for Math.ceil, floor, rint.
> >>
> >> Best regards,
> >> Michihiro
> >>
> >
> >
> >



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20191007/16aaee02/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.gif
Type: image/gif
Size: 105 bytes
Desc: image001.gif
URL: <https://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20191007/16aaee02/image001.gif>