On Wed, 4 Mar 2026 16:56:22 GMT, Axel Boldt-Christmas <[email protected]> wrote:
>> `obj_at(int)` on `flatArrayOopDesc` should never be called as it may fail >> with an out of memory error and crash. Similarly `objArrayOopDesc` should >> not provide a shared interface to this member function which is only valid >> to call on a `refArrayOopDesc`. >> >> This RFE cleans up the last potentially invalid uses by using and asserting >> stricter types at the call sites. >> >> Most cleanup are straight forward, however there are a few parts where some >> extra feedback is appreciated. >> >> First the primate caches (ac32b67e811945ed474ac62d07275e8a464e72b9). >> [JDK-8369921](https://bugs.openjdk.org/browse/JDK-8369921)/#1685 conditioned >> the use of the caches behind the `!PreviewFeatures.isEnabled()` such that >> they are not used when running with Valhalla. However it did not do this for >> Character, even though both the issue and the PR mentions this Class as >> well, and could not find any discussion in the PR. >> >> Update: @jsikstro pointed me to >> [JDK-8372619](https://bugs.openjdk.org/browse/JDK-8372619), I am curious to >> where and how we access these caches (except for `valueOf`), as this change >> which removes the caches in Valhalla seems to run through our testing. >> >> Second the caches were still created even if they were not used. In this PR >> I chose to not create these caches at all if we are in preview mode. Also I >> am not sure if it is the case that we always trust the internal final values >> and do fold them, but I changed it to use `@stable` checks vs a sentinel, >> which I know are trusted as final. However maybe this is unnecessary, and >> some core library / compiler person has some input. >> >> Second is all the JVMCI changes (fe98ae0dc03c99c770dad1364fb1f010ea1ee7bf). >> This patch changes it so that the internal JVMCI Classes that that hotspot >> knows about are required to be RefArrays (this was already the assumption, >> but this hardens this by changing their types). And for external arrays they >> either throw an exception if it is an unexpected type (the case for >> `executeHotSpotNmethod`) or use `obj_at(int, TRAP)` and returns with an out >> of memory error. >> >> Testing: >> * Running tier 1-7 with `--enable-preview' > > Axel Boldt-Christmas has updated the pull request with a new target base due > to a merge or a rebase. The pull request now contains 16 commits: > > - Deleted obj_at and added obj_at_is_null > - Merge remote-tracking branch 'upstream_valhalla/lworld' into > lworld-remove-obj-at > - Merge remote-tracking branch 'upstream_valhalla/lworld' into > lworld-remove-obj-at > - Remove unused obj_at > - HotSpotJVMCI should use RefArrayOops internally, and expect exceptions > externally > - Fix ZERO > - Fix jvmti_GetThreadGroupChildren > - Fix ForeignGlobals > - Fix java_lang_invoke_MethodType > - Fix oop_verify_on > - ... and 6 more: > https://git.openjdk.org/valhalla/compare/a6b20a25...e417b2a5 I think where it was used was removed later. I think it was used for constant folding. In case we had a constant object, we could need to query its null marker and replace a load by a constant with with `jboolean`, so I think it was the right type. But indeed, it was only for nullable things, but it made sense: we shouldn't have a load trying to get the null marker of a null-restricted thing in the first place. ------------- PR Comment: https://git.openjdk.org/valhalla/pull/2157#issuecomment-4003375680
