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