[lworld] RFR: 8374006: [lworld] Improve field accessor methods in InlineTypeNode
Quan Anh Mai
qamai at openjdk.org
Tue Jan 20 11:35:26 UTC 2026
On Fri, 16 Jan 2026 13:15:54 GMT, Marc Chevalier <mchevalier at openjdk.org> 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
LGTM
-------------
Marked as reviewed by qamai (Committer).
PR Review: https://git.openjdk.org/valhalla/pull/1922#pullrequestreview-3681730613
More information about the valhalla-dev
mailing list