On Fri, 16 Jan 2026 13:15:54 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 Nice cleanup! Looks good to me. Some files might need copyright updates though. ------------- Marked as reviewed by thartmann (Committer). PR Review: https://git.openjdk.org/valhalla/pull/1922#pullrequestreview-3681856159
