On Tue, 10 Sep 2024 16:26:38 GMT, Sandhya Viswanathan
<sviswanat...@openjdk.org> wrote:
>> Srinivas Vamsi Parasa has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> update libm tanh reference test with code review suggestions
>
> src/hotspot/cpu/x86/c1_LIRGenerator_x86.cpp line 810:
>
>> 808: x->id() == vmIntrinsics::_dpow || x->id() == vmIntrinsics::_dcos
>> ||
>> 809: x->id() == vmIntrinsics::_dsin || x->id() == vmIntrinsics::_dtan
>> ||
>> 810: x->id() == vmIntrinsics::_dlog10 || x->id() ==
>> vmIntrinsics::_dtanh) {
>
> Need to have the tanh under #Ifdef _LP64 as we are generating stub only for
> 64 bit.
Please see the newly added `#ifdef `in the updated code.
> src/hotspot/cpu/x86/c1_LIRGenerator_x86.cpp line 1000:
>
>> 998: if (StubRoutines::dtanh() != nullptr) {
>> 999: __ call_runtime_leaf(StubRoutines::dtanh(), getThreadTemp(),
>> result_reg, cc->args());
>> 1000: } // TODO: else clause?
>
> You could instead have an assert here that StubRoutines::dtanh() is not null.
> Thereby no need for the else clause.
Please see the newly added assert in the updated code.
> src/hotspot/cpu/x86/templateInterpreterGenerator_x86_32.cpp line 376:
>
>> 374: // [ hi(arg) ]
>> 375: //
>> 376: if (kind == Interpreter::java_lang_math_tanh) {
>
> Need to update the copyright year to 2024 in this file.
Please see the year updated to 2024 in the updated code.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20657#discussion_r1752962967
PR Review Comment: https://git.openjdk.org/jdk/pull/20657#discussion_r1752962670
PR Review Comment: https://git.openjdk.org/jdk/pull/20657#discussion_r1752963446
