On Thu, 22 Jan 2026 12:59:45 GMT, Marc Chevalier <[email protected]> wrote:

>> This PR remove from `InlineTypeNode` the methods:
>> 
>> int     field_offset(uint index) const;
>> uint    field_index(int offset) const;
>> ciType* field_type(uint index) const;
>> bool    field_is_flat(uint index) const;
>> bool    field_is_null_free(uint index) const;
>> bool    field_is_volatile(uint index) const;
>> int     field_null_marker_offset(uint index) const;
>> 
>> in favor of a single
>> 
>> ciField*      field(uint index) const;
>> 
>> from which we directly access the various properties we are interested in.
>> 
>> I've called it `field` as suggested in the original discussion. I've started 
>> with `declared_nonstatic_field_at` to be consistent with `ciInstanceKlass` 
>> and to avoid ambiguity wrt which kind of fields we are talking about, but 
>> this name is rather long... Since we are only talking about the declared 
>> fields, and we have no easy getter for `nonstatic_field_at`, then I think 
>> it's ok to have a shorter name. Also, in the context of a `InlineTypeNode`, 
>> the "non-static" part of the long name seems rather redundant.
>> 
>> The other question is what we should do with asserts? Methods 
>> `field_is_flat`, `field_is_null_free` and `field_is_volatile` had the assert
>> 
>> assert(!field->is_flat() || field->type()->is_inlinetype(), "must be an 
>> inline type");
>> 
>> and `field_null_marker_offset` had
>> 
>> assert(field->is_flat(), "must be an inline type");
>> 
>> 
>> I've tried to propagate them to the call-site of such functions, and where 
>> it makes a bit of sense, and not subsumed by surrounding `if()` and 
>> `assert`. Let me know if it seems unnecessary or not well-placed in some 
>> cases.
>> 
>> Tested with 
>> tier1,tier2,tier3,hs-precheckin-comp,hs-comp-stress,valhalla-comp-stress. 
>> Looks fine, but it doesn't seem too risky.
>> 
>> Thanks,
>> Marc
>
> Marc Chevalier has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Copyright

Marked as reviewed by thartmann (Committer).

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

PR Review: 
https://git.openjdk.org/valhalla/pull/1922#pullrequestreview-3692333438

Reply via email to