On Tue, 24 Mar 2026 13:28:51 GMT, Coleen Phillimore <[email protected]> wrote:

>> Frederic Parain has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Apply suggestion from review
>
> 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

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

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

Reply via email to