[aarch64-port-dev ] RFR(M): 8196402: AARCH64: create intrinsic for Math.log

[Dmitry Samersoff]

Dmitrij,

Looks good to me.

-Dmitry


On 06/21/2018 06:30 PM, Dmitrij Pochepko wrote:
> Hi,
> 
> I need official reviewer to review this version together with "8204289:
> AARCH64: enable math intrinsics usage in interpreter and C1" to wrap
> this up.
> 
> Thanks,
> 
> Dmitrij
> 
> 
> On 20.06.2018 19:51, White, Derek wrote:
>> Hi Dmitrij,
>>
>> Looks good to me. Thanks!
>> - Derek
>>
>>> -----Original Message-----
>>> From: Dmitrij Pochepko [mailto:dmitrij.pochepko at bell-sw.com]
>>> Sent: Wednesday, June 20, 2018 10:26 AM
>>> To: Andrew Haley <aph at redhat.com>; White, Derek
>>> <Derek.White at cavium.com>
>>> Cc: aarch64-port-dev at openjdk.java.net
>>> Subject: Re: [aarch64-port-dev ] RFR(M): 8196402: AARCH64: create
>>> intrinsic
>>> for Math.log
>>>
>>> External Email
>>>
>>> Hi,
>>>
>>>
>>> On 20.06.2018 11:31, Andrew Haley wrote:
>>>> On 06/20/2018 12:45 AM, White, Derek wrote:
>>>>>    - Line 68: Could align to 64 bytes. We're going to load the
>>>>> first 64 bytes,
>>> may as well use one cache line.
>>> Agree. It'll be a bit better to use 64 bytes alignment. I changed it to
>>> 64 and tested the revised patch. I could not see a difference in
>>> synthetic
>>> benchmark, but single invocation might gain a small difference in
>>> case of L1
>>> miss.
>>>>>    - Lines 71, 77, 79. You could comment these lines with "_coeff",
>>>>> "_log2",
>>> and "_L_tbl", for clearer correspondence to x86 code.
>>> Added respective comment.
>>>>> - Line 204: I believe that we should be looking for equal to 0.0:
>>>>> 204 //      if (X = 0.0d) return -INFINITY;
>>>> Yes.  if (X == 0.0d).
>>> Thank you for noticing this typo. Assembly is correct. I fixed this
>>> comment.
>>>>> - The (float)(zeroExponent(X) code was fun.
>>>>>
>>>>> - Are there any concerns about denormalized inputs? I didn't see any
>>> obvious problem in the code pulling aaprt and adding in exponents, but
>>> there's nothing obvious in that code to me.
>>>> IEEE denormals don't usually require any special handling.
>>> There is no denormals-specific code in this algorithm.
>>>>> - The RETURN_MINF_OR_NAN block can generate a wide range of
>>> signaling and non-signaling NaNs. I forget how signaling Nans are
>>> treated in
>>> the Java math mode - are they just treated as quiet NaNs or are they
>>> never
>>> supposed to appear?
>>>> Java doesn't care.  However, it is possible to use DoubleToRawLongBits
>>>> to extract some information, and this is fine.
>>> I also aligned comments as in sin/cos patch and fixed the copyright.
>>>
>>> Here, the original code has table in HEX and also the article has HEX
>>> representation, so I kept HEX in the table.
>>>
>>> Please take a look at updated webrev:
>>> http://cr.openjdk.java.net/~dpochepk/8196402/webrev.06/
>>>
>>> Thanks,
>>> Dmitrij
> 


-- 
Dmitry Samersoff
http://devnull.samersoff.net
* There will come soft rains ...