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

[Doerr, Martin]

Hi Michihiro,

I think sharedRuntime is not a good place for the C2 enum.
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>
Sent: Montag, 7. Oktober 2019 09:14
To: Vladimir Ivanov <vladimir.x.ivanov at oracle.com>
Cc: hotspot compiler <hotspot-compiler-dev at openjdk.java.net>; ppc-aix-port-dev at openjdk.java.net; Doerr, Martin <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>>
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>, hotspot compiler <hotspot-compiler-dev at openjdk.java.net<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
>