[PATCH] 8217561 : X86: Add floating-point Math.min/max intrinsics, approval request

[B. Blaser]

On Mon, 25 Feb 2019 at 19:42, Andrew Haley <aph at redhat.com> wrote:
>
> On 2/25/19 4:09 PM, B. Blaser wrote:
> > Here it is:
> >
> > http://cr.openjdk.java.net/~bsrbnd/jdk8217561/webrev.05/
> >
> > I've just added the interesting use-case of inserting elements in a search tree.
> > The insertion is absolutely unpredictable with random elements whereas
> > fully predictable with sorted elements.
> >
> > Benchmarking this example is hard as it'd compute the average
> > insertion time in a search tree which highly depends on its
> > topography.
> > That's why I decided to add it to the test-case doing some manual
> > measurement using 'System.nanoTime()'.
> >
> > This experiment revealed at no surprise that the intrinsic is really
> > worth being used only in the non-predictable case which the heuristic
> > is for.
> >
> > Any Reviewer feedback and hopefully approval would be welcome
> > (hotspot:tier1 is still OK on x86_64 xeon)!
>
> 6612 #ifdef AMD64
>
> Why? It's the same for every out-of-order processor with speculation.
>
> And the code *really* needs a comment to say what you're doing.

Thanks for your comments, I've updated the patch accordingly here under.

As it affects shared code, should I already push it to jdk/submit?
Is the following procedure up-to-date?

https://wiki.openjdk.java.net/display/Build/Submit+Repo

Thanks,
Bernard


diff --git a/src/hotspot/share/opto/library_call.cpp
b/src/hotspot/share/opto/library_call.cpp
--- a/src/hotspot/share/opto/library_call.cpp
+++ b/src/hotspot/share/opto/library_call.cpp
@@ -6609,6 +6609,31 @@

 //------------------------------inline_fp_min_max------------------------------
 bool LibraryCallKit::inline_fp_min_max(vmIntrinsics::ID id) {
+  // The intrinsic should be used only when the API branches aren't
predictable,
+  // the last one performing the most important comparison. The
following heuristic
+  // uses the branch statistics to eventually bail out if necessary.
+
+  ciMethod *c = callee();
+  ciMethodData *d = c->method_data();
+
+  int taken = 0, not_taken = 0;
+  int invocations = d->invocation_count();
+
+  if (invocations > 0) {
+    for (ciProfileData *p = d->first_data(); d->is_valid(p); p =
d->next_data(p)) {
+      if (p->is_BranchData()) {
+        taken = ((ciBranchData*)p)->taken();
+        not_taken = ((ciBranchData*)p)->not_taken();
+      }
+    }
+
+    double balance = (((double)taken) - ((double)not_taken)) /
((double)invocations);
+    balance = balance < 0 ? -balance : balance;
+    if ( balance > 0.2 ) {
+      return false;
+    }
+  }
+
   Node *a = NULL;
   Node *b = NULL;
   Node *n = NULL;