[16] RFR(S): 8251525: AARCH64: Faster Math.signum(fp)

[Dmitry Chuyko]

Hi Vladimir,

Thank you for the comments. Here is a version with simplified node 
definitions:

http://cr.openjdk.java.net/~dchuyko/8251525/webrev.01/

-Dmitry

On 8/13/20 2:51 PM, Vladimir Ivanov wrote:
> Hi Dmitry,
>
> Some comments on shared code changes:
>
> src/hotspot/share/opto/library_call.cpp:
>
> +  case vmIntrinsics::_dsignum:
> +    return UseSignumIntrinsic && 
> (Matcher::match_rule_supported(Op_SignumD) ? inline_double_math(id) : 
> false);
>
> There's no need in repeating UseSignumIntrinsic and 
> (Matcher::match_rule_supported(Op_SignumD) checks.
> C2Compiler::is_intrinsic_supported() already covers taht.
>
>
> src/hotspot/share/opto/signum.hpp:
>
>   32 class SignumNode : public Node {
>   33 public:
>   34   SignumNode(Node* in) : Node(0, in) {}
>   35   virtual int Opcode() const;
>   36   virtual const Type *bottom_type() const { return NULL; }
>   37   virtual uint ideal_reg() const { return Op_RegD; }
>   38 };
>
> Any particular reason to keep SignumNode? I don't see any and would 
> just drop it.
>
> Also, having a dedicated header file just for a couple of nodes with 
> trivial implementations looks like an overkill. As an alternative 
> location, intrinsicnode.cpp should be a better option.
>
> Best regards,
> Vladimir Ivanov
>
> On 13.08.2020 14:04, Dmitry Chuyko wrote:
>> Hello,
>>
>> Please review a faster version of Math.signum() for AArch64.
>>
>> Two new intrinsics (double and float) are introduced in general code, 
>> with appropriate new nodes. New JTreg test is added to cover the 
>> intrinsic case (enabled only for aarch64).
>>
>> AArch64 implementation uses FACGT (compare abslute fp values) and BSL 
>> (fp bit selection) to avoid branches and moves to non-fp registers 
>> and back.
>>
>> Performance results show ~30% better time in the benchmark with a 
>> black hole [1] on Cortex. E.g. on random numbers 4.8 ns/op --> 3.5 
>> ns/op, overhead is 2.9 ns/op.
>>
>> rfe: https://bugs.openjdk.java.net/browse/JDK-8251525
>> webrev: http://cr.openjdk.java.net/~dchuyko/8251525/webrev.00/
>> testing: jck, jtreg including new dedicated test
>>
>> -Dmitry
>>
>> [1] https://cr.openjdk.java.net/~dchuyko/8249198/DoubleSignum.java
>>