[PATCH] 8217561 : X86: Add floating-point Math.min/max intrinsics
bsrbnd at gmail.com
Mon Feb 11 11:17:24 UTC 2019
Hi Jatin, All,
Thanks for your first contribution, I'll push it to the main-line
repository once approved by a Reviewer which we are waiting for.
Last note about x86_32:
"Only 64-bit Java virtual machines (JVMs) are certified"
which is why I decided to drop this part in the latest webrev:
There's definitely no use taking care of implementing, testing and
maintaining intrinsics for legacy architectures.
On Sun, 10 Feb 2019 at 10:10, Bhateja, Jatin <jatin.bhateja at intel.com> wrote:
> 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.
> > -----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
More information about the hotspot-compiler-dev