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

[B. Blaser]

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
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?

Thanks,
Bernard