Skip to content

Commit

Permalink
Fix null restricted array related issues
Browse files Browse the repository at this point in the history 
(1) Add recognized methods from jdk/internal/value/ValueClass
and jdk/internal/value/NullRestrictedCheckedType

(2)
Add VP fixed class constraint if ValueClass.newArrayInstance
creates null restricted array using NullRestrictedCheckedType

(3)
Rename isArrayCompTypePrimitiveValueType
to isArrayNullRestricted to reflect the updated logic

Update isArrayNullRestricted on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

(6)
Enable previous disabled JIT Valhalla functional tests

Fixes: #19913, #19914, #19708

Signed-off-by: Annabelle Huo <[email protected]>
  • Loading branch information
@a7ehuo
a7ehuo committed Oct 11, 2024
1 parent b442fee commit 135b856
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 84 deletions.
3 changes: 3 additions & 0 deletions runtime/compiler/codegen/J9RecognizedMethodsEnum.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,9 @@
LastVectorMethod = LastVectorIntrinsicMethod,

java_lang_reflect_Array_getLength,
jdk_internal_value_ValueClass_newArrayInstance,
jdk_internal_value_ValueClass_newNullRestrictedArray,
jdk_internal_value_NullRestrictedCheckedType_of,
java_lang_reflect_Method_invoke,
java_util_Arrays_fill,
java_util_Arrays_equals,
Expand Down
16 changes: 16 additions & 0 deletions runtime/compiler/env/j9method.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3847,6 +3847,19 @@ void TR_ResolvedJ9Method::construct()
{ TR::unknownMethod}
};

static X ValueClassMethods[] =
{
{x(TR::jdk_internal_value_ValueClass_newArrayInstance, "newArrayInstance", "(Ljdk/internal/value/CheckedType;I)[Ljava/lang/Object;")},
{x(TR::jdk_internal_value_ValueClass_newNullRestrictedArray, "newNullRestrictedArray", "(Ljava/lang/Class;I)[Ljava/lang/Object;")},
{ TR::unknownMethod}
};

static X NullRestrictedCheckedTypeMethods[] =
{
{x(TR::jdk_internal_value_NullRestrictedCheckedType_of, "of", "(Ljava/lang/Class;)Ljdk/internal/value/NullRestrictedCheckedType;")},
{ TR::unknownMethod}
};

static X SpreadHandleMethods[] =
{
{x(TR::java_lang_invoke_SpreadHandle_numArgsToPassThrough, "numArgsToPassThrough", "()I")},
Expand Down Expand Up @@ -4183,6 +4196,7 @@ void TR_ResolvedJ9Method::construct()
{ "sun/nio/cs/ISO_8859_1$Decoder", EncodeMethods },
{ "java/io/ByteArrayOutputStream", ByteArrayOutputStreamMethods },
{ "java/lang/ScopedValue$Carrier", ScopedValueMethods },
{ "jdk/internal/value/ValueClass", ValueClassMethods },
{ 0 }
};

Expand Down Expand Up @@ -4311,6 +4325,7 @@ void TR_ResolvedJ9Method::construct()
{
{ "java/lang/invoke/ConvertHandle$FilterHelpers", ConvertHandleFilterHelpersMethods },
{ "java/lang/invoke/DirectMethodHandle$Accessor", DirectMethodHandleAccessorMethods },
{ "jdk/internal/value/NullRestrictedCheckedType", NullRestrictedCheckedTypeMethods },
{ 0 }
};

Expand Down Expand Up @@ -7765,6 +7780,7 @@ TR_OpaqueClassBlock * TR_J9MethodParameterIterator::getOpaqueClass()
TR_J9VMBase *fej9 = (TR_J9VMBase *)(_comp.fe());
TR_ASSERT(*_sig == '[' || *_sig == 'L', "Asked for class of incorrect Java parameter.");
if (_nextIncrBy == 0) getDataType();

