Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Null-restricted array flattening and test fix up #19995

Merged
merged 1 commit into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions debugtools/DDR_VM/src/com/ibm/j9ddr/AuxFieldInfo29.dat
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ J9Class.instanceLeafDescription = required
J9Class.lockOffset = required
J9Class.module = required
J9Class.nextClassInSegment = required
J9Class.nullRestrictedArrayClass = J9Class*
J9Class.ramConstantPool = required
J9Class.ramMethods = required
J9Class.ramStatics = required
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,11 @@ public J9ClassPointer[] findNestedClassHierarchy(J9ClassPointer containerClazz,
int index = 0;

if (Pattern.matches("\\[\\d+\\]", nestingHierarchy[0])) {
resultClasses[0] = containerClazz.arrayClass();
try {
resultClasses[0] = containerClazz.nullRestrictedArrayClass();
} catch (NoSuchFieldException e) {
throw new CorruptDataException("J9Class.nullRestrictedArrayClass field does not exist", e);
}
index = 1;
}
resultClasses[index] = containerClazz;
Expand Down
6 changes: 3 additions & 3 deletions runtime/gc_base/ObjectAccessBarrier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1141,7 +1141,7 @@ MM_ObjectAccessBarrier::copyObjectFieldsToFlattenedArrayElement(J9VMThread *vmTh
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_PRIMITIVE_VALUETYPE(elementClazz));
Assert_MM_true(J9_IS_J9CLASS_ALLOW_DEFAULT_VALUE(elementClazz));
Assert_MM_true(elementClazz == arrayClazz->leafComponentType);

elementStartOffset += J9CLASS_PREPADDING_SIZE(elementClazz);
Expand All @@ -1165,7 +1165,7 @@ MM_ObjectAccessBarrier::copyObjectFieldsFromFlattenedArrayElement(J9VMThread *vm
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(destObject, vmThread);
Assert_MM_true(J9_IS_J9CLASS_PRIMITIVE_VALUETYPE(elementClazz));
Assert_MM_true(J9_IS_J9CLASS_ALLOW_DEFAULT_VALUE(elementClazz));
Assert_MM_true(elementClazz == arrayClazz->leafComponentType);

elementStartOffset += J9CLASS_PREPADDING_SIZE(elementClazz);
Expand Down Expand Up @@ -1416,7 +1416,7 @@ MM_ObjectAccessBarrier::structuralCompareFlattenedObjects(J9VMThread *vmThread,
UDATA limit = J9CLASS_UNPADDED_INSTANCE_SIZE(valueClass);
UDATA offset = 0;

Assert_MM_true(J9_IS_J9CLASS_PRIMITIVE_VALUETYPE(valueClass));
Assert_MM_true(J9_IS_J9CLASS_ALLOW_DEFAULT_VALUE(valueClass));

if (hasReferences) {
UDATA descriptionIndex = J9_OBJECT_DESCRIPTION_SIZE - 1;
Expand Down
5 changes: 0 additions & 5 deletions runtime/oti/j9.h
Original file line number Diff line number Diff line change
Expand Up @@ -340,10 +340,6 @@ static const struct { \
* Disable flattening of volatile field that is > 8 bytes for now, as the current implementation of copyObjectFields() will tear this field.
*/
#define J9_IS_FIELD_FLATTENED(fieldClazz, romFieldShape) \
(J9_IS_J9CLASS_FLATTENED(fieldClazz) && \
(J9_ARE_NO_BITS_SET((romFieldShape)->modifiers, J9AccVolatile) || (J9CLASS_UNPADDED_INSTANCE_SIZE(fieldClazz) <= sizeof(U_64))))
/* This will replace J9_IS_FIELD_FLATTENED when QTypes are removed. J9_IS_J9CLASS_FLATTENED will return false since the current check requires a primitive value type. */
#define J9_IS_NULL_RESTRICTED_FIELD_FLATTENED(fieldClazz, romFieldShape) \
(J9ROMFIELD_IS_NULL_RESTRICTED(romFieldShape) && \
J9_IS_J9CLASS_FLATTENED(fieldClazz) && \
(J9_ARE_NO_BITS_SET((romFieldShape)->modifiers, J9AccVolatile) || (J9CLASS_UNPADDED_INSTANCE_SIZE(fieldClazz) <= sizeof(U_64))))
Expand All @@ -357,7 +353,6 @@ static const struct { \
#define J9_IS_J9CLASS_FLATTENED(clazz) FALSE
#define J9ROMFIELD_IS_NULL_RESTRICTED(romField) FALSE
#define J9_IS_FIELD_FLATTENED(fieldClazz, romFieldShape) FALSE
#define J9_IS_NULL_RESTRICTED_FIELD_FLATTENED(fieldClazz, romFieldShape) FALSE
#define J9_VALUETYPE_FLATTENED_SIZE(clazz)((UDATA) 0) /* It is not possible for this macro to be used since we always check J9_IS_J9CLASS_FLATTENED before ever using it. */
#define J9_IS_J9ARRAYCLASS_NULL_RESTRICTED(clazz) FALSE
#endif /* defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) */
Expand Down
4 changes: 2 additions & 2 deletions runtime/oti/vm_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -2819,7 +2819,7 @@ UDATA
getFlattenableFieldOffset(J9Class *fieldOwner, J9ROMFieldShape *field);

/**
* Returns if a field is flattened. `J9_IS_J9CLASS_FLATTENED` will be deprecated.
* Returns if a field is flattened.
* This helper assumes field is null-restricted.
*
* @param[in] fieldOwner the J9class that defines the field
Expand All @@ -2831,7 +2831,7 @@ BOOLEAN
isFlattenableFieldFlattened(J9Class *fieldOwner, J9ROMFieldShape *field);

/**
* Returns the type of an instance field. `J9_IS_J9CLASS_FLATTENED` will be deprecated.
* Returns the type of an instance field.
* This helper assumes field is null-restricted.
*
* @param[in] fieldOwner the J9class that defines the field
Expand Down
5 changes: 4 additions & 1 deletion runtime/vm/BytecodeInterpreter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1232,7 +1232,10 @@ class INTERPRETER_CLASS
} else {
rc = THROW_ARRAY_STORE;
}
} else if (J9_IS_J9CLASS_FLATTENED(srcClazz) || J9_IS_J9CLASS_FLATTENED(destClazz) || J9_IS_J9CLASS_PRIMITIVE_VALUETYPE(destComponentClass)) {
} else if (J9_IS_J9CLASS_FLATTENED(srcClazz)
|| J9_IS_J9CLASS_FLATTENED(destClazz)
|| J9_IS_J9CLASS_ALLOW_DEFAULT_VALUE(destComponentClass)
) {
/* VM_ArrayCopyHelpers::referenceArrayCopy cannot handle flattened arrays or null elements being copied into arrays of primitive value types, so for those cases use copyFlattenableArray instead */
updateVMStruct(REGISTER_ARGS);
I_32 value = VM_ValueTypeHelpers::copyFlattenableArray(_currentThread, _objectAccessBarrier, _objectAllocate, srcObject, destObject, srcStart, destStart, elementCount);
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/createramclass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1990,7 +1990,7 @@ loadFlattenableFieldValueClasses(J9VMThread *currentThread, J9ClassLoader *class
J9NLS_VM_NULLRESTRICTED_MUST_BE_IN_DEFAULT_IMPLICITCREATION_VALUE_CLASS);
}

if (!J9_IS_NULL_RESTRICTED_FIELD_FLATTENED(valueClass, field)) {
if (!J9_IS_FIELD_FLATTENED(valueClass, field)) {
*valueTypeFlags |= (J9ClassContainsUnflattenedFlattenables | J9ClassHasReferences);
eligibleForFastSubstitutability = false;
} else if (J9_ARE_NO_BITS_SET(valueClass->classFlags, J9ClassCanSupportFastSubstitutability)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
*/
package org.openj9.test.lworld;

import java.lang.reflect.Array;
import jdk.internal.value.ValueClass;
import static org.openj9.test.lworld.ValueTypeTestClasses.*;

public class DDRBackfillLayoutTest {
Expand Down Expand Up @@ -60,10 +60,10 @@ private static void createAndCheckValueType() throws Throwable {
ValueTypeQuadLong quadLongInstance = new ValueTypeQuadLong(doubleLongInstance, new ValueTypeLong(ValueTypeTests.defaultLongNew2), ValueTypeTests.defaultLongNew3);
ValueTypeDoubleQuadLong doubleQuadLongInstance = new ValueTypeDoubleQuadLong(quadLongInstance, doubleLongInstance, new ValueTypeLong(ValueTypeTests.defaultLongNew4), ValueTypeTests.defaultLongNew5);

Object flatUnAlignedSingleBackfill2Array = Array.newInstance(ValueTypeTests.flatUnAlignedSingleBackfillClass2, 3);
Array.set(flatUnAlignedSingleBackfill2Array, 1, flatUnAlignedSingleBackfill2Instance);
Object[] flatUnAlignedSingleBackfill2Array = ValueClass.newNullRestrictedArray(ValueTypeTests.flatUnAlignedSingleBackfillClass2, 3);
flatUnAlignedSingleBackfill2Array[1] = flatUnAlignedSingleBackfill2Instance;

ValueTypeQuadLong[] quadLongArray = new ValueTypeQuadLong[3];
Object[] quadLongArray = ValueClass.newNullRestrictedArray(ValueTypeQuadLong.class, 3);
quadLongArray[1] = quadLongInstance;

ValueTypeTests.checkObject(flatSingleBackfillInstance,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodType;
import java.lang.reflect.Array;
import jdk.internal.value.ValueClass;

public class DDRValueTypeTest {
public static void main(String[] args) {
Expand Down Expand Up @@ -62,9 +62,9 @@ private static void createAndCheckValueType() throws Throwable {
Object assortedValueWithSingleAlignmentAlt = ValueTypeTests.createAssorted(makeAssortedValueWithSingleAlignment, ValueTypeTests.typeWithSingleAlignmentFields, altFields);
Object valueTypeWithVolatileFields = ValueTypeTests.createValueTypeWithVolatileFields();

Object valArray = Array.newInstance(assortedValueWithSingleAlignmentClass, 2);
Array.set(valArray, 0, assortedValueWithSingleAlignment);
Array.set(valArray, 1, assortedValueWithSingleAlignmentAlt);
Object[] valArray = ValueClass.newNullRestrictedArray(assortedValueWithSingleAlignmentClass, 2);
valArray[0] = assortedValueWithSingleAlignment;
valArray[1] = assortedValueWithSingleAlignmentAlt;

ValueTypeTests.checkObject(assortedValueWithSingleAlignment,
assortedValueWithSingleAlignmentAlt,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
import jdk.internal.value.ValueClass;
import org.testng.Assert;
import static org.testng.Assert.*;
import org.testng.annotations.Test;
Expand Down Expand Up @@ -535,12 +536,11 @@ static public void testCreateArrayFlattenedLine2D() throws Throwable {
Object en2 = makePoint2D.invoke(x4, y4);
Object line2D_2 = makeFlattenedLine2D.invoke(st2, en2);

Object arrayObject = Array.newInstance(flattenedLine2DClass, 3);
Array.set(arrayObject, 1, line2D_1);
Array.set(arrayObject, 2, line2D_2);

Object line2D_1_check = Array.get(arrayObject, 1);
Object line2D_2_check = Array.get(arrayObject, 2);
Object[] arrayObject = ValueClass.newNullRestrictedArray(flattenedLine2DClass, 2);
arrayObject[0] = line2D_1;
arrayObject[1] = line2D_2;
Object line2D_1_check = arrayObject[0];
Object line2D_2_check = arrayObject[1];

assertEquals(getX.invoke(getFlatSt.invoke(line2D_1_check)), getX.invoke(getFlatSt.invoke(line2D_1)));
assertEquals(getX.invoke(getFlatSt.invoke(line2D_2_check)), getX.invoke(getFlatSt.invoke(line2D_2)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0-only WITH Classpath-ex
<!DOCTYPE suite SYSTEM "cmdlinetester.dtd">

<suite id="J9 Value Type ddr test with flattening enabled" timeout="600">
<variable name="ARGS" value="--enable-preview -XX:ValueTypeFlatteningThreshold=999999 -XX:+EnableArrayFlattening -Xverify:none --add-opens java.base/jdk.internal.misc=ALL-UNNAMED" />
<variable name="ARGS" value="--enable-preview -XX:ValueTypeFlatteningThreshold=999999 -XX:+EnableArrayFlattening -Xverify:none --add-opens java.base/jdk.internal.misc=ALL-UNNAMED --add-exports java.base/jdk.internal.value=ALL-UNNAMED" />
<variable name="JARS" value="-cp $ASMJAR$$CPDL$$JCOMMANDERJAR$$CPDL$$TESTNGJAR$$CPDL$$VALUETYPEJAR$" />
<variable name="PROGRAM" value="org.openj9.test.lworld.DDRBackfillLayoutTest" />
<variable name="DUMPFILE" value="j9core.dmp" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
<!DOCTYPE suite SYSTEM "cmdlinetester.dtd">

<suite id="J9 Value Type ddr test with flattening enabled" timeout="600">
<variable name="ARGS" value="-Xint --enable-preview -XX:ValueTypeFlatteningThreshold=999999 -XX:+EnableArrayFlattening -Xverify:none --add-opens java.base/jdk.internal.misc=ALL-UNNAMED" />
<variable name="ARGS_NOCR" value="-Xint --enable-preview -Xnocompressedrefs -XX:ValueTypeFlatteningThreshold=999999 -XX:+EnableArrayFlattening -Xverify:none --add-opens java.base/jdk.internal.misc=ALL-UNNAMED" />
<variable name="ARGS" value="-Xint --enable-preview -XX:ValueTypeFlatteningThreshold=999999 -XX:+EnableArrayFlattening -Xverify:none --add-opens java.base/jdk.internal.misc=ALL-UNNAMED --add-exports java.base/jdk.internal.value=ALL-UNNAMED" />
<variable name="ARGS_NOCR" value="-Xint --enable-preview -Xnocompressedrefs -XX:ValueTypeFlatteningThreshold=999999 -XX:+EnableArrayFlattening -Xverify:none --add-opens java.base/jdk.internal.misc=ALL-UNNAMED --add-exports java.base/jdk.internal.value=ALL-UNNAMED" />
<variable name="JARS" value="-cp $ASMJAR$$CPDL$$JCOMMANDERJAR$$CPDL$$TESTNGJAR$$CPDL$$VALUETYPEJAR$" />
<variable name="PROGRAM" value="org.openj9.test.lworld.DDRValueTypeTest" />
<variable name="DUMPFILE" value="j9core.dmp" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<!DOCTYPE suite SYSTEM "cmdlinetester.dtd">

<suite id="J9 Value Type ddr test with flattening disabled" timeout="600">
<variable name="ARGS" value="--enable-preview -Xint -Xverify:none --add-opens java.base/jdk.internal.misc=ALL-UNNAMED" />
<variable name="ARGS" value="--enable-preview -Xint -Xverify:none --add-opens java.base/jdk.internal.misc=ALL-UNNAMED --add-exports java.base/jdk.internal.value=ALL-UNNAMED" />
<variable name="JARS" value="-cp $ASMJAR$$CPDL$$JCOMMANDERJAR$$CPDL$$TESTNGJAR$$CPDL$$VALUETYPEJAR$" />
<variable name="PROGRAM" value="org.openj9.test.lworld.DDRValueTypeTest" />
<variable name="DUMPFILE" value="j9core.dmp" />
Expand Down