On Thu, 29 Aug 2024 22:29:43 GMT, Dean Long <dl...@openjdk.org> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add in graal flags and a comment.
>
> src/hotspot/share/ci/ciKlass.cpp line 231:
>
>> 229: // ------------------------------------------------------------------
>> 230: // ciKlass::misc_flags
>> 231: jint ciKlass::misc_flags() {
>
> Suggestion:
>
> u1 ciKlass::misc_flags() {
>
> I think this should match what misc_flags() returns. Ideally, I think it
> should be a typedef like KlassFlags_t so we don't have to make a lot of
> changes if grows to u2.
Also, using the correct, narrowed type will help with -Wconversion warnings
later, because u1 can be converted to both int and size_t without a cast.
> src/hotspot/share/oops/klass.hpp line 436:
>
>> 434: #endif
>> 435: static ByteSize bitmap_offset() { return
>> byte_offset_of(Klass, _bitmap); }
>> 436: static ByteSize misc_flags_offset() { return
>> byte_offset_of(Klass, _misc_flags); }
>
> Suggestion:
>
> static ByteSize misc_flags_offset() { return
> byte_offset_of(Klass, _misc_flags._flags); }
We probably shouldn't assume the _flags field starts at offset 0.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20719#discussion_r1737296853
PR Review Comment: https://git.openjdk.org/jdk/pull/20719#discussion_r1737306718
