On Mon, 9 Sep 2024 18:30:21 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:

>> Roman Kennke has updated the pull request incrementally with six additional 
>> commits since the last revision:
>> 
>>  - Print as warning when UCOH doesn't match in CDS archive
>>  - Improve initialization of mark-word in CDS ArchiveHeapWriter
>>  - Simplify getKlass() in SA
>>  - Simplify oopDesc::init_mark()
>>  - Get rid of forward_safe_* methods
>>  - GCForwarding touch-ups
>
> src/hotspot/share/oops/markWord.inline.hpp line 90:
> 
>> 88:   ShouldNotReachHere();
>> 89:   return markWord();
>> 90: #endif
> 
> Is the ifdef _LP64 necessary, since UseCompactObjectHeaders should always be 
> false for 32 bits?

Kindof. The problem is that klass_shift is larger than 31, and shifting with it 
would thus be UB and generate a compiler warning. I opted to simply not compile 
any of that code in 32bit builds. We could also define klass_shift differently 
on 32bit.
Long-term (maybe with Lilliput2/4-byte-headers?) it would be nice to 
consolidate the header layout between 32 and 64 bit builds and not make any 
distinction anywhere. E.g. define markWord (or objectHeader?) in a single way, 
and use that to extract all the relevant stuff. It's not totally unlikely that 
we deprecate 32-bit builds before that can happen, though.

> src/hotspot/share/oops/oop.inline.hpp line 90:
> 
>> 88:   } else {
>> 89:     return markWord::prototype();
>> 90:   }
> 
> Could this be unconditional since prototoype_header is initialized for all 
> Klasses?

yes, but there is ongoing effort (at Oracle) to get rid of 
```Klass::_prototype_header``` altogether. Let's wait for that and see how it 
looks then.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1765003983
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1765006669

Reply via email to