On Mon, 2 Feb 2026 16:33:05 GMT, Quan Anh Mai <[email protected]> wrote:
>> Marc Chevalier has updated the pull request incrementally with 10 additional
>> commits since the last revision:
>>
>> - Details
>> - exception handling
>> - oops
>> - Also cache correctly whole stable fields
>> - fix comment
>> - Remove debug
>> - ShouldNotReachHere
>> - explicit casts
>> - Clean up
>> - expand_constant
>
> src/hotspot/share/ci/ciInstance.cpp line 154:
>
>> 152: return ciConstant();
>> 153: }
>> 154: if (field->original_holder() == nullptr) {
>
> I think simply comparing `field` and `containing_field` is enough.
I thought too, but then, I was surprised.
For instance from `GraphBuilder::access_field` (that does `ciConstant
field_value = field->constant_value_of(const_oop);`), we get the field from
`ciField* field = stream()->get_field(will_link);`. Deep inside, the field is
created in `ciField* ciEnv::get_field_by_index_impl(ciInstanceKlass* accessor,
int index, Bytecodes::Code bc)` and put in the `field_cache` (whose role is not
clear to me). The interesting part is that it's a field created here.
And later, when the fields are computed by `void
ciInstanceKlass::compute_nonstatic_fields()`, new `ciField` objects are
created, but the field_cache ones aren't reused. In such a case, I get a
`field` and `containing_field` that have identical content but with different
pointers. If I simply compare the pointers, the `field` is wrongly interpreted
as a sub-field of `containing_field`. I guess I could compare their symbols
(since offset and signatures might not be enough), but that doesn't seem much
better to me.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1923#discussion_r2758606244
