On Mon, 23 Mar 2026 13:32:20 GMT, Frederic Parain <[email protected]> wrote:

>> The main change in this patch is the use of the C++ acmp_map of the super 
>> class to generate the acmp_map of a sub-class instead of using the Java heap 
>> allocated int array. There's another fix in the computation of the shift 
>> applied to the payload to satisfy atomicity alignment constraints. A few 
>> asserts have been added to improve the robustness of the code.
>> 
>> Testing in progress tiers 1 to 4.
>> 
>> Thank you,
>> 
>> Fred
>
> 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?

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?

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

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

Reply via email to