RFR: 8139457: Array bases are aligned at HeapWord granularity [v38]
Roman Kennke
rkennke at openjdk.org
Mon Jul 3 18:30:12 UTC 2023
On Mon, 3 Jul 2023 14:44:47 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:
>> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Build fixes
>
> src/hotspot/cpu/arm/c1_MacroAssembler_arm.hpp line 50:
>
>> 48:
>> 49: void allocate_object(Register obj, Register tmp1, Register tmp2, Register tmp3,
>> 50: int header_size_in_bytes, int object_size,
>
> I don't see the related change in `C1_MacroAssembler::allocate_object` definition. At very least, it affects the assert that compares with `object_size` (in words?).
Hrmpf, this is actually wrong. This method accepts the header size in *words*. We do, in-fact, jump through some hoops to get it word-sized:
__ allocate_object(dst, scratch1, scratch2, scratch3, scratch4,
--> checked_cast<int>(heap_word_size(instanceOopDesc::base_offset_in_bytes())),
instance_size, klass_reg, !klass->is_initialized(), slow_path);
However, it is a discrepancy vs allocate_array() which (now) accepts byte-sized header-size.
OTOH, none of the implementations of allocate_object() seem to actually use this information, except for the assert, which seems quite bogus. Also, for objects, the header-size (or base-offset) would always be the same, there seems no need to pass it around. Maybe this warrants a larger refactoring to get rid of that header_size argument to begin with? Maybe as separate PR?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/11044#discussion_r1251178920
More information about the hotspot-dev
mailing list