On Tue, 24 Mar 2026 14:31:12 GMT, Frederic Parain <[email protected]> wrote:

>> src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 1269:
>> 
>>> 1267:       required_alignment = nullable_atomic_layout_size_in_bytes();
>>> 1268:     }
>>> 1269:     int shift = (required_alignment - (first_field->offset() % 
>>> required_alignment)) % required_alignment;
>> 
>> Is there an align method that does some of this expression like:
>> required_alignment = required_alignment - align_up(field_offset, 
>> required_alignment) ?
>> no that's wrong.
>> Can't read this!  Are the () in the right place?
>> required_alignment - (first_field->offset() % required_alignment).  must be 
>> < required_alignment so why is the last % needed?
>
> Because when the payload is already correctly aligned, we want a shift of 
> zero.
> For example, the payload starts at offset 12, and the alignment constraint is 
> 1.
> Without the trailing modulo operation: => 1 - (12%1) = 1 - 0  = 1 => Payload 
> is shifted
> With the trailing modulo operation: => (1 - (12%1))%1 = (1 - 0)%1 = 1%1 = 0 
> => Payload stays where it is

Ok, I cannot think of a comment that would help that.

>> src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 1385:
>> 
>>> 1383:   Array<int>* super_map = ik->acmp_maps_array();
>>> 1384:   assert(super_map != nullptr, "super class must have an acmp map");
>>> 1385:   int nb_nonoop_field = super_map->at(0);
>> 
>> Is 'nb' next block?
>
> No, number. In the acmp_map format described in classFileParser.cpp, the 
> first element of the array is `number_of_nonoop_entries`.

oh right, can you rename 'nb' to 'num' for me?  nb is so many different things. 
 It's just 3 places.

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2248#discussion_r2983171391
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2248#discussion_r2983179043

Reply via email to