return _resolvedMethod == NULL ? NULL :
fej9->getClassFromSignature(_sig, _nextIncrBy, _resolvedMethod);
}
Expand Down
188 changes: 126 additions & 62 deletions runtime/compiler/optimizer/J9ValuePropagation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -973,7 +973,7 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node)
TR::Node *indexNode = node->getChild(elementIndexOpIndex);
TR::Node *arrayRefNode = node->getChild(arrayRefOpIndex);
TR::VPConstraint *arrayConstraint = getConstraint(arrayRefNode, arrayRefGlobal);
TR_YesNoMaybe isCompTypePrimVT = isArrayCompTypePrimitiveValueType(arrayConstraint);
TR_YesNoMaybe isNullRestrictedArray = isArrayNullRestricted(arrayConstraint);

TR::Node *storeValueNode = NULL;
TR::VPConstraint *storeValueConstraint = NULL;
Expand Down Expand Up @@ -1053,21 +1053,21 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node)
}

// Transform the helper call to regular aaload and aastore if array flattening is not enabled,
// or the array is known to be of a primitive value type that is not flattened.
// or the array is known to be of a null-restricted array that is not flattened.
bool canTransformUnflattenedArrayElementLoadStore = TR::Compiler->om.isValueTypeArrayFlatteningEnabled() ? false : true;
if (!canTransformUnflattenedArrayElementLoadStore &&
arrayConstraint &&
(isCompTypePrimVT == TR_yes) &&
(isNullRestrictedArray == TR_yes) &&
!TR::Compiler->cls.isValueTypeClassFlattened(arrayConstraint->getClass()))
{
canTransformUnflattenedArrayElementLoadStore = true;
}

// If the array is known to have a component type that is not a primitive value type or
// If the array is not a null-restricted array or
// the value being stored is known not to be a value type, transform the helper
// call to a regular aaload or aastore
bool canTransformIdentityArrayElementLoadStore = false;
if ((arrayConstraint != NULL && isCompTypePrimVT == TR_no)
if ((arrayConstraint != NULL && isNullRestrictedArray == TR_no)
|| (isStoreFlattenableArrayElement && isStoreValueVT == TR_no))
{
canTransformIdentityArrayElementLoadStore = true;
Expand All @@ -1076,7 +1076,9 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node)
bool canTransformFlattenedArrayElementLoadStoreUseTypeHint = false;
bool canTransformUnflattenedArrayElementLoadStoreUseTypeHint = false;
bool canTransformIdentityArrayElementLoadStoreUseTypeHint = false;
static const char *disableFlattenedArrayElementTypeHintXForm = feGetEnv("TR_DisableFlattenedArrayElementTypeHintXForm");
// Disable transformation based on type hint which is no longer sufficient enough
// to decide whether the array is null-restricted or not
static const char *enableFlattenedArrayElementTypeHintXForm = feGetEnv("TR_EnableFlattenedArrayElementTypeHintXForm");
static const char *enableUnflattenedArrayElementTypeHintXForm = feGetEnv("TR_EnableUnflattenedArrayElementTypeHintXForm");
TR_OpaqueClassBlock *typeHintClass = arrayConstraint ? arrayConstraint->getTypeHintClass() : NULL;

Expand All @@ -1098,7 +1100,7 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node)
{
if (TR::Compiler->cls.isValueTypeClassFlattened(hintComponentClass))
{
if (!disableFlattenedArrayElementTypeHintXForm)
if (enableFlattenedArrayElementTypeHintXForm)
{
if (isLoadFlattenableArrayElement)
{
Expand Down Expand Up @@ -1260,8 +1262,8 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node)
//
if (storeValueBaseNode == NULL || getValueNumber(storeValueBaseNode) != getValueNumber(arrayRefNode))
{
// If storing to an array whose component type is or might be a primitive value
// type and the value that's being assigned is or might be null, both a run-time
// If storing to an array that is or might be null-restricted
// and the value that's being assigned is or might be null, both a run-time
// NULLCHK of the value is required (guarded by a check of whether the
// component type is a value type) and an ArrayStoreCHK are required;
// otherwise, only the ArrayStoreCHK is required.
Expand All @@ -1273,10 +1275,11 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node)
flagsForTransform.set(ValueTypesHelperCallTransform::RequiresStoreCheck);
}

// If the value being stored is NULL and the destination array component is null-restricted at runtime,
// a NPE is expected to throw. Therefore, when the array component type is not known to be identity type
// in compilation time, a NULLCHK on store value is required
if ((isCompTypePrimVT != TR_no) &&
// If the value being stored is NULL and the destination array is null-restricted at runtime,
// an ArrayStoreException is expected to throw. Therefore, when the array component type is not
// known to be identity type in compilation time, a call to <nonNullableArrayNullStoreCheck> is
// required to check whether a null reference is being stored to a null-restricted array
if ((isNullRestrictedArray != TR_no) &&
(storeValueConstraint == NULL || !storeValueConstraint->isNonNullObject()) &&
!owningMethodDoesNotContainNonNullableArrayNullStoreCheck(this, node))
{
Expand Down Expand Up @@ -1317,12 +1320,9 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node)
//
if (storeValueBaseNode == NULL || getValueNumber(storeValueBaseNode) != getValueNumber(arrayRefNode))
{
// If storing to an array whose component type is or might be a primitive value
// type and the value that's being assigned is or might be null, both a run-time
// NULLCHK of the value is required (guarded by a check of whether the
// component type is a value type) and an ArrayStoreCHK are required;
// otherwise, only the ArrayStoreCHK is required.
//
// If storing to an array that is or might be null restricted and the value that's being assigned
// is or might be null, a call to <nonNullableArrayNullStoreCheck> is required to check whether
// a null reference is being stored to a null-restricted array
bool mustFail = false;
if (!owningMethodDoesNotContainStoreChecks(this, node) &&
isArrayStoreCheckNeeded(arrayRefNode, storeValueNode, mustFail, storeClassForCheck, componentClassForCheck))
Expand Down Expand Up @@ -1385,11 +1385,11 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node)
{
reason = "no-array-constraint";
}
else if (isCompTypePrimVT == TR_yes)
else if (isNullRestrictedArray == TR_yes)
{
reason = "comp-type-is-vt";
}
else if (isCompTypePrimVT == TR_maybe)
else if (isNullRestrictedArray == TR_maybe)
{
reason = "comp-type-may-be-vt";
}
Expand Down Expand Up @@ -2949,9 +2949,9 @@ J9::ValuePropagation::isArrayElementFlattened(TR::VPConstraint *arrayConstraint)
return TR_no;
}

TR_YesNoMaybe isCompTypePrimVT = isArrayCompTypePrimitiveValueType(arrayConstraint);
TR_YesNoMaybe isNullRestrictedArray = isArrayNullRestricted(arrayConstraint);

if (isCompTypePrimVT == TR_yes)
if (isNullRestrictedArray == TR_yes)
{
TR_OpaqueClassBlock *arrayClass = arrayConstraint->getClass();
if (TR::Compiler->cls.isValueTypeClassFlattened(arrayClass))
Expand All @@ -2965,73 +2965,71 @@ J9::ValuePropagation::isArrayElementFlattened(TR::VPConstraint *arrayConstraint)
}

// Return TR_maybe or TR_no
return isCompTypePrimVT;
return isNullRestrictedArray;
}

TR_YesNoMaybe
J9::ValuePropagation::isArrayCompTypePrimitiveValueType(TR::VPConstraint *arrayConstraint)
J9::ValuePropagation::isArrayNullRestricted(TR::VPConstraint *arrayConstraint)
{
if (!TR::Compiler->om.areValueTypesEnabled() ||
!TR::Compiler->om.areFlattenableValueTypesEnabled()) // Only null-restricted or primitive value type are flattenable
!TR::Compiler->om.areFlattenableValueTypesEnabled()) // Only null-restricted arrays are flattenable
{
return TR_no;
}

// If there's no constraint for the array operand, or no information
// is available about the class of the array, or the operand is not
// even definitely known to be an array, VP has to assume that it might
// have a component type that is a primitive value type
// even definitely known to be an array, VP has to assume that the array
// might be null-restricted
//
if (!(arrayConstraint && arrayConstraint->getClass()
&& arrayConstraint->getClassType()->isArray() == TR_yes))
{
if (trace())
traceMsg(comp(), "%s: return TR_maybe. arrayConstraint %p\n", __FUNCTION__, arrayConstraint);
return TR_maybe;
}

TR_OpaqueClassBlock *arrayComponentClass = fe()->getComponentClassFromArrayClass(arrayConstraint->getClass());
TR_OpaqueClassBlock *arrayClass = arrayConstraint->getClass();

// Cases to consider:
//
// - Is no information available about the component type of the array?
// If not, assume it might be a primitive value type.
// - Is the component type definitely a identity type?
// - Is the component type definitely a primitive value type?
// - Is the component type definitely a value type, but not primitive?
// - Is the component type either an abstract class or an interface
// (i.e., not a concrete class)? If so, it might be a value type.
// - Is the array an array of java/lang/Object? See below.
// - Otherwise, it must be a concrete class known not to be a value
// type
//
if (!arrayComponentClass)
{
return TR_maybe;
}

// No need to check array class type because array classes should be marked as having identity.
if (TR::Compiler->cls.classHasIdentity(arrayComponentClass))
{
return TR_no;
}

if (TR::Compiler->cls.isPrimitiveValueTypeClass(arrayComponentClass))
if (TR::Compiler->cls.isArrayNullRestricted(comp(), arrayClass))
{
if (trace())
traceMsg(comp(), "%s: return TR_yes. arrayClass %p\n", __FUNCTION__, arrayClass);
return TR_yes;
}

if (TR::Compiler->cls.isValueTypeClass(arrayComponentClass))
TR_OpaqueClassBlock *arrayComponentClass = fe()->getComponentClassFromArrayClass(arrayConstraint->getClass());

if (!arrayComponentClass)
{
return TR_no;
if (trace())
traceMsg(comp(), "%s: return TR_maybe. arrayComponentClass NULL\n", __FUNCTION__);
return TR_maybe;
}

if (!TR::Compiler->cls.isConcreteClass(comp(), arrayComponentClass))
{
return TR_maybe;
// Interface shouldn't have identity flag set and it can be implemented by both
// value class and identity class.
// If abstract class has identity flag set, it cannot be extended by value class.
if (TR::Compiler->cls.classHasIdentity(arrayComponentClass))
{
if (trace())
traceMsg(comp(), "%s: return TR_no. abstract classHasIdentity\n", __FUNCTION__);
return TR_no;
}
else
{
if (trace())
traceMsg(comp(), "%s: return TR_maybe. Not concrete class\n", __FUNCTION__);
return TR_maybe;
}
}

int32_t len;
const char *sig = arrayConstraint->getClassSignature(len);

TR_YesNoMaybe ret;
// If the array is an array of java/lang/Object, and it is fixed to
// that type, the component type is not a value type (though it
// can still hold references to instances of value types). If it is
Expand All @@ -3041,14 +3039,20 @@ J9::ValuePropagation::isArrayCompTypePrimitiveValueType(TR::VPConstraint *arrayC
if (sig && sig[0] == '[' && len == 19
&& !strncmp(sig, "[Ljava/lang/Object;", 19))
{
return (arrayConstraint->isFixedClass()) ? TR_no : TR_maybe;
ret = (arrayConstraint->isFixedClass()) ? TR_no : TR_maybe;
if (trace())
traceMsg(comp(), "%s: return %s. java.lang.Object\n", __FUNCTION__, (ret == TR_no) ? "TR_no" : "TR_maybe");
return ret;
}

// If we get to this point, we know this is not an array of
// java/lang/Object, and we know the component must be a concrete
// class that is not a value type.
// class.
//
return TR_no;
ret = TR::Compiler->cls.classHasIdentity(arrayComponentClass) ? TR_no : TR_maybe;
if (trace())
traceMsg(comp(), "%s: return %s. Concrete class\n", __FUNCTION__, (ret == TR_no) ? "TR_no" : "TR_maybe");
return ret;
}

void
Expand Down Expand Up @@ -3604,6 +3608,25 @@ J9::ValuePropagation::getParmValues()
bool isClassErased = false;

TR_OpaqueClassBlock *opaqueClass = parmIterator->getOpaqueClass();

if (opaqueClass &&
parmIterator->isArray() &&
TR::Compiler->om.areFlattenableValueTypesEnabled())
{
TR_OpaqueClassBlock *arrayComponentClass = comp()->fej9()->getComponentClassFromArrayClass(opaqueClass);
uint32_t len = 0;

// Both regular nullable array and null-restricted array have the same signature.
// If the array component class is not an identity type, the array might either be
// a null-restricted array or a nullable array. Therefore, we can't trust the array
// class returned by the signature.
if (!TR::Compiler->cls.classHasIdentity(arrayComponentClass) ||
!strncmp(parmIterator->getUnresolvedJavaClassSignature(len), "[Ljava/lang/Object;", 19))
{
opaqueClass = NULL;
}
}

if (opaqueClass)
{
TR_OpaqueClassBlock *prexClass = NULL;
Expand Down Expand Up @@ -4121,8 +4144,49 @@ J9::ValuePropagation::innerConstrainAcall(TR::Node *node)
addGlobalConstraint(node, TR::VPNonNullObject::create(this));
}
}
else if ((method->getRecognizedMethod() == TR::jdk_internal_value_ValueClass_newArrayInstance) &&
(node->getFirstChild()->getOpCodeValue() == TR::acall))
{
/*
* n12n acall jdk/internal/value/ValueClass.newArrayInstance(Ljdk/internal/value/CheckedType;I)[Ljava/lang/Object;
* n9n acall jdk/internal/value/NullRestrictedCheckedType.of(Ljava/lang/Class;)Ljdk/internal/value/NullRestrictedCheckedType;
* n8n aloadi <javaLangClassFromClass>
* n7n loadaddr SomeValueClass
* n11n iload Test.ARRAY_SIZE
*/
bool isGlobal;
TR::Node *firstChildAcallNode = node->getFirstChild();
constraint = getConstraint(firstChildAcallNode, isGlobal);
TR_ResolvedMethod *owningMethod = symRef->getOwningMethod(comp());
TR_OpaqueClassBlock *nullRestrictedCheckedTypeClass = fe()->getClassFromSignature("jdk/internal/value/NullRestrictedCheckedType", 44, owningMethod);

if (constraint &&
constraint->isFixedClass() &&
nullRestrictedCheckedTypeClass &&
(comp()->fej9()->isInstanceOf(constraint->getClass(), nullRestrictedCheckedTypeClass, true, true) == TR_yes) &&
(firstChildAcallNode->getSymbol()->getResolvedMethodSymbol()->getRecognizedMethod() == TR::jdk_internal_value_NullRestrictedCheckedType_of))
{
if (trace())
traceMsg(comp(), "%s: node n%dn its first child fixed class %p is an instance of NullRestrictedCheckedType\n", __FUNCTION__, node->getGlobalIndex(), constraint->getClass());

constraint = firstChildAcallNode->getFirstChild() ? getConstraint(firstChildAcallNode->getFirstChild(), isGlobal) : NULL;
TR_OpaqueClassBlock *arrayComponentClass = (constraint && constraint->isFixedClass()) ? constraint->getClass() : NULL;
TR_OpaqueClassBlock *nullRestrictedArrayClass = arrayComponentClass ? fe()->getNullRestrictedArrayClassFromComponentClass(arrayComponentClass) : NULL;

if (trace())
traceMsg(comp(), "%s: node n%dn arrayComponentClass %p nullRestrictedArrayClass %p\n", __FUNCTION__, node->getGlobalIndex(), arrayComponentClass, nullRestrictedArrayClass);

if (nullRestrictedArrayClass)
{
TR::VPConstraint *newConstraint = TR::VPFixedClass::create(this, nullRestrictedArrayClass);
addBlockOrGlobalConstraint(node, newConstraint, isGlobal);
addGlobalConstraint(node, TR::VPNonNullObject::create(this));
return node;
}
}
}
}
else
else // if (!node->getOpCode().isIndirect())
{
if ((method->getRecognizedMethod() == TR::java_math_BigDecimal_add) ||
(method->getRecognizedMethod() == TR::java_math_BigDecimal_subtract) ||
Expand Down
Loading

0 comments on commit 135b856

Please sign in to comment.