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

[Andrew Haley]

On 05/21/2018 09:35 PM, Dmitrij Pochepko wrote:
> 
> 
> On 21.05.2018 17:45, Andrew Haley wrote:
>> On 05/21/2018 02:55 PM, Dmitrij Pochepko wrote:
>>> webrev: http://cr.openjdk.java.net/~dpochepk/8196402/webrev.01/
>>>
>>> CR: https://bugs.openjdk.java.net/browse/JDK-8196402
>> This looks basically reasonable, given that it seems to be a somewhat
>> straightforward adaptation of the x86 code.
>>    However, a couple of
>> things:
>>
>> The assembler changes should be parameterized as much as possible.
>> For example, fmlavvd, fmulxvsd, and fmulxssd are cases of the "AdvSIMD
>> vector x indexed element" class.  fmlavvd is a case of "AdvSIMD three
>> same", some of which already exist.  If you can extend an existing
>> group of instructions, please do.
> I couldn't merge new instructions with existing (encoding looks quite 
> different), but I've managed to parametrize several new instructions 
> into more compact code.

Looks much better.

>>
>> Given that this is an adaptation of the Intel code, please do two
>> things:
>>
>> Put it in the same place that corresponds to its position in the x86
>> files.
>>
>> Keep Intel's copyright.
> done.

And add your own.  With the current date:-)

>> Thanks.
>>
> Please take a look at 2nd webrev: 
> http://cr.openjdk.java.net/~dpochepk/8196402/webrev.02/
> I've tested modified patch with jdk jtreg tests.

This comment looks wrong:

  41 //    Get B~1/mx based on the output of rcpss instruction (B0)

Otherwise OK.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671