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

[Bhateja, Jatin]

Hi Bernard,

Thanks for your detailed review, I do not have commit access yet since this is my first contribution.
I request you to kindly upstream the changes from link you shared and add me as a contributor. 

Regards,
Jatin
	
> -----Original Message-----
> From: B. Blaser [mailto:bsrbnd at gmail.com]
> Sent: Sunday, February 10, 2019 12:13 AM
> To: Bhateja, Jatin <jatin.bhateja at intel.com>
> Cc: Vladimir Kozlov <vladimir.kozlov at oracle.com>; 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
> 
> Hi Jatin, All,
> 
> On Fri, 8 Feb 2019 at 06:55, Bhateja, Jatin <jatin.bhateja at intel.com> wrote:
> >
> > Hi Bernard, All,
> >
> > Please find below the link to updated patch.
> >
> > http://cr.openjdk.java.net/~sviswanathan/Jatin/8217561/webrev.03/
> >
> > Regards,
> > Jatin
> 
> Your patch seems quite good to me.
> 
> However, I'd like to suggest the following variant (which is mostly a
> copy/paste of your code) focusing on x86_64 as I don't think that taking care
> of x86_32 is worth the additional complexity. It uses the min/max scalar
> variant which is conceptually better than the packed one. Regarding the test
> case, I simply added a main loop along with an
Ok, but 32 bit execution context we do have access to XMM0-XMM7 and new 
instruction sequence did show gain there. 

> x86_64 test configuration (similar to yours) without modifying the existing
> AArch64 parameters:
> 
> http://cr.openjdk.java.net/~bsrbnd/jdk8217561/webrev.04/
> 
> This variant also includes the following small code improvements:
> 
> * copyright years updated where necessary
> * useless assertions removed ('vex_prefix_and_encode' already checks the
> register encoding in 'legacy_mode')
> * constants used to improve readability like 0 -> AVX_128bit, 0x3 -> _false, ...
> * several comments fixed like
>   * 'java.lang.Max()' -> 'java.lang.Math.max()', ...
>   * 'if b < 0' -> 'if b < +0.0'
> * unnecessary trailing spaces removed
> 
> Test-tier1 is OK on x86_64 (xeon).
> 
> Could we then have a Reviewer feedback (and eventually an approval) for
> this?

Above changes looks good.
> 
> Thanks,
> Bernard