[lworld] RFR: 8375441: [lworld] C2: assert(is_instance()) failed: bad cast [v2]

[Marc Chevalier]

On Wed, 28 Jan 2026 09:19:18 GMT, Marc Chevalier <mchevalier at openjdk.org> 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
>
> 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

While we are allowed to consider non-zero fields as constant, since they can be updated concurrently, so we need to cache them to get consistent value during computation. But that's not enough. In the case of a stable flat field, we need to cache the whole (declared) field at once, even if we need only a sub-field of it, because later during the compilation, we may want to see the other field, and then we must make sure not to observe inconsistent values across the sub-fields of the same flat field. This new implementation will find the declared field containing the requested field, copy the whole flat field, and access the field from the copy. The same goes for flat array: we cache the whole element at the given index, and access the field from that.

I've also made the expansion of flat loads more robust: expanding them non-atomically was correct as long as all the generated load can be replaced by a constant during the compilation. Otherwise, we would get non-atomic access at runtime which is not the desired behavior. Rather than hoping or asserting, I get the constant value from which the flat load is getting fields, and directly get the constant values of the requested fields in a new version of the expansion. This is simpler (we don't create load nodes to simplify), and safer (no risk of leaving a non-atomic load behind). Same tests as the original PR description are still passing.

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

PR Comment: https://git.openjdk.org/valhalla/pull/1923#issuecomment-3810049635