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

[B. Blaser]

Hi Jatin,

On Mon, 4 Feb 2019 at 19:57, Bhateja, Jatin <jatin.bhateja at intel.com> wrote:
>
> Hi All,
>
> Please find the link to updated patch.
>
> http://cr.openjdk.java.net/~sviswanathan/Jatin/8217561/webrev.02/
>
> It contains fix to stop unnecessary register to register copy when source and destination are same,
> test case updated to reflect correct copyright year and has been tested over both 32 and 64 bit variants.
>
> Regards,
> Jatin

I like your patch and I guess it works but I still disapprove the test
case parameters.

Try this:

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
@@ -6640,6 +6640,7 @@
   default:  fatal_unexpected_iid(id);  break;
   }
   set_result(_gvn.transform(n));
+printf("#### inline_fp_min_max ####\n");
   return true;
 }

or add 'printf()' in:

void Assembler::vmaxpd(XMMRegister dst, XMMRegister nds, XMMRegister
src, int vector_len) {
printf("#### vmaxpd ####\n");

and run the test case:

$ make test TEST="jtreg:test/hotspot/jtreg/compiler/intrinsics/math/TestFpMinMaxIntrinsics.java"

Then, check the test report:

command: main -XX:+UnlockDiagnosticVMOptions -Xcomp
-XX:-TieredCompilation -XX:+IgnoreUnrecognizedVMOptions
-XX:CompileOnly=java/lang/Math
compiler.intrinsics.math.TestFpMinMaxIntrinsics
reason: User specified action: run main/othervm
-XX:+UnlockDiagnosticVMOptions -Xcomp -XX:-TieredCompilation
-XX:+IgnoreUnrecognizedVMOptions -XX:CompileOnly=java/lang/Math
compiler.intrinsics.math.TestFpMinMaxIntrinsics
Mode: othervm [/othervm specified]
elapsed time (seconds): 0.671
----------configuration:(0/0)----------
----------System.out:(0/0)----------
----------System.err:(1/15)----------
STATUS:Passed.

We don't see any 'printf()' meaning that the intrinsic isn't inlined!

Now that you added enough iterations in your main loop (>10K as
Vladimir said), add the following line to the test case:

 * @run main/othervm  compiler.intrinsics.math.TestFpMinMaxIntrinsics

rerun the test and check again the report:

command: main compiler.intrinsics.math.TestFpMinMaxIntrinsics
reason: User specified action: run main/othervm
compiler.intrinsics.math.TestFpMinMaxIntrinsics
Mode: othervm [/othervm specified]
elapsed time (seconds): 0.068
----------configuration:(0/0)----------
----------System.out:(24/584)----------
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### vmaxpd ####
#### vmaxpd ####
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### vmaxpd ####
#### vmaxpd ####
#### vmaxpd ####
#### vmaxpd ####
#### vmaxpd ####
#### vmaxpd ####
----------System.err:(1/15)----------
STATUS:Passed.

The intrinsic is clearly inlined now.

As I said earlier, you have to enable the compilation of the caller
(TestFpMinMaxIntrinsics::dTest) to inline the intrinsic in its
generated code and I guess this is what Nils meant.
The problem with the current parameters is that
'-XX:CompileOnly=java/lang/Math' prevents the compilation of
'TestFpMinMaxIntrinsics::dTest', so simply try to replace it with:

-XX:CompileOnly=compiler.intrinsics.math.TestFpMinMaxIntrinsics::dTest

and rerun the test:

command: main -XX:+UnlockDiagnosticVMOptions -Xcomp
-XX:-TieredCompilation -XX:+IgnoreUnrecognizedVMOptions
-XX:CompileOnly=compiler.intrinsics.math.TestFpMinMaxIntrinsics::dTest
compiler.intrinsics.math.TestFpMinMaxIntrinsics
reason: User specified action: run main/othervm
-XX:+UnlockDiagnosticVMOptions -Xcomp -XX:-TieredCompilation
-XX:+IgnoreUnrecognizedVMOptions
-XX:CompileOnly=compiler.intrinsics.math.TestFpMinMaxIntrinsics::dTest
compiler.intrinsics.math.TestFpMinMaxIntrinsics
Mode: othervm [/othervm specified]
elapsed time (seconds): 0.634
----------configuration:(0/0)----------
----------System.out:(8/180)----------
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### vmaxpd ####
#### vmaxpd ####
#### inline_fp_min_max ####
#### inline_fp_min_max ####
#### vmaxpd ####
#### vmaxpd ####
----------System.err:(1/15)----------
STATUS:Passed.

The intrinsic is also inlined!

But, playing with these parameters is tricky as Vladimir said, so I
suggest you simply add:

 * @run main/othervm  compiler.intrinsics.math.TestFpMinMaxIntrinsics

with a loop of more than 10K iterations.

Got it?

I'll run test-tier1 on x86_64 (xeon) but I don't work anymore on
x86_32, so I let Oracle comment on this part as I'm not sure how much
do they still use it?

Regards,
Bernard