Remove Implicit creation work #22975

AditiS11 · 2025-11-19T12:24:24Z

Related: #22642
Remove the implicit creation work.
Update the ValhallaAttribute tests to follow the new spec.

theresa-m

First pass.

theresa-m · 2025-11-20T14:58:15Z

debugtools/DDR_VM/src/com/ibm/j9ddr/vm29/tools/ddrinteractive/RomClassWalker.java

 		}
-		if (J9ROMClassHelper.hasImplicitCreationAttribute(romClass)) {
-			classWalkerCallback.addSlot(clazz, SlotType.J9_SRP, cursor, "implicitCreationAttributeSRP");
-			implicitCreationAttributeDo(U32Pointer.cast(cursor.get()));


The implicitCreationAttributeDo should also be removed.

theresa-m · 2025-11-20T14:58:56Z

runtime/util/romclasswalk.c

-		rangeValid = callbacks->validateRangeCallback(romClass, cursor, sizeof(J9SRP), userData);
-		if (rangeValid) {
-			callbacks->slotCallback(romClass, J9ROM_SRP, cursor, "implicitCreationAttributeSRP", userData);
-			allSlotsInImplicitCreationAttributeDo(romClass, SRP_PTR_GET(cursor, U_32*), callbacks, userData);


The allSlotsInImplicitCreationAttributeDo method should also be removed.

theresa-m · 2025-11-20T15:03:06Z

runtime/bcutil/ClassFileOracle.cpp

 					_isClassValueBased = true;
 				}
-#if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES)
-				if (containsKnownAnnotation(foundAnnotations, LOOSELYCONSISTENTVALUE_ANNOTATION)) {


You can remove all the references to LOOSELYCONSISTENTVALUE_ANNOTATION as well since we aren't doing anything with it right now.

theresa-m · 2025-11-20T15:07:17Z

runtime/bcutil/ClassFileWriter.cpp

-#if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES)
-	if (J9_ARE_ALL_BITS_SET(_romClass->optionalFlags, J9_ROMCLASS_OPTINFO_IMPLICITCREATION_ATTRIBUTE)) {
-		U_32 implicitCreationFlags = (U_32)getImplicitCreationFlags(_romClass);
-		addEntry((void*) &IMPLICITCREATION, 0, CFR_CONSTANT_Utf8);


Remove the IMPLICITCREATION definition in line 71.

theresa-m · 2025-11-20T15:09:37Z

runtime/bcutil/ROMClassWriter.cpp

-ROMClassWriter::writeImplicitCreation(Cursor *cursor, bool markAndCountOnly)
-{
-	if (_classFileOracle->hasImplicitCreation()) {
-		cursor->mark(_implicitCreationSRPKey);


Remove _implicitCreationSRPKey from line 327.

theresa-m · 2025-11-20T15:11:51Z

runtime/oti/j9javaaccessflags.h

 #define J9_STATIC_FIELD_STRICT_INIT_WAS_WRITTEN(classAndFlags) \
 		(J9StaticFieldRefStrictInitWritten == ((classAndFlags) & J9StaticFieldRefStrictInitMask))
 #endif /* defined(J9VM_OPT_VALHALLA_STRICT_FIELDS) */



Please remove the extra space here as well.

theresa-m · 2025-11-20T15:12:30Z

runtime/oti/j9nonbuilder.h

 #define J9_ROMCLASS_OPTINFO_LOADABLEDESCRIPTORS_ATTRIBUTE 0x20000
 #endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
-#if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES)
-#define J9_ROMCLASS_OPTINFO_IMPLICITCREATION_ATTRIBUTE 0x40000


Instead of removing this rename it as UNUSED like the other values below.

theresa-m · 2025-11-20T15:34:56Z

runtime/nls/j9vm/j9vm.nls

 # END NON-TRANSLATABLE

 # NullRestricted is not translatable
-J9NLS_VM_NULLRESTRICTED_MUST_BE_IN_DEFAULT_IMPLICITCREATION_VALUE_CLASS=An instance field with a NullRestricted attribute must be in a value class with an implicit constructor


You can't rename NLS messages, create a new one at the end of the file.

theresa-m · 2025-11-20T15:41:17Z

runtime/gc_base/ObjectAccessBarrier.cpp

 	U_8 *elementAddress = (U_8*)indexableEffectiveAddress(vmThread, arrayRef, index, J9ARRAYCLASS_GET_STRIDE((J9Class *) arrayClazz));
 	IDATA elementOffset = (elementAddress - (U_8*)arrayRef);
 	J9Class *elementClazz = J9GC_J9OBJECT_CLAZZ_THREAD(srcObject, vmThread);
-	Assert_MM_true(J9_IS_J9CLASS_ALLOW_DEFAULT_VALUE(elementClazz));


Instead of removing these I think we can at least check that elementClazz is a value class.

theresa-m · 2025-11-20T15:48:01Z

runtime/vm/classsupport.c

 	 * Don't try to initialize class for null-restricted array creation since
 	 * the regular arrayClass will always be created first.
 	 */
-	if (J9_IS_J9CLASS_ALLOW_DEFAULT_VALUE(elementClass)


Please update the comment above this line as well.

theresa-m

Second pass. Please also remove the ImplicitCreationAttribute class in ValhallaUtils.java.

theresa-m · 2025-11-21T19:13:51Z

runtime/bcutil/attrlookup.h

 inline
 #endif
 #endif
-static size_t


please remove the type changes here and line 188.

theresa-m · 2025-11-21T19:14:00Z

runtime/bcutil/attrlookup.h

  return len + asso_values[(unsigned char)str[1]];
 }

-#ifdef __GNUC__


please keep this unchanged.

theresa-m · 2025-11-21T19:16:30Z

runtime/bcutil/cfreader.c

-				offset = address;
-				goto _errorFound;
-			}
-			implicitCreationAttributeRead = TRUE;


Please remove the implicitCreationAttributeRead declaration.

theresa-m · 2025-11-21T19:17:49Z

runtime/bcutil/cfreader.c

 		}

-#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
-		if (valueTypeClass) {


Did you mean to remove this?

theresa-m · 2025-11-21T19:22:22Z

runtime/gc_base/ObjectAccessBarrier.cpp

 	IDATA elementOffset = (elementAddress - (U_8*)arrayRef);
 	J9Class *elementClazz = J9GC_J9OBJECT_CLAZZ_THREAD(srcObject, vmThread);
-	Assert_MM_true(J9_IS_J9CLASS_ALLOW_DEFAULT_VALUE(elementClazz));
+	Assert_MM_true(J9_IS_J9CLASS_VALUEBASED(elementClazz));


You're looking for J9_IS_J9CLASS_VALUETYPE here. J9_IS_J9CLASS_VALUEBASED refers to the ValueBased annotation.

theresa-m · 2025-11-21T19:22:59Z

runtime/nls/cfre/cfrerr.nls

 J9NLS_CFR_ERR_PRELOAD_CLASS_ENTRY_NOT_CLASS_TYPE.user_response=Contact the provider of the classfile for a corrected version.
 # END NON-TRANSLATABLE

-# ImplicitCreation is not translatable


Please don't remove these, see notes at the top of the file.

theresa-m · 2025-11-21T19:23:54Z

runtime/nls/j9vm/j9vm.nls

 J9NLS_VM_CRIU_RESTORE_RESET_VIRTUALTHREAD_FORKJOINPOOL_PARALLELISM_INVALID_PARALLELISM_OFFSET.user_response=View CRIU documentation to determine how to resolve the exception.
 # END NON-TRANSLATABLE

-# NullRestricted is not translatable


These can't be removed, see notes at the top of the file.

theresa-m · 2025-11-21T19:31:11Z

runtime/oti/j9nonbuilder.h

 #endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
 #if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES)
-#define J9_ROMCLASS_OPTINFO_IMPLICITCREATION_ATTRIBUTE 0x40000
+#define J9_ROMCLASS_OPTINFO_IMPLICITCREATION_ATTRIBUTE_UNUSED_40000 0x40000


J9_ROMCLASS_OPTINFO_IMPLICITCREATION_ATTRIBUTE_UNUSED_40000 -> J9_ROMCLASS_OPTINFO_UNUSED_40000

theresa-m · 2025-11-21T19:37:32Z

runtime/bcutil/ROMClassWriter.cpp

 #endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
-#if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES)
-	if (_classFileOracle->hasImplicitCreation()) {
-		cursor->writeSRP(_implicitCreationSRPKey, Cursor::SRP_TO_GENERIC);


Please remove the declaration for _implicitCreationSRPKey as well as the comment SRP to ImplicitCreation attribute in ROMClassWriter.cpp.

theresa-m · 2025-11-21T19:43:57Z

test/functional/Valhalla/src/org/openj9/test/lworld/ValhallaAttributeTests.java

-	/* A class may have no more than one ImplicitCreation attribute. */
-	@Test(expectedExceptions = java.lang.ClassFormatError.class, expectedExceptionsMessageRegExp = ".*Multiple ImplicitCreation attributes.*")
-	public static void testMultipleImplicitCreationAttributes() throws Throwable {
-		ValhallaAttributeGenerator.generateClassWithTwoImplicitCreationAttributes("MultiImplicitCreationAttributes");


Please remove the ValhallaAttributeGenerator functions corresponding to the removed tests such as ValhallaAttributeGenerator.generateClassWithTwoImplicitCreationAttributes.

theresa-m · 2025-11-21T19:45:25Z

runtime/vm/createramclass.cpp

 						if (!J9ROMCLASS_IS_VALUE(valueROMClass)
-							|| J9_ARE_NO_BITS_SET(valueROMClass->optionalFlags, J9_ROMCLASS_OPTINFO_IMPLICITCREATION_ATTRIBUTE)
-							|| J9_ARE_NO_BITS_SET(getImplicitCreationFlags(valueROMClass), J9AccImplicitCreateHasDefaultValue)
 						) {


Please move this to the same line as the if statement.

theresa-m · 2025-11-21T19:48:41Z

runtime/vm/createramclass.cpp

-		if (J9_ARE_ALL_BITS_SET(romClass->optionalFlags, J9_ROMCLASS_OPTINFO_IMPLICITCREATION_ATTRIBUTE)) {
-			U_16 implicitCreationFlags = getImplicitCreationFlags(romClass);
-			if (J9_ARE_ALL_BITS_SET(implicitCreationFlags, J9AccImplicitCreateNonAtomic)) {
-				classFlags |= J9ClassAllowsNonAtomicCreation;


Remove the definition of J9ClassAllowsNonAtomicCreation and comment referencing it in on line 3363.

~~Please remove references to J9ClassIsPrimitiveValueType as well while you are here, this is very out of date.~~ Chatting with Aditi and this is staying in for now due to references in the jit.

theresa-m · 2025-11-21T19:53:49Z

runtime/vm/createramclass.cpp

 				}
 			}
-#if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES)
-			if (J9_ARE_ALL_BITS_SET(classFlags, J9ClassAllowsInitialDefaultValue)) {


This logic is still needed to measure classes for flattening. Instead of checking for J9ClassAllowsInitialDefaultValue this should be done for all value classes.

theresa-m · 2025-11-21T19:54:30Z

runtime/vm/createramclass.cpp

-				classFlags |= J9ClassAllowsNonAtomicCreation;
-			}
-			if (J9_ARE_ALL_BITS_SET(implicitCreationFlags, J9AccImplicitCreateHasDefaultValue)) {
-				classFlags |= J9ClassAllowsInitialDefaultValue;


Please remove other references to J9ClassAllowsInitialDefaultValue in the code base.

theresa-m · 2025-11-21T20:13:05Z

runtime/vm/classsupport.c

 	 * the regular arrayClass will always be created first.
 	 */
-	if (J9_IS_J9CLASS_ALLOW_DEFAULT_VALUE(elementClass)
+	if (J9ROMCLASS_IS_VALUE(elementClass->romClass)


Actually sorry I don't think we need this section at all. Its no longer the case that a value type array will be initialized with default values.

theresa-m · 2025-11-25T18:20:29Z

runtime/bcutil/attrlookup.h

 __attribute__ ((__gnu_inline__))
 #endif
 #endif
+


This line does not need to be added.

theresa-m · 2025-11-25T18:33:27Z

runtime/vm/createramclass.cpp

 			 *
 			 *              + J9ClassEnsureHashed (inherited)
 			 *             + J9ClassHasOffloadAllowSubtasksNatives
-			 *            + J9ClassIsPrimitiveValueType | J9ClassAllowsInitialDefaultValue


Please leave the J9ClassIsPrimitiveValueType since it hasn't been fully removed from the code base.

theresa-m · 2025-11-25T18:51:32Z

runtime/vm/ClassInitialization.cpp


 						if (isStatic) {
+							U_32 fieldModifiers = entry->field->modifiers;
+							if (J9_ARE_ALL_BITS_SET(fieldModifiers, J9FieldFlagIsNullRestricted)) {


While you're here can you add back the test testStaticNullRestrictedFieldMustBeInValueClass that was removed in #22922 ? I think I got rid of it too hastily.

theresa-m · 2025-11-25T19:08:34Z

runtime/vm/createramclass.cpp

-#endif /* defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) */
-					) {
+					if (J9_ARE_ALL_BITS_SET(options, J9_FINDCLASS_FLAG_CLASS_OPTION_NULL_RESTRICTED_ARRAY)) {
 						ramArrayClass->classFlags |= J9ClassContainsUnflattenedFlattenables;


The J9ClassContainsUnflattenedFlattenables flag will automatically set default values for fields. This flag and the code that uses it will need to be removed as well. I expect this will be another large chunk of work so I suggest doing this in another pull request.

theresa-m · 2025-11-27T21:01:20Z

runtime/vm/createramclass.cpp

 			 *              + J9ClassEnsureHashed (inherited)
 			 *             + J9ClassHasOffloadAllowSubtasksNatives
 			 *            + J9ClassIsPrimitiveValueType | J9ClassAllowsInitialDefaultValue
-			 *           + J9ClassAllowsNonAtomicCreation


Replace this with Unused instead of removing the line like is done in line 3387.

theresa-m · 2025-11-27T21:04:25Z

runtime/vm/createramclass.cpp

-				classFlags |= J9ClassAllowsInitialDefaultValue;
-			}
-		}
+			UDATA instanceSize = state->ramClass->totalInstanceSize;


I would suggest getting rid of this variable and directly adding state->ramClass->totalInstanceSize in the if statement.

Related: eclipse-openj9#22642 Remove the implicit creation work. Update the tests to follow the new spec.

theresa-m self-requested a review November 19, 2025 15:01

theresa-m added comp:vm project:valhalla Used to track Project Valhalla related work labels Nov 19, 2025

theresa-m suggested changes Nov 20, 2025

View reviewed changes

AditiS11 force-pushed the implicit_1 branch 2 times, most recently from d1a364b to 96a5772 Compare November 21, 2025 15:04

theresa-m suggested changes Nov 21, 2025

View reviewed changes

AditiS11 force-pushed the implicit_1 branch from 96a5772 to c0b46ef Compare November 24, 2025 15:50

theresa-m suggested changes Nov 25, 2025

View reviewed changes

theresa-m reviewed Nov 25, 2025

View reviewed changes

theresa-m mentioned this pull request Nov 25, 2025

Valhalla implementation required for latest OpenJDK/Valhalla updates #22642

Open

AditiS11 force-pushed the implicit_1 branch from c0b46ef to 3740d84 Compare November 26, 2025 18:13

theresa-m suggested changes Nov 27, 2025

View reviewed changes

Remove Implicit creation work

9c17f24

Related: eclipse-openj9#22642 Remove the implicit creation work. Update the tests to follow the new spec.

AditiS11 force-pushed the implicit_1 branch from 3740d84 to 9c17f24 Compare November 28, 2025 12:53

Remove Implicit creation work #22975

Are you sure you want to change the base?

Remove Implicit creation work #22975

Uh oh!

Conversation

AditiS11 commented Nov 19, 2025

Uh oh!

theresa-m left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theresa-m left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theresa-m Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

theresa-m Nov 21, 2025 •

edited

Loading