RFR: 8308396: Fix offset_of conversion warnings in runtime code [v4]
Frederic Parain
fparain at openjdk.org
Fri May 19 15:11:53 UTC 2023
On Fri, 19 May 2023 12:49:35 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:
>> Please review this change to use ByteSize and byte_offset_of() to refer to offsets to metadata and other types that are used in generated code. This avoids the int narrowing conversion warning for -Wconversion. There were a couple that I just added an (int) cast instead because these offsets are either being used in other code currently being changed (in oopDesc) or there are too many (like displaced_header_offset_in_bytes) and should be their own change.
>>
>> This change takes a chunk out of the -Wconversion warnings - see CR for more info.
>>
>> It might be easier and less tedious to review the commits separately. One commit renames blah_offset_in_bytes to blah_offset, since in_bytes(blah_offset()) is typically used (except in Address constructor which has an overload for ByteSize).
>>
>> Tested with tier1-4, x86 and aarch64, and built linux-x64-zero linux-x64-zero-debug linux-aarch64-debug linux-s390x-open linux-arm32-debug linux-ppc64le-debug linux-riscv64-debug locally.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>
> Fix indentation
Looks good to me, only a few comments on the style.
src/hotspot/cpu/ppc/macroAssembler_ppc.cpp line 2741:
> 2739: // Current thread already owns the lock. Just increment recursions.
> 2740: Register recursions = displaced_header;
> 2741: ld(recursions, in_bytes(ObjectMonitor::recursions_offset()-ObjectMonitor::owner_offset()), temp);
Minus sign should be surrounded by spaces.
src/hotspot/cpu/ppc/macroAssembler_ppc.cpp line 2743:
> 2741: ld(recursions, in_bytes(ObjectMonitor::recursions_offset()-ObjectMonitor::owner_offset()), temp);
> 2742: addi(recursions, recursions, 1);
> 2743: std(recursions, in_bytes(ObjectMonitor::recursions_offset()-ObjectMonitor::owner_offset()), temp);
Minus sign should be surrounded by spaces.
src/hotspot/cpu/x86/macroAssembler_x86.cpp line 5119:
> 5117: movptr(holder, Address(method, Method::const_offset())); // ConstMethod*
> 5118: movptr(holder, Address(holder, ConstMethod::constants_offset())); // ConstantPool*
> 5119: movptr(holder, Address(holder, ConstantPool::pool_holder_offset())); // InstanceKlass*
Comment could be aligned with the comment of the line above.
-------------
Marked as reviewed by fparain (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14053#pullrequestreview-1434536063
PR Review Comment: https://git.openjdk.org/jdk/pull/14053#discussion_r1199029302
PR Review Comment: https://git.openjdk.org/jdk/pull/14053#discussion_r1199029485
PR Review Comment: https://git.openjdk.org/jdk/pull/14053#discussion_r1199052897
More information about the graal-dev
mailing list