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

[Bhateja, Jatin]

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   

> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Monday, February 4, 2019 11:02 PM
> To: Bhateja, Jatin <jatin.bhateja at intel.com>; B. Blaser <bsrbnd at gmail.com>
> Cc: Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; hotspot-
> compiler-dev at openjdk.java.net; Deshpande, Vivek R
> <vivek.r.deshpande at intel.com>
> Subject: Re: [PATCH] 8217561 : X86: Add floating-point Math.min/max
> intrinsics
> 
> You can easy check if method's call is inlined or intrinsified by running with
> 
> -XX:CICompilerCount=1 -XX:+PrintCompilation -
> XX:+UnlockDiagnosticVMOptions -XX:+PrintInlining
> 
> or with -XX:+UnlockDiagnosticVMOptions -XX:+LogCompilaiton which writes
> all details about compilations into file.
> 
> I general for short running methods (like these min/max) it is preferable to
> call test method which call min/max in a loop enough time (>10K times) to
> trigger its compilation instead of using -Xcomp which has some side effects
> (compileonly option is tricky - better avoid it). And use -Xbatch to wait
> compilations to finish when it triggered by invocation count.
> 
> You can also lower CompileThreshold value (1000?) since you run with -XX:-
> TieredCompilation. But better to use
> -XX:CompileThresholdScaling=0.1 which works in all cases.
> 
> Regards,
> Vladimir
> 
> On 2/4/19 7:35 AM, Bhateja, Jatin wrote:
> > Hi Blaser,
> >
> > Please see my response embedder.
> >
> > Best Regards,
> > Jatin
> >
> >> -----Original Message-----
> >> From: B. Blaser [mailto:bsrbnd at gmail.com]
> >> Sent: Monday, February 4, 2019 8:01 PM
> >> To: Bhateja, Jatin <jatin.bhateja at intel.com>
> >> Cc: Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; hotspot-
> >> compiler-dev at openjdk.java.net; Vladimir Kozlov
> >> <vladimir.kozlov at oracle.com>; Deshpande, Vivek R
> >> <vivek.r.deshpande at intel.com>
> >> Subject: Re: [PATCH] 8217561 : X86: Add floating-point Math.min/max
> >> intrinsics
> >>
> >> Hi Jatin,
> >>
> >> On Mon, 4 Feb 2019 at 13:53, Bhateja, Jatin <jatin.bhateja at intel.com>
> >> wrote:
> >>>
> >>> Hi Blaser,
> >>>
> >>> Please see response embedded in following mail.
> >>>
> >>> Best Regards,
> >>> Jatin
> >>>
> >>>> -----Original Message-----
> >>>> From: B. Blaser [mailto:bsrbnd at gmail.com]
> >>>> Sent: Monday, February 4, 2019 5:13 PM
> >>>>
> >>>> Yes, of course, but I meant:
> >>>>
> >>>> http://cr.openjdk.java.net/~sviswanathan/Jatin/8217561/webrev.01/te
> >>>> s
> >>>> t/ho
> >>>> tspot/jtreg/compiler/intrinsics/math/TestFpMinMaxIntrinsics.java.ht
> >>>> m
> >>>> l
> >>>>
> >>>> I believe '-XX:CompileOnly=java/lang/Math' induce the compilation
> >>>> of 'Math.max()' but not its intrinsification which occurs when the
> >>>> call is inlined,
> >>>> see:
> >>>>
> >>>> http://hg.openjdk.java.net/jdk/jdk/rev/f15af1e2c683#l9.29
> >>>>
> >>>
> >>> I can see that if a method is a candidate for intrinsification
> >>> (which is decided during initialization as we create explicit
> >>> vmSymbols for them)
> >> then during initial graph creation itself a specialized node is
> >> created for the call site (refer to processing of invoke* bytecodes done in
> Parse::do_call()).
> >> Thus intrinsification in this case will implicitly do the inlining.
> >>>
> >>>> http://hg.openjdk.java.net/jdk/jdk/file/d997c227e968/src/hotspot/sh
> >>>> a
> >>>> re/op
> >>>> to/library_call.cpp#l6611
> >>>>
> >>>> But this would be good if someone knowledgeable in this area could
> >>>> confirm this?
> >>> An experienced hand could comment more on this.
> >>
> >> I think this is what Nils meant here:
> >>
> >> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-
> >> January/032528.html
> >>
> >>>>> Allow compilation of the methods in the test:
> >>>>>
> >>>>> "-
> >>
> XX:CompileCommand=compileonly,compiler.intrinsics.math.TestFpMinMaxI
> >> ntrinsics*"
> >>>>>
> >>>>> And add a loop in the main method, so the test case is run
> >>>>> multiple times
> >>
> >> This is also what I observed in my example; you have to enable
> >> compilation of the caller '-XX:CompileOnly=MinMax::test' and run it
> >> multiple times to trigger inlining of the intrinsic (I'm not sure
> >> that
> >> 100 iterations as you added in 'TestFpMinMaxIntrinsics' are enough
> though).
> >
> > @run is being passed -XX:-TieredCompilation -Xcomp, this should enable
> highest compilation level (C2).  But with no loop there is a chance of the
> method being interpreted till the time compilation thread emits the jit code,
> thus a loop of 100 will make sure  that successive invocation of method uses
> the jit code as soon as its available.
> >
> > BTW, we have taken care of your comment regarding spurious register
> copy before blend and next patch update will fix it. Thanks!!
> >
> >>
> >> Bernard
> >