RFR: 8339694: ciTypeFlow does not correctly handle unresolved constant dynamic of array type [v5]

Vladimir Kozlov kvn at openjdk.org
Thu Oct 17 16:58:59 UTC 2024


On Thu, 17 Oct 2024 14:13:28 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> After [JDK-8280473](https://bugs.openjdk.org/browse/JDK-8280473), the CI will represent an unresolved constant dynamic as unloaded `ciConstant` of a `java/lang/Object` `ciInstanceKlass`:
>> 
>> https://github.com/openjdk/jdk/blob/6133866150cf6131ab578f1537f84c239703fa67/src/hotspot/share/ci/ciEnv.cpp#L721-L723
>> 
>> Now with a constant dynamic of array type, we hit an assert in type flow analysis on `*astore` because the type is not a primitive array type:
>> 
>> https://github.com/openjdk/jdk/blob/6133866150cf6131ab578f1537f84c239703fa67/src/hotspot/share/ci/ciTypeFlow.cpp#L967-L971
>> 
>> ->
>> https://github.com/openjdk/jdk/blob/6133866150cf6131ab578f1537f84c239703fa67/src/hotspot/share/ci/ciTypeFlow.hpp#L343-L346
>> 
>> The same problem exists with object array types. New `TestUnresolvedConstantDynamic` triggers both.
>> 
>> Similar to `unloaded_ciinstance` used by [JDK-8280473](https://bugs.openjdk.org/browse/JDK-8280473) (see [here](https://github.com/openjdk/jdk/commit/88fc3bfdff7f89a02fcfb16909df144e6173c658#diff-7c032de54e85754d39e080fd24d49b7469543b163f54229eb0631c6b1bf26450R784)), we could now add an unloaded `ciObjArray` and `ciTypeArray` to represent an unresolved dynamic constant of array type. We would then add a trap when parsing the array access. However, this requires rather invasive changes for this edge case and it wouldn't make much sense because even though type flow analysis would continue, we would add a trap when parsing `_ldc` anyway if the constant is not loaded: 
>> https://github.com/openjdk/jdk/blob/037f11b864734734dd7fbce029b2e8b4bc17f3ab/src/hotspot/share/opto/parse2.cpp#L1962-L1979
>> 
>> I propose to do the same in type flow analysis, i.e., trap early in `_ldc` when the constant is not loaded.
>> 
>> Thanks,
>> Tobias
>
> Tobias Hartmann has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits:
> 
>  - Merge with master
>  - More cleanups
>  - Modified ciTypeFlow::can_trap
>  - Missed a return
>  - First prototype
>    
>    Fix

Looks good.

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

Marked as reviewed by kvn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21470#pullrequestreview-2375788874


More information about the hotspot-compiler-dev mailing list