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 method
`jdk/internal/value/ValueClass.newArrayInstance`

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

(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

Related: eclipse-openj9#19914, eclipse-openj9#19913

Signed-off-by: Annabelle Huo <[email protected]>
  • Loading branch information
@a7ehuo
a7ehuo committed Sep 4, 2024
1 parent 1deceda commit 516a676
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 45 deletions.
1 change: 1 addition & 0 deletions runtime/compiler/codegen/J9RecognizedMethodsEnum.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,7 @@
LastVectorMethod = LastVectorIntrinsicMethod,

java_lang_reflect_Array_getLength,
jdk_internal_value_ValueClass_newArrayInstance,
java_lang_reflect_Method_invoke,
java_util_Arrays_fill,
java_util_Arrays_equals,
Expand Down
18 changes: 18 additions & 0 deletions runtime/compiler/env/j9method.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3853,6 +3853,12 @@ 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;")},
{ TR::unknownMethod}
};

static X SpreadHandleMethods[] =
{
{x(TR::java_lang_invoke_SpreadHandle_numArgsToPassThrough, "numArgsToPassThrough", "()I")},
Expand Down Expand Up @@ -4189,6 +4195,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 @@ -4508,6 +4515,11 @@ void TR_ResolvedJ9Method::construct()
if (!strncmp(name, "invokeExact_thunkArchetype_", 27))
setRecognizedMethodInfo(TR::java_lang_invoke_DirectHandle_invokeExact);
}
else if ((classNameLen == 29) && !strncmp(className,"jdk/internal/value/ValueClass", 29))
{
if (!strncmp(name, "newArrayInstance_", 17))
setRecognizedMethodInfo(TR::jdk_internal_value_ValueClass_newArrayInstance);
}
else if ((classNameLen == 30) && !strncmp(className, "java/lang/invoke/VirtualHandle", 30))
{
if (!strncmp(name, "virtualCall_", 12))
Expand Down Expand Up @@ -7736,6 +7748,12 @@ 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();

// We can't trust the array class returned by signature if flattenable array is enabled.
// Both regular nullable array and null restricted array have the same signature.
if (*_sig == '[' && TR::Compiler->om.areFlattenableValueTypesEnabled())
return NULL;

return _resolvedMethod == NULL ? NULL :
fej9->getClassFromSignature(_sig, _nextIncrBy, _resolvedMethod);
}
Expand Down
115 changes: 78 additions & 37 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 isCompTypePrimVT = isArrayNullRestricted(arrayConstraint);

TR::Node *storeValueNode = NULL;
TR::VPConstraint *storeValueConstraint = NULL;
Expand Down Expand Up @@ -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 or not
// 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 @@ -2948,7 +2950,7 @@ J9::ValuePropagation::isArrayElementFlattened(TR::VPConstraint *arrayConstraint)
return TR_no;
}

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

if (isCompTypePrimVT == TR_yes)
{
Expand All @@ -2968,7 +2970,7 @@ J9::ValuePropagation::isArrayElementFlattened(TR::VPConstraint *arrayConstraint)
}

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
Expand All @@ -2984,48 +2986,46 @@ J9::ValuePropagation::isArrayCompTypePrimitiveValueType(TR::VPConstraint *arrayC
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());

// 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;
}
TR_OpaqueClassBlock *arrayClass = arrayConstraint->getClass();

if (TR::Compiler->cls.isPrimitiveValueTypeClass(arrayComponentClass))
if (TR::Compiler->cls.isArrayNullRestricted(comp(), arrayClass))
{
if (trace())
traceMsg(comp(), "%s: return TR_yes. arrayClass isArrayNullRestricted\n", __FUNCTION__);
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 subclassed by both
// value class and identity class.
// If abstract class has identity flag set, it cannot be subclassed 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;
Expand All @@ -3040,14 +3040,18 @@ J9::ValuePropagation::isArrayCompTypePrimitiveValueType(TR::VPConstraint *arrayC
if (sig && sig[0] == '[' && len == 19
&& !strncmp(sig, "[Ljava/lang/Object;", 19))
{
if (trace())
traceMsg(comp(), "%s: return %s. java.lang.Object\n", __FUNCTION__, (arrayConstraint->isFixedClass()) ? "TR_no" : "TR_maybe");
return (arrayConstraint->isFixedClass()) ? TR_no : TR_maybe;
}

// 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;
if (trace())
traceMsg(comp(), "%s: return %s. Concrete classHasIdentity %d\n", __FUNCTION__, (TR::Compiler->cls.classHasIdentity(arrayComponentClass)) ? "TR_no" : "TR_maybe", TR::Compiler->cls.classHasIdentity(arrayComponentClass));
return TR::Compiler->cls.classHasIdentity(arrayComponentClass) ? TR_no : TR_maybe;
}

void
Expand Down Expand Up @@ -4120,8 +4124,45 @@ J9::ValuePropagation::innerConstrainAcall(TR::Node *node)
addGlobalConstraint(node, TR::VPNonNullObject::create(this));
}
}
else if (method->getRecognizedMethod() == TR::jdk_internal_value_ValueClass_newArrayInstance)
{
/*
* 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;
constraint = getConstraint(node->getFirstChild(), isGlobal);
TR_ResolvedMethod *owningMethod = symRef->getOwningMethod(comp());
TR_OpaqueClassBlock *nullRestrictedCheckedTypeClass = fe()->getClassFromSignature("jdk/internal/value/NullRestrictedCheckedType", 44, owningMethod);

if (constraint &&
constraint->isFixedClass() &&
(comp()->fej9()->isInstanceOf(constraint->getClass(), nullRestrictedCheckedTypeClass, true, true) == TR_yes))
{
if (trace())
traceMsg(comp(), "%s: node n%dn fixed class %p is instance of nullRestrictedCheckedTypeClass\n", __FUNCTION__, node->getGlobalIndex(), constraint->getClass());

constraint = getConstraint(node->getFirstChild()->getFirstChild(), isGlobal);
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
10 changes: 5 additions & 5 deletions runtime/compiler/optimizer/J9ValuePropagation.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,14 @@ class ValuePropagation : public OMR::ValuePropagation
virtual TR_YesNoMaybe isValue(TR::VPConstraint *constraint, TR_OpaqueClassBlock *& clazz);

/**
* Determine whether the component type of an array is, or might be, a primitive value
* type.
* Determine whether the array is, or might be, null restricted
*
* \param arrayConstraint The \ref TR::VPConstraint type constraint for the array reference
* \returns \c TR_yes if the array's component type is definitely a primitive value type;\n
* \c TR_no if it is definitely not a primitive value type; or\n
* \returns \c TR_yes if the array is definitely a null restricted array;\n
* \c TR_no if it is definitely not a null restricted array; or\n
* \c TR_maybe otherwise.
*/
virtual TR_YesNoMaybe isArrayCompTypePrimitiveValueType(TR::VPConstraint *arrayConstraint);
virtual TR_YesNoMaybe isArrayNullRestricted(TR::VPConstraint *arrayConstraint);

/**
* \brief
Expand Down
13 changes: 10 additions & 3 deletions runtime/compiler/runtime/SymbolValidationManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1254,9 +1254,16 @@ TR::SymbolValidationManager::validateArrayClassFromComponentClassRecord(uint16_t
TR_OpaqueClassBlock *componentClass = getClassFromID(componentClassID);
if (validateSymbol(arrayClassID, _fej9->getArrayClassFromComponentClass(componentClass)))
return true;

TR_OpaqueClassBlock *nullRestrictedArray = _fej9->getNullRestrictedArrayClassFromComponentClass(componentClass);
return nullRestrictedArray ? validateSymbol(arrayClassID, nullRestrictedArray) : false;

if (TR::Compiler->om.areFlattenableValueTypesEnabled())
{
TR_OpaqueClassBlock *nullRestrictedArray = _fej9->getNullRestrictedArrayClassFromComponentClass(componentClass);
return nullRestrictedArray ? validateSymbol(arrayClassID, nullRestrictedArray) : false;
}
else
{
return false;
}
}
else
{
Expand Down

0 comments on commit 516a676

Please sign in to comment.