On Tue, 27 May 2025 22:30:17 GMT, Sandhya Viswanathan
<sviswanat...@openjdk.org> wrote:
>> Mohamed Issa has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add new set of cbrt micro-benchmarks
>
> src/hotspot/cpu/x86/macroAssembler_x86.hpp line 1251:
>
>> 1249: void movapd(XMMRegister dst, Address src) { Assembler::movapd(dst,
>> src); }
>> 1250: void movapd(XMMRegister dst, AddressLiteral src, Register rscratch =
>> noreg);
>> 1251:
>
> You could write it as:
> using Assembler::movapd;
> void movapd(XMMRegister dst, AddressLiteral src, Register rscratch = noreg);
Ok, this is updated now.
> src/hotspot/cpu/x86/macroAssembler_x86.hpp line 1323:
>
>> 1321: void unpckhpd(XMMRegister dst, XMMRegister src) {
>> Assembler::unpckhpd(dst, src); }
>> 1322: void unpcklpd(XMMRegister dst, XMMRegister src) {
>> Assembler::unpcklpd(dst, src); }
>> 1323:
>
> Do we need these declarations here?
No, they were superfluous as cbrt stub generator could already access them. I
removed them now.
> src/hotspot/cpu/x86/stubGenerator_x86_64_cbrt.cpp line 43:
>
>> 41: //
>> 42: // Special cases:
>> 43: // cbrt(NaN) = quiet NaN, and raise invalid exception
>
> No exception is raised so the comment needs to be corrected.
This is corrected now.
> src/hotspot/cpu/x86/stubGenerator_x86_64_cbrt.cpp line 226:
>
>> 224: __ andl(rcx, 248);
>> 225: __ lea(r8, ExternalAddress(rcp_table));
>> 226: __ movsd(xmm4, Address(r8, rcx, Address::times_1));
>
> This address and other instructions using similar address could be written as
> Address(rcx, r8, Address::times_1).
Ok, I have changed the order, so rcx is viewed as base and r8 is viewed as
index now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24470#discussion_r2112516974
PR Review Comment: https://git.openjdk.org/jdk/pull/24470#discussion_r2112518997
PR Review Comment: https://git.openjdk.org/jdk/pull/24470#discussion_r2112520793
PR Review Comment: https://git.openjdk.org/jdk/pull/24470#discussion_r2112520486
