On Tue, 20 Jan 2026 11:30:38 GMT, Quan Anh Mai <[email protected]> wrote:

>> Some code added by 
>> [JDK-8372700](https://bugs.openjdk.org/browse/JDK-8372700) can compute the 
>> constant value of a field of a (flatten) element in a flat array. We get a 
>> crash when the element of the array is known to be `null`, and so the field 
>> doesn't exist.
>> 
>> So, let's just check in `ciConstant ciFlatArray::field_value(int index, 
>> ciField* field)` whether we get a null constant before interpreting it as a 
>> `ciInstance` and trying to retrieve a field from there. This should be 
>> enough since a `ciObject` is (directly) derived by `ciNullObject`, 
>> `ciInstance` and `ciArray`. Since we are looking up a value of a flat array, 
>> an element cannot be a `ciArray` (arrays have identities and can't be 
>> contained in a flat array). After looking up whether the flat array element 
>> is null, the `obj->as_instance()` cast acts as an assert, should we ever add 
>> another derived class from `ciObject`.
>> 
>> In case of a null array element, `field_value` simply returns an invalid 
>> `ciConstant`.
>> 
>> Tested with 
>> tier1,tier2,tier3,hs-precheckin-comp,hs-comp-stress,valhalla-comp-stress. 
>> Looks good.
>> 
>> Thanks,
>> Marc
>
> src/hotspot/share/ci/ciFlatArray.cpp line 136:
> 
>> 134: 
>> 135: ciConstant ciFlatArray::field_value(int index, ciField* field) {
>> 136:   auto get_field_from_object_constant = [field](const ciConstant& v) -> 
>> ciConstant {
> 
> I don't really agree with this fix, `ciFlatArray::field_value` should be 
> dumber, it is the caller who knows that we do not fold the load if the 
> element is `null`, the callee should just return the field as it is.

I fear I don't understand. Let's say, I have a flat array `MyValue[] arr` where 
`MyValue` is a value class with a single field `f`. Let's also assume `arr[0] 
== null`, `arr.field_value(0, f)` (assuming the `ci...` versions of it with 
matching names) tries to get the constant value of the field `f` of `arr[0]`, 
and `arr[0].f` is not null, it's rather undefined. It's not about stability and 
folding. On the other hand, if `arr[0]` is not null, but `arr[0].f` is null, 
`arr.field_value(0, f)` already returns `null` (the `ciConstant` that means 
that).

Am I missing something?

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1923#discussion_r2708233184

Reply via email to