RFR: 8189105: AARCH64: create intrinsic for sin and cos
Andrew Haley
aph at redhat.com
Tue Jun 19 17:46:00 UTC 2018
On 06/18/2018 07:53 PM, Dmitrij Pochepko wrote:
> If you have any other ideas how to make the code easier to read and
> maintain I’m happy to improve it.
That's great! Really, that's a vast improvement. Excellent work.
One or two relatively minor things:
Why do you say the polynomial in kernel_sin/kernel_cos is a Taylor
series? There's no such comment in sharedRuntimeTrig.cpp, and I doubt
it. I would have thought it was a least-maximum polynomial
approximation, not a Taylor series.
What's the point of keeping the doubles in tables in hex form?
sharedRuntimeTrig.cpp doesn't do that, and it looks like obfuscation.
Even if you must keep them as hex, at least move the comment which
shows their true values next to their declaration, but I really can't
see the point.
It'd make the code much more readable if you lined up the comments,
like so:
fmovd(v28, v3); // t = r
fmuld(v29, v2, v6); // w = v29 = fn * pio2_2
fsubd(v3, v28, v29); // r = t - w
fsubd(v31, v28, v3); // v31 = (t - r)
fsubd(v31, v29, v31); // v31 = w - (t - r) = - ((t - r) - w)
fmaddd(v26, v2, v7, v31); // v26 = w = fn*pio2_2t - ((t - r) - w)
fsubd(v4, v3, v26); // y[0] = r - w
fmovd(jx, v4);
lsl(jx, jx, 1);
sub(tmp3, tmp5, jx, LSR, 32 + 20 + 1); // r7 = j-(((*(i0+(int*)&y[0]))>>20)&0x7ff);
But it's probably not a great idea to make the code-super wide with
the comments so you have to open a huge editor window. So, don't
go crazy, but a little bit of white space like this makes it much
easier to read.
Thanks.
--
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
More information about the hotspot-compiler-dev
mailing list