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