On Mon, 25 Aug 2025 07:17:16 GMT, Tobias Hartmann <[email protected]> wrote:
>> Marc Chevalier has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Rephrase summary >> - +copyright > > src/hotspot/share/ci/ciType.cpp line 151: > >> 149: assert(type->is_inlinetype() >> 150: // An abstract value type is an instance_klass >> 151: || (type->is_instance_klass() && >> type->as_instance_klass()->flags().is_abstract() && >> !type->as_instance_klass()->flags().is_identity()) > > Just wondering, is the `type->as_instance_klass()->flags().is_abstract()` > condition even needed? Same in `ciTypeFlow::get_start_state()`. Probably not. An instance class without identity must be an abstract value class. At least, I don't see another way... But just to be sure, I'm running some tests. > test/hotspot/jtreg/compiler/valhalla/inlinetypes/LarvalDetectionAboveOSR.java > line 29: > >> 27: * @summary In OSR compilation, value objects coming from above the OSR >> start >> 28: * must be correctly found to be early larval so we know they are >> 29: * immutable, and they can be scalarized. > > I suggest to rephrase this: "to be early larval so we know they are > immutable" is confusing because early larvals are **mutable**. Wow, yeah, I see what I wanted to say, but it's terrible! I've tried something else, hopefully better. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1531#discussion_r2297532414 PR Review Comment: https://git.openjdk.org/valhalla/pull/1531#discussion_r2297539481
