From 9353a543d2a5f6cf133874071c884209ffc396a1 Mon Sep 17 00:00:00 2001 From: Annabelle Huo Date: Sun, 1 Sep 2024 13:42:13 -0400 Subject: [PATCH] Fix null restricted array related issues (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: #19914, #19913 Signed-off-by: Annabelle Huo --- .../codegen/J9RecognizedMethodsEnum.hpp | 2 + runtime/compiler/env/j9method.cpp | 32 ++++ .../compiler/optimizer/J9ValuePropagation.cpp | 142 ++++++++++++------ .../compiler/optimizer/J9ValuePropagation.hpp | 10 +- 4 files changed, 135 insertions(+), 51 deletions(-) diff --git a/runtime/compiler/codegen/J9RecognizedMethodsEnum.hpp b/runtime/compiler/codegen/J9RecognizedMethodsEnum.hpp index e80575d85ef..1b2eb8d28b8 100644 --- a/runtime/compiler/codegen/J9RecognizedMethodsEnum.hpp +++ b/runtime/compiler/codegen/J9RecognizedMethodsEnum.hpp @@ -484,6 +484,8 @@ LastVectorMethod = LastVectorIntrinsicMethod, java_lang_reflect_Array_getLength, + jdk_internal_value_ValueClass_newArrayInstance, + jdk_internal_value_ValueClass_newNullRestrictedArray, java_lang_reflect_Method_invoke, java_util_Arrays_fill, java_util_Arrays_equals, diff --git a/runtime/compiler/env/j9method.cpp b/runtime/compiler/env/j9method.cpp index d5b28f8f174..37a6e990661 100644 --- a/runtime/compiler/env/j9method.cpp +++ b/runtime/compiler/env/j9method.cpp @@ -3850,6 +3850,13 @@ 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, "newArrayInstance", "(Ljava/lang/Class;I)[Ljava/lang/Object;")}, + { TR::unknownMethod} + }; + static X SpreadHandleMethods[] = { {x(TR::java_lang_invoke_SpreadHandle_numArgsToPassThrough, "numArgsToPassThrough", "()I")}, @@ -4186,6 +4193,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 } }; @@ -4396,6 +4404,10 @@ void TR_ResolvedJ9Method::construct() class17 }; + TR::Compilation *comp = TR::comp(); + bool trace = comp ? comp->getOption(TR_TraceILGen) : false; + if (trace) traceMsg(comp, "%s: DEBUG isMethodInValidLibrary %d\n", __FUNCTION__, isMethodInValidLibrary()); + if (isMethodInValidLibrary()) { char *className = convertToMethod()->classNameChars(); @@ -4405,28 +4417,42 @@ void TR_ResolvedJ9Method::construct() char *sig = convertToMethod()->signatureChars(); int sigLen = convertToMethod()->signatureLength(); + if (trace) + { + traceMsg(comp, "%s: DEBUG className %s classNameLen %d name %s nameLen %d\n", __FUNCTION__, className, classNameLen, name, nameLen); + traceMsg(comp, "%s: DEBUG sig %s sigLen %d\n", __FUNCTION__, sig, sigLen); + traceMsg(comp, "%s: DEBUG minRecognizedClassLength %d maxRecognizedClassLength %d\n", __FUNCTION__, minRecognizedClassLength, maxRecognizedClassLength); + } + if (classNameLen >= minRecognizedClassLength && classNameLen <= maxRecognizedClassLength) { Y * cl = recognizedClasses[classNameLen - minRecognizedClassLength]; if (cl) for (; cl->_class; ++cl) + { if (!strncmp(cl->_class, className, classNameLen)) { + if (trace) traceMsg(comp, "%s: DEBUG cl->_class %s className %s classNameLen %d\n", __FUNCTION__, cl->_class, className, classNameLen); for (X * m = cl->_methods; m->_enum != TR::unknownMethod; ++m) { + if (trace) traceMsg(comp, "%s: DEBUG m->_nameLen %d m->_sigLen %d sigLen %d m->_name %s name %s nameLen %d\n", __FUNCTION__, + m->_nameLen, m->_sigLen, sigLen, m->_name, name, nameLen); if (m->_nameLen == nameLen && (m->_sigLen == sigLen || m->_sigLen == (int16_t)-1) && !strncmp(m->_name, name, nameLen) && (m->_sigLen == (int16_t)-1 || !strncmp(m->_sig, sig, sigLen))) { setRecognizedMethodInfo(m->_enum); + if (trace) traceMsg(comp, "%s: DEBUG setRecognizedMethodInfo\n", __FUNCTION__); break; } } } + } } if (TR::Method::getMandatoryRecognizedMethod() == TR::unknownMethod) { + if (trace) traceMsg(comp, "%s: DEBUG TR::Method::getMandatoryRecognizedMethod() == TR::unknownMethod\n", __FUNCTION__); // Cases where multiple method names all map to the same RecognizedMethod // if ((classNameLen == 13) && !strncmp(className, "java/util/Map", 13)) @@ -7749,6 +7775,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); } diff --git a/runtime/compiler/optimizer/J9ValuePropagation.cpp b/runtime/compiler/optimizer/J9ValuePropagation.cpp index 7e8e829a31a..b5772c90134 100644 --- a/runtime/compiler/optimizer/J9ValuePropagation.cpp +++ b/runtime/compiler/optimizer/J9ValuePropagation.cpp @@ -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; @@ -1057,7 +1057,7 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node) 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; @@ -1067,7 +1067,7 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node) // 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; @@ -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; @@ -1098,7 +1100,7 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node) { if (TR::Compiler->cls.isValueTypeClassFlattened(hintComponentClass)) { - if (!disableFlattenedArrayElementTypeHintXForm) + if (enableFlattenedArrayElementTypeHintXForm) { if (isLoadFlattenableArrayElement) { @@ -1276,7 +1278,7 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node) // 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 ((isNullRestrictedArray != TR_no) && (storeValueConstraint == NULL || !storeValueConstraint->isNonNullObject()) && !owningMethodDoesNotContainNonNullableArrayNullStoreCheck(this, node)) { @@ -1385,11 +1387,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"; } @@ -2949,9 +2951,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)) @@ -2965,11 +2967,11 @@ 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 @@ -2985,53 +2987,51 @@ 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 %p isArrayNullRestricted\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 @@ -3041,14 +3041,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 @@ -4121,8 +4127,52 @@ J9::ValuePropagation::innerConstrainAcall(TR::Node *node) addGlobalConstraint(node, TR::VPNonNullObject::create(this)); } } + else if (method->getRecognizedMethod() == TR::jdk_internal_value_ValueClass_newArrayInstance) + { + if (trace()) + traceMsg(comp(), "%s: node n%dn TR::jdk_internal_value_ValueClass_newArrayInstance\n", __FUNCTION__, node->getGlobalIndex()); + } + 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 + * 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() && + nullRestrictedCheckedTypeClass && + (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) || diff --git a/runtime/compiler/optimizer/J9ValuePropagation.hpp b/runtime/compiler/optimizer/J9ValuePropagation.hpp index 467375cae51..3f11d45871a 100644 --- a/runtime/compiler/optimizer/J9ValuePropagation.hpp +++ b/runtime/compiler/optimizer/J9ValuePropagation.hpp @@ -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