Replace some Valhalla flags with JAVA_SPEC_VERSION >= 28

[keithc-ca]

Uses of these flags are replaced by JAVA_SPEC_VERSION >= 28:

• INLINE-TYPES
• J9VM_OPT_VALHALLA_STRICT_FIELDS
• J9VM_OPT_VALHALLA_VALUE_TYPES

In some conditions, the Valhalla portion disappears; e.g. (JAVA_SPEC_VERSION >= 16) | INLINE-TYPES simplifies to JAVA_SPEC_VERSION >= 16.

Issue: #24484.

[keithc-ca]

I question the clang-format check:

13:55:26  -#endif /* JAVA_SPEC_VERSION >= 15 */
13:55:26  +#endif  /* JAVA_SPEC_VERSION >= 15 */

Why should two spaces be expected after that #endif while line 237 (and several others) have only one?

#endif /* defined(J9VM_OPT_JITSERVER) */

[hangshao0]

This PR is touching JIT and GC code, inviting @hzongaro and @dmitripivkine to review.

[dmitripivkine]

Personally I am not a fan of this kind of changes. Definitions like J9VM_OPT_VALHALLA_VALUE_TYPES specifies feature type we have deal with, but check for java version makes it faceless. After such replacement we can not search for code introduced for feature.
The majority of this code is in VM, so I would like to have @tajila opinion.
If this is team decision I am kind of ok with it.

[hangshao0]

It seems that JVM_VirtualThreadHideFrames has been removed since JDK24. so there is no need to use || (JAVA_SPEC_VERSION >= 28) here.

[keithc-ca]

Fixed in 7208d4b.

[tajila]

The majority of this code is in VM, so I would like to have @tajila opinion.
If this is team decision I am kind of ok with it.

I agree Im not a fan of the of replacing feature flags with JDKVERSION flags. It removes valuable information and makes it harder to understand the code.

[keithc-ca]

I'm not a fan of the of replacing feature flags with JDKVERSION flags. It removes valuable information and makes it harder to understand the code.

That ship sailed years ago. We have many, many JPEs/features guarded by the Java version in which they were first introduced. Adopting a different stance for JEPs 401 & 539 creates inconsistency.

I can rework this to keep the flags in (generated) j9cfg.h, but just remove the cmake configuration points. Would that be acceptable?

[hzongaro]

As this is a preview feature, will we need a way of turning the feature on or off from the command line? We'd previously just relied on its being always enabled or always disabled with J9VM_OPT_VALHALLA_VALUE_TYPES.

[hangshao0]

As this is a preview feature, will we need a way of turning the feature on or off from the command line?

The option to control this is --enable-preview. I think we can add a check here whether preview is enabled or not, but this can be done in a separate PR.

[keithc-ca]

That's a good observation. I expect the right answer would be to check for

    J9_ARE_ANY_BITS_SET(vm->extendedRuntimeFlags2, J9_EXTENDED_RUNTIME2_ENABLE_PREVIEW)

in this and similarly sensitive functions. I think that deserves to be a separate change.

[hzongaro]

The JIT compiler changes look fine to me. I'm OK with either JAVA_SPEC_VERSION >= 28 or defined(J9VM_OPT_VALHALLA_...).

[tajila]

That ship sailed years ago. We have many, many JPEs/features guarded by the Java version in which they were first introduced. Adopting a different stance for JEPs 401 & 539 creates inconsistency.

I dont think this makes it any more inconsistent. We still use flags like J9VM_OPT_OPENJDK_METHODHANDLE, J9VM_OPT_SHARED_CLASSES, J9VM_OPT_METHOD_HANDLE and others.

In any case, I dont think consistency is a reason to make things worse. Especially for a feature as complex as this, keeping the original flags makes it easier to find relevant code.

If consistency is desired, Im more inclined to go the other way and re-introduce the feature flags.

[dmitripivkine]

I wondering what we are going to do (after this change) if this feature is going to be postponed in Java 28, even it has been announced?