On Thu, 27 Aug 2026 09:06:22 GMT, Andrew Haley <[email protected]> wrote:
>> Fixes a small bug in class space allocation on aarch64: >> >> We attempt to allocate for movk-compatible mode by allocating a base that is >> 32-bit aligned and < 2ˆ48 (so, bits [32,48) are allowed). >> >> https://github.com/openjdk/jdk/blob/51bb52c5c0a5a764dd080dc23bed0829fd0ae638/src/hotspot/cpu/aarch64/compressedKlass_aarch64.cpp#L102-L104 >> >> However, the decoding then uses the right-shifted base before the movk. So >> the base would have to be [32+shift, 48+shift) to work with that: >> >> https://github.com/openjdk/jdk/blob/51bb52c5c0a5a764dd080dc23bed0829fd0ae638/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp#L5612-L5620 >> >> With +COH, shift can be up to 10, whereas before COH it usually was 0. >> >> So the base we allocated may not actually be usable for movk mode. E.g. if >> we allocated at 0x5'0000'0000, the right-shifted base would be 0x500'000, >> which spills into the lower 32 bits reserved for the narrowKlass offset. >> >> [JDK-8387962](https://bugs.openjdk.org/browse/JDK-8387962) sort of handles >> this now: before that patch, we would abort, but now we use the slightly >> less optimal fallback decode mode. Still, it should be fixed, and that would >> also simplify the decoding. >> >> --- >> >> Testing: tier1 on aarch64; gtests on aarch64. Note that the gtests are very >> thorough and test decoding and encoding for all possible corner cases. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5612: > >> 5610: case KlassDecodeMovk: { // 1-2 instructions >> 5611: lsl(dst, src, shift); >> 5612: movk(dst, (uint64_t)base >> 32, 32); > > This looks wrong, because we lose the top bits of `dst`. If we assert > somewhere that the top bits must be 0, that would help. And we add a comment > to the assertion here. That cannot happen. src is valid narrowKlass and a narrowKlass left-shifted is never larger than 4GB since it is tied to the maximum class space size. See e.g. assertion here (max_klass_range_size() is capped at 4GB): https://github.com/openjdk/jdk/blob/76e3d4da924101556fe2a4ab84e3980964a28d75/src/hotspot/share/oops/compressedKlass.cpp#L222-L227 We also have pretty thorough tests that check that the highest possible narrowKlass that can occur (a Klass living right at the end of a maximally expanded class space) can still be round-trip encoded. Note that this tests the MacroAssembler functions, not the C++ implementation: https://github.com/openjdk/jdk/blob/76e3d4da924101556fe2a4ab84e3980964a28d75/test/hotspot/gtest/aarch64/test_assembler_aarch64.cpp#L575-L599 I can, of course, add a debug-only check to the decoding. But it would increase the generated code size in debug builds, and I like memory sizes between debug and release to be somewhat similar, so that we test the same things we ship. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32520#discussion_r3871563622
