From 178ee93ffc399a69371beddf5159f95fabdec856 Mon Sep 17 00:00:00 2001 From: Annabelle Huo Date: Mon, 15 May 2023 16:18:49 -0400 Subject: [PATCH 1/2] Add two APIs to J9Object - `areFlattenableValueTypesEnabled`: Whether or not flattenable value object (aka null restricted) type is enabled - `isQDescriptorForValueTypesSupported`: Whether or not `Q` signature is supported - Update `isValueTypeArrayFlatteningEnabled` to check `areFlattenableValueTypesEnabled` first Signed-off-by: Annabelle Huo --- runtime/compiler/env/J9ObjectModel.cpp | 51 +++++++++++++++++++++++++- runtime/compiler/env/J9ObjectModel.hpp | 12 ++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/runtime/compiler/env/J9ObjectModel.cpp b/runtime/compiler/env/J9ObjectModel.cpp index 5638c714873..2ec2e388689 100644 --- a/runtime/compiler/env/J9ObjectModel.cpp +++ b/runtime/compiler/env/J9ObjectModel.cpp @@ -122,6 +122,45 @@ J9::ObjectModel::areValueTypesEnabled() return javaVM->internalVMFunctions->areValueTypesEnabled(javaVM); } +bool +J9::ObjectModel::areFlattenableValueTypesEnabled() + { +#if defined(J9VM_OPT_JITSERVER) + if (auto stream = TR::CompilationInfo::getStream()) + { +#if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) + return true; +#else /* defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) */ + return false; +#endif /* defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) */ + } +#endif /* defined(J9VM_OPT_JITSERVER) */ + + J9JavaVM * javaVM = TR::Compiler->javaVM; + return javaVM->internalVMFunctions->areFlattenableValueTypesEnabled(javaVM); + } + +bool +J9::ObjectModel::isQDescriptorForValueTypesSupported() + { + // TODO: Implementation is required to determine under which case 'Q' descriptor is removed +#if defined(J9VM_OPT_JITSERVER) + if (auto stream = TR::CompilationInfo::getStream()) + { +#if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) + return true; +#else /* defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) */ + return false; +#endif /* defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) */ + } +#endif /* defined(J9VM_OPT_JITSERVER) */ + + J9JavaVM * javaVM = TR::Compiler->javaVM; + if (javaVM->internalVMFunctions->areFlattenableValueTypesEnabled(javaVM)) + return true; + + return false; + } bool J9::ObjectModel::areValueBasedMonitorChecksEnabled() @@ -144,13 +183,21 @@ J9::ObjectModel::isValueTypeArrayFlatteningEnabled() #if defined(J9VM_OPT_JITSERVER) if (auto stream = TR::CompilationInfo::getStream()) { +#if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) auto *vmInfo = TR::compInfoPT->getClientData()->getOrCacheVMInfo(stream); - return J9_ARE_ANY_BITS_SET(vmInfo->_extendedRuntimeFlags2, J9_EXTENDED_RUNTIME2_ENABLE_VT_ARRAY_FLATTENING); + return J9_ARE_ANY_BITS_SET(vmInfo->_extendedRuntimeFlags2, J9_EXTENDED_RUNTIME2_ENABLE_VT_ARRAY_FLATTENING); +#else /* defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) */ + return false; +#endif /* defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) */ } #endif /* defined(J9VM_OPT_JITSERVER) */ J9JavaVM *javaVM = TR::Compiler->javaVM; - return J9_ARE_ALL_BITS_SET(javaVM->extendedRuntimeFlags2, J9_EXTENDED_RUNTIME2_ENABLE_VT_ARRAY_FLATTENING); + + if (javaVM->internalVMFunctions->areFlattenableValueTypesEnabled(javaVM)) + return J9_ARE_ALL_BITS_SET(javaVM->extendedRuntimeFlags2, J9_EXTENDED_RUNTIME2_ENABLE_VT_ARRAY_FLATTENING); + else + return false; } int32_t diff --git a/runtime/compiler/env/J9ObjectModel.hpp b/runtime/compiler/env/J9ObjectModel.hpp index fb1cad4c19d..3d226362e85 100644 --- a/runtime/compiler/env/J9ObjectModel.hpp +++ b/runtime/compiler/env/J9ObjectModel.hpp @@ -62,7 +62,19 @@ class ObjectModel : public OMR::ObjectModelConnector bool mayRequireSpineChecks(); + /** + * @brief Whether or not value object is enabled + */ bool areValueTypesEnabled(); + /** + * @brief Whether or not flattenable value object (aka null restricted) type is enabled + */ + bool areFlattenableValueTypesEnabled(); + /** + * @brief Whether or not `Q` signature is supported + */ + bool isQDescriptorForValueTypesSupported(); + /** * @brief Whether the check is enabled on monitor object being value based class type */ From 156b344b12c82d5211f96f8c177e522e4ae2e5ab Mon Sep 17 00:00:00 2001 From: Annabelle Huo Date: Mon, 15 May 2023 16:23:16 -0400 Subject: [PATCH 2/2] Check flattenable value types and if Q signature is supported - Check `areFlattenableValueTypesEnabled` before checking if element is flattened - Check if Q signature is supported or not before using Q signature to determine if the element is a value type. - Remove special handling on checking primitive value type in `genCheckcast` and disable `testCheckCastValueTypeOnNull` since the latest spec doesn't require special handling on null restricted value types. Signed-off-by: Annabelle Huo --- runtime/compiler/env/J9ClassEnv.cpp | 23 ++- runtime/compiler/env/VMJ9.cpp | 8 +- runtime/compiler/env/j9method.cpp | 6 +- runtime/compiler/env/j9methodServer.cpp | 4 +- runtime/compiler/ilgen/IlGenerator.cpp | 6 +- runtime/compiler/ilgen/Walker.cpp | 134 ++++++++++-------- .../compiler/optimizer/J9ValuePropagation.cpp | 33 +++-- runtime/compiler/runtime/JITClientSession.cpp | 4 +- .../openj9/test/lworld/ValueTypeTests.java | 5 +- 9 files changed, 135 insertions(+), 88 deletions(-) diff --git a/runtime/compiler/env/J9ClassEnv.cpp b/runtime/compiler/env/J9ClassEnv.cpp index 8b15db404c4..fb22def29d3 100644 --- a/runtime/compiler/env/J9ClassEnv.cpp +++ b/runtime/compiler/env/J9ClassEnv.cpp @@ -520,8 +520,21 @@ static void addEntryForFieldImpl(TR_VMField *field, TR::TypeLayoutBuilder &tlb, uint32_t mergedLength = 0; J9UTF8 *signature = J9ROMFIELDSHAPE_SIGNATURE(field->shape); - if (TR::Compiler->om.areValueTypesEnabled() && - vm->internalVMFunctions->isNameOrSignatureQtype(signature) && + bool isFieldPrimitiveValueType = false; + + if (TR::Compiler->om.areFlattenableValueTypesEnabled()) + { + if (TR::Compiler->om.isQDescriptorForValueTypesSupported()) + { + isFieldPrimitiveValueType = vm->internalVMFunctions->isNameOrSignatureQtype(signature); + } + else + { + TR_ASSERT_FATAL(false, "Support for null-restricted types without Q descriptor is to be implemented!!!"); + } + } + + if (isFieldPrimitiveValueType && vm->internalVMFunctions->isFlattenableFieldFlattened(definingClass, field->shape)) { char *prefixForChild = buildTransitiveFieldNames(prefix, prefixLength, field->shape, comp->trMemory()->currentStackRegion(), mergedLength); @@ -1043,7 +1056,11 @@ J9::ClassEnv::classNameToSignature(const char *name, int32_t &len, TR::Compilati { len += 2; sig = (char *)comp->trMemory()->allocateMemory(len+1, allocKind); - if (clazz && TR::Compiler->om.areValueTypesEnabled() && self()->isPrimitiveValueTypeClass(clazz)) + if (clazz && + TR::Compiler->om.areFlattenableValueTypesEnabled() && + TR::Compiler->om.isQDescriptorForValueTypesSupported() && + self()->isPrimitiveValueTypeClass(clazz) + ) sig[0] = 'Q'; else sig[0] = 'L'; diff --git a/runtime/compiler/env/VMJ9.cpp b/runtime/compiler/env/VMJ9.cpp index 7ab1f23e37e..cbb1fcd116b 100644 --- a/runtime/compiler/env/VMJ9.cpp +++ b/runtime/compiler/env/VMJ9.cpp @@ -2532,7 +2532,9 @@ TR_J9VMBase::getClassSignature_DEPRECATED(TR_OpaqueClassBlock * clazz, int32_t & sig[i] = '['; if (* name != '[') { - if (TR::Compiler->om.areValueTypesEnabled() && TR::Compiler->cls.isPrimitiveValueTypeClass(myClass)) + if (TR::Compiler->om.areFlattenableValueTypesEnabled() && + TR::Compiler->om.isQDescriptorForValueTypesSupported() && + TR::Compiler->cls.isPrimitiveValueTypeClass(myClass)) sig[i++] = 'Q'; else sig[i++] = 'L'; @@ -2565,7 +2567,9 @@ TR_J9VMBase::getClassSignature(TR_OpaqueClassBlock * clazz, TR_Memory * trMemory sig[i] = '['; if (* name != '[') { - if (TR::Compiler->om.areValueTypesEnabled() && TR::Compiler->cls.isPrimitiveValueTypeClass(myClass)) + if (TR::Compiler->om.areFlattenableValueTypesEnabled() && + TR::Compiler->om.isQDescriptorForValueTypesSupported() && + TR::Compiler->cls.isPrimitiveValueTypeClass(myClass)) sig[i++] = 'Q'; else sig[i++] = 'L'; diff --git a/runtime/compiler/env/j9method.cpp b/runtime/compiler/env/j9method.cpp index 455c9dba71b..70396cd1ee0 100644 --- a/runtime/compiler/env/j9method.cpp +++ b/runtime/compiler/env/j9method.cpp @@ -9286,7 +9286,7 @@ TR_J9ByteCodeIlGenerator::packReferenceChainOffsets(TR::Node *node, std::vector< bool TR_ResolvedJ9Method::isFieldQType(int32_t cpIndex) { - if (!TR::Compiler->om.areValueTypesEnabled() || + if (!TR::Compiler->om.areFlattenableValueTypesEnabled() || (-1 == cpIndex)) return false; @@ -9301,7 +9301,7 @@ TR_ResolvedJ9Method::isFieldQType(int32_t cpIndex) bool TR_ResolvedJ9Method::isFieldFlattened(TR::Compilation *comp, int32_t cpIndex, bool isStatic) { - if (!TR::Compiler->om.areValueTypesEnabled() || + if (!TR::Compiler->om.areFlattenableValueTypesEnabled() || (-1 == cpIndex)) return false; @@ -9309,7 +9309,7 @@ TR_ResolvedJ9Method::isFieldFlattened(TR::Compilation *comp, int32_t cpIndex, bo J9ROMFieldShape *fieldShape = NULL; TR_OpaqueClassBlock *containingClass = definingClassAndFieldShapeFromCPFieldRef(comp, cp(), cpIndex, isStatic, &fieldShape); - // No lock is required here. Entires in J9Class::flattenedClassCache are only written during classload. + // No lock is required here. Entries in J9Class::flattenedClassCache are only written during classload. // They are effectively read only when being exposed to the JIT. return vmThread->javaVM->internalVMFunctions->isFlattenableFieldFlattened(reinterpret_cast(containingClass), fieldShape); } diff --git a/runtime/compiler/env/j9methodServer.cpp b/runtime/compiler/env/j9methodServer.cpp index a970f10c7f7..7199e7d86c8 100644 --- a/runtime/compiler/env/j9methodServer.cpp +++ b/runtime/compiler/env/j9methodServer.cpp @@ -1843,7 +1843,7 @@ TR_ResolvedJ9JITServerMethod::archetypeArgPlaceholderSlot() bool TR_ResolvedJ9JITServerMethod::isFieldQType(int32_t cpIndex) { - if (!TR::Compiler->om.areValueTypesEnabled() || + if (!TR::Compiler->om.areFlattenableValueTypesEnabled() || (-1 == cpIndex)) return false; @@ -1859,7 +1859,7 @@ TR_ResolvedJ9JITServerMethod::isFieldQType(int32_t cpIndex) bool TR_ResolvedJ9JITServerMethod::isFieldFlattened(TR::Compilation *comp, int32_t cpIndex, bool isStatic) { - if (!TR::Compiler->om.areValueTypesEnabled() || + if (!TR::Compiler->om.areFlattenableValueTypesEnabled() || (-1 == cpIndex)) return false; diff --git a/runtime/compiler/ilgen/IlGenerator.cpp b/runtime/compiler/ilgen/IlGenerator.cpp index dd753b40fc4..57593e7cafd 100644 --- a/runtime/compiler/ilgen/IlGenerator.cpp +++ b/runtime/compiler/ilgen/IlGenerator.cpp @@ -558,11 +558,7 @@ TR_J9ByteCodeIlGenerator::genILFromByteCodes() if (currNode->getOpCodeValue() == TR::checkcast && currNode->getSecondChild()->getOpCodeValue() == TR::loadaddr - && currNode->getSecondChild()->getSymbolReference()->isUnresolved() - && // check whether the checkcast class is primitive valuetype. Expansion is only needed for checkcast to reference type. - (!TR::Compiler->om.areValueTypesEnabled() - || !TR::Compiler->cls.isClassRefPrimitiveValueType(comp(), method()->classOfMethod(), - currNode->getSecondChild()->getSymbolReference()->getCPIndex()))) + && currNode->getSecondChild()->getSymbolReference()->isUnresolved()) { unresolvedCheckcastTopsNeedingNullGuard.add(currTree); } diff --git a/runtime/compiler/ilgen/Walker.cpp b/runtime/compiler/ilgen/Walker.cpp index aa394dc06af..a9c868a1c69 100644 --- a/runtime/compiler/ilgen/Walker.cpp +++ b/runtime/compiler/ilgen/Walker.cpp @@ -2225,8 +2225,6 @@ TR_J9ByteCodeIlGenerator::genCheckCast() * * If the class specified in the bytecode is unresolved, this leaves out the * ResolveCHK since it has to be conditional on a non-null object. - * If the class specified in the bytecode is a primitive value type, it has to - * be resolved unconditionally, regardless of whether the value is null. * * @param cpIndex The constant pool entry of the class given in the bytecode * @@ -2236,20 +2234,7 @@ TR_J9ByteCodeIlGenerator::genCheckCast() void TR_J9ByteCodeIlGenerator::genCheckCast(int32_t cpIndex) { - if (TR::Compiler->om.areValueTypesEnabled() - && TR::Compiler->cls.isClassRefPrimitiveValueType(comp(), method()->classOfMethod(), cpIndex)) - { - TR::Node * objNode = _stack->top(); - - loadClassObject(cpIndex); - - TR::Node *passThruNode = TR::Node::create(TR::PassThrough, 1, objNode); - genTreeTop(genNullCheck(passThruNode)); - } - else - { - loadClassObjectForTypeTest(cpIndex, TR_DisableAOTCheckCastInlining); - } + loadClassObjectForTypeTest(cpIndex, TR_DisableAOTCheckCastInlining); genCheckCast(); } @@ -4962,17 +4947,24 @@ TR_J9ByteCodeIlGenerator::loadInstance(int32_t cpIndex) comp()->failCompilation("NO support for AOT in field watch"); TR_ResolvedJ9Method * owningMethod = static_cast(_methodSymbol->getResolvedMethod()); - if (TR::Compiler->om.areValueTypesEnabled() && owningMethod->isFieldQType(cpIndex)) + if (TR::Compiler->om.areFlattenableValueTypesEnabled()) { - if (!isFieldResolved(comp(), owningMethod, cpIndex, false)) + if (!TR::Compiler->om.isQDescriptorForValueTypesSupported()) { - abortForUnresolvedValueTypeOp("getfield", "field"); + TR_ASSERT_FATAL(false, "Support for null-restricted types without Q descriptor is to be implemented!!!"); } - else if (owningMethod->isFieldFlattened(comp(), cpIndex, _methodSymbol->isStatic())) + else if (owningMethod->isFieldQType(cpIndex)) { - return comp()->getOption(TR_UseFlattenedFieldRuntimeHelpers) ? - loadFlattenableInstanceWithHelper(cpIndex) : - loadFlattenableInstance(cpIndex); + if (!isFieldResolved(comp(), owningMethod, cpIndex, false)) + { + abortForUnresolvedValueTypeOp("getfield", "field"); + } + else if (owningMethod->isFieldFlattened(comp(), cpIndex, _methodSymbol->isStatic())) + { + return comp()->getOption(TR_UseFlattenedFieldRuntimeHelpers) ? + loadFlattenableInstanceWithHelper(cpIndex) : + loadFlattenableInstance(cpIndex); + } } } @@ -5906,8 +5898,7 @@ TR_J9ByteCodeIlGenerator::loadArrayElement(TR::DataType dataType, TR::ILOpCodes // helper is needed. // if (mayBeValueType && - TR::Compiler->om.areValueTypesEnabled() && - TR::Compiler->om.isValueTypeArrayFlatteningEnabled() && + TR::Compiler->om.isValueTypeArrayFlatteningEnabled() && // isValueTypeArrayFlatteningEnabled() checks areFlattenableValueTypesEnabled() !TR::Compiler->om.canGenerateArraylets() && dataType == TR::Address) { @@ -6204,11 +6195,18 @@ TR_J9ByteCodeIlGenerator::genWithField(int32_t fieldCpIndex) } TR_ResolvedJ9Method * owningMethod = static_cast(_methodSymbol->getResolvedMethod()); - if (owningMethod->isFieldQType(fieldCpIndex) && owningMethod->isFieldFlattened(comp(), fieldCpIndex, _methodSymbol->isStatic())) + if (TR::Compiler->om.areFlattenableValueTypesEnabled()) { - return comp()->getOption(TR_UseFlattenedFieldRuntimeHelpers) ? - genFlattenableWithFieldWithHelper(fieldCpIndex) : - genFlattenableWithField(fieldCpIndex, valueClass); + if (!TR::Compiler->om.isQDescriptorForValueTypesSupported()) + { + TR_ASSERT_FATAL(false, "Support for null-restricted types without Q descriptor is to be implemented!!!"); + } + else if (owningMethod->isFieldQType(fieldCpIndex) && owningMethod->isFieldFlattened(comp(), fieldCpIndex, _methodSymbol->isStatic())) + { + return comp()->getOption(TR_UseFlattenedFieldRuntimeHelpers) ? + genFlattenableWithFieldWithHelper(fieldCpIndex) : + genFlattenableWithField(fieldCpIndex, valueClass); + } } bool isStore = false; @@ -6567,32 +6565,46 @@ TR_J9ByteCodeIlGenerator::genAconst_init(TR_OpaqueClassBlock *valueTypeClass, in // for that value type. That's handled with a recursive call to genAconst_init. // If the signature does not begin with a Q, the field is an identity type whose default value is a Java null /// reference. - if (fieldSignature[0] == 'Q') + bool isNullRestricted = false; + if (TR::Compiler->om.areFlattenableValueTypesEnabled()) { - // In non-SVM AOT compilation, cpIndex is required for AOT relocation. - // In this case, cpindex is unknown for the field. - if (comp()->compileRelocatableCode() && !comp()->getOption(TR_UseSymbolValidationManager)) + if (!TR::Compiler->om.isQDescriptorForValueTypesSupported()) { - abortForUnresolvedValueTypeOp("aconst_init", "field"); + TR_ASSERT_FATAL(false, "Support for null-restricted types without Q descriptor is to be implemented!!!"); } - - TR_OpaqueClassBlock *fieldClass = fej9()->getClassFromSignature(fieldSignature, (int32_t)strlen(fieldSignature), - comp()->getCurrentMethod()); - if (comp()->getOption(TR_TraceILGen)) + else if (fieldSignature[0] == 'Q') { - traceMsg(comp(), "fieldSignature %s fieldClass %p\n", fieldSignature, fieldClass); - } + isNullRestricted = true; - // Set cpIndex as -1 since it's unknown for the field class - genAconst_init(fieldClass, -1 /* cpIndex */); - } - else if (comp()->target().is64Bit()) - { - loadConstant(TR::aconst, (int64_t)0); + // In non-SVM AOT compilation, cpIndex is required for AOT relocation. + // In this case, cpindex is unknown for the field. + if (comp()->compileRelocatableCode() && !comp()->getOption(TR_UseSymbolValidationManager)) + { + abortForUnresolvedValueTypeOp("aconst_init", "field"); + } + + TR_OpaqueClassBlock *fieldClass = fej9()->getClassFromSignature(fieldSignature, (int32_t)strlen(fieldSignature), + comp()->getCurrentMethod()); + if (comp()->getOption(TR_TraceILGen)) + { + traceMsg(comp(), "fieldSignature %s fieldClass %p\n", fieldSignature, fieldClass); + } + + // Set cpIndex as -1 since it's unknown for the field class + genAconst_init(fieldClass, -1 /* cpIndex */); + } } - else + + if (!isNullRestricted) { - loadConstant(TR::aconst, (int32_t)0); + if (comp()->target().is64Bit()) + { + loadConstant(TR::aconst, (int64_t)0); + } + else + { + loadConstant(TR::aconst, (int32_t)0); + } } break; } @@ -6941,17 +6953,25 @@ TR_J9ByteCodeIlGenerator::storeInstance(int32_t cpIndex) comp()->failCompilation("NO support for AOT in field watch"); TR_ResolvedJ9Method * owningMethod = static_cast(_methodSymbol->getResolvedMethod()); - if (TR::Compiler->om.areValueTypesEnabled() && owningMethod->isFieldQType(cpIndex)) + + if (TR::Compiler->om.areFlattenableValueTypesEnabled()) { - if (!isFieldResolved(comp(), owningMethod, cpIndex, true)) + if (!TR::Compiler->om.isQDescriptorForValueTypesSupported()) { - abortForUnresolvedValueTypeOp("putfield", "field"); + TR_ASSERT_FATAL(false, "Support for null-restricted types without Q descriptor is to be implemented!!!"); } - else if (owningMethod->isFieldFlattened(comp(), cpIndex, _methodSymbol->isStatic())) + else if (owningMethod->isFieldQType(cpIndex)) { - return comp()->getOption(TR_UseFlattenedFieldRuntimeHelpers) ? - storeFlattenableInstanceWithHelper(cpIndex) : - storeFlattenableInstance(cpIndex); + if (!isFieldResolved(comp(), owningMethod, cpIndex, true)) + { + abortForUnresolvedValueTypeOp("putfield", "field"); + } + else if (owningMethod->isFieldFlattened(comp(), cpIndex, _methodSymbol->isStatic())) + { + return comp()->getOption(TR_UseFlattenedFieldRuntimeHelpers) ? + storeFlattenableInstanceWithHelper(cpIndex) : + storeFlattenableInstance(cpIndex); + } } } @@ -7417,7 +7437,9 @@ TR_J9ByteCodeIlGenerator::storeArrayElement(TR::DataType dataType, TR::ILOpCodes // we won't have flattening, so no call to flattenable array element access // helper is needed. // - if (TR::Compiler->om.areValueTypesEnabled() && !TR::Compiler->om.canGenerateArraylets() && dataType == TR::Address) + if (TR::Compiler->om.areFlattenableValueTypesEnabled() && + !TR::Compiler->om.canGenerateArraylets() && + dataType == TR::Address) { TR::Node* elementIndex = pop(); TR::Node* arrayBaseAddress = pop(); diff --git a/runtime/compiler/optimizer/J9ValuePropagation.cpp b/runtime/compiler/optimizer/J9ValuePropagation.cpp index 6fd14577e45..01d6218292c 100644 --- a/runtime/compiler/optimizer/J9ValuePropagation.cpp +++ b/runtime/compiler/optimizer/J9ValuePropagation.cpp @@ -1000,7 +1000,7 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node) } if (!canTransformIdentityArrayElementLoadStoreUseTypeHint && - TR::Compiler->cls.isValueTypeClass(hintComponentClass)) + TR::Compiler->cls.isPrimitiveValueTypeClass(hintComponentClass)) { if (TR::Compiler->cls.isValueTypeClassFlattened(hintComponentClass)) { @@ -1153,7 +1153,7 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node) flagsForTransform.set(ValueTypesHelperCallTransform::InsertDebugCounter); - if (isStoreFlattenableArrayElement && !owningMethodDoesNotContainStoreChecks(this, node)) + if (isStoreFlattenableArrayElement) { // Determine whether the value is being copied from the same array that is the target // of the array element store. If so, there's no need for an ArrayStoreCHK or a call @@ -1173,20 +1173,21 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node) // otherwise, only the ArrayStoreCHK is required. // bool mustFail = false; - if (isArrayStoreCheckNeeded(arrayRefNode, storeValueNode, mustFail, storeClassForCheck, componentClassForCheck)) + if (!owningMethodDoesNotContainStoreChecks(this, node) && + isArrayStoreCheckNeeded(arrayRefNode, storeValueNode, mustFail, storeClassForCheck, componentClassForCheck)) { flagsForTransform.set(ValueTypesHelperCallTransform::RequiresStoreCheck); } - if ((isCompTypePrimVT != TR_no) && (storeValueConstraint == NULL || !storeValueConstraint->isNonNullObject())) + //TODO: Require the non-helper if !canSkipNonNullableArrayNullValueChecks(...) + if (canTransformUnflattenedArrayElementLoadStore && + (isCompTypePrimVT != TR_no) && + (storeValueConstraint == NULL || !storeValueConstraint->isNonNullObject())) { flagsForTransform.set(ValueTypesHelperCallTransform::RequiresNullValueCheck); } } - } - if (isStoreFlattenableArrayElement) - { callToTransform = new (trStackMemory()) ArrayElementStoreHelperCallTransform(_curTree, node, flagsForTransform, arrayLength, NULL, storeClassForCheck, componentClassForCheck); @@ -1204,7 +1205,7 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node) TR_OpaqueClassBlock *storeClassForCheck = NULL; TR_OpaqueClassBlock *componentClassForCheck = NULL; - if (isStoreFlattenableArrayElement && !owningMethodDoesNotContainStoreChecks(this, node)) + if (isStoreFlattenableArrayElement) { TR::Node *storeValueBaseNode = getStoreValueBaseNode(storeValueNode, symRefTab); @@ -1220,20 +1221,20 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node) // otherwise, only the ArrayStoreCHK is required. // bool mustFail = false; - if (isArrayStoreCheckNeeded(arrayRefNode, storeValueNode, mustFail, storeClassForCheck, componentClassForCheck)) + if (!owningMethodDoesNotContainStoreChecks(this, node) && + isArrayStoreCheckNeeded(arrayRefNode, storeValueNode, mustFail, storeClassForCheck, componentClassForCheck)) { flagsForTransform.set(ValueTypesHelperCallTransform::RequiresStoreCheck); } - if (canTransformUnflattenedArrayElementLoadStoreUseTypeHint && (!storeValueConstraint || !storeValueConstraint->isNonNullObject())) + //TODO: Require the non-helper if !canSkipNonNullableArrayNullValueChecks(...) + if (canTransformUnflattenedArrayElementLoadStoreUseTypeHint && + (!storeValueConstraint || !storeValueConstraint->isNonNullObject())) { flagsForTransform.set(ValueTypesHelperCallTransform::RequiresNullValueCheck); } } - } - if (isStoreFlattenableArrayElement) - { callToTransform = new (trStackMemory()) ArrayElementStoreHelperCallTransform(_curTree, node, flagsForTransform, arrayLength, typeHintClass, storeClassForCheck, componentClassForCheck); } @@ -2621,7 +2622,8 @@ J9::ValuePropagation::transformUnflattenedArrayElementLoadStoreUseTypeHint(TR_Op TR_YesNoMaybe J9::ValuePropagation::isArrayElementFlattened(TR::VPConstraint *arrayConstraint) { - if (!TR::Compiler->om.areValueTypesEnabled()) + if (!TR::Compiler->om.areValueTypesEnabled() || + !TR::Compiler->om.isValueTypeArrayFlatteningEnabled()) // isValueTypeArrayFlatteningEnabled() checks areFlattenableValueTypesEnabled() { return TR_no; } @@ -2648,7 +2650,8 @@ J9::ValuePropagation::isArrayElementFlattened(TR::VPConstraint *arrayConstraint) TR_YesNoMaybe J9::ValuePropagation::isArrayCompTypePrimitiveValueType(TR::VPConstraint *arrayConstraint) { - if (!TR::Compiler->om.areValueTypesEnabled()) + if (!TR::Compiler->om.areValueTypesEnabled() || + !TR::Compiler->om.areFlattenableValueTypesEnabled()) // Only null restricted or primitive value type are flattenable { return TR_no; } diff --git a/runtime/compiler/runtime/JITClientSession.cpp b/runtime/compiler/runtime/JITClientSession.cpp index 99c3a7a3bcc..4fcbe72f956 100644 --- a/runtime/compiler/runtime/JITClientSession.cpp +++ b/runtime/compiler/runtime/JITClientSession.cpp @@ -201,7 +201,9 @@ ClientSessionData::processUnloadedClasses(const std::vectorom.areValueTypesEnabled() && TR::Compiler->cls.isPrimitiveValueTypeClass(clazz)) + if (TR::Compiler->om.areFlattenableValueTypesEnabled() && + TR::Compiler->om.isQDescriptorForValueTypesSupported() && + TR::Compiler->cls.isPrimitiveValueTypeClass(clazz)) sigStr[0] = 'Q'; else sigStr[0] = 'L'; diff --git a/test/functional/Valhalla/src/org/openj9/test/lworld/ValueTypeTests.java b/test/functional/Valhalla/src/org/openj9/test/lworld/ValueTypeTests.java index e8f2b7279ea..feaf05a192c 100644 --- a/test/functional/Valhalla/src/org/openj9/test/lworld/ValueTypeTests.java +++ b/test/functional/Valhalla/src/org/openj9/test/lworld/ValueTypeTests.java @@ -2506,8 +2506,11 @@ static public void testCheckCastValueTypeOnInvalidQtype() throws Throwable { /* * Ensure that casting null to a value type class will throw a null pointer exception + * This test is disabled since the latest spec from + * https://cr.openjdk.org/~dlsmith/jep401/jep401-20230519/specs/types-cleanup-jvms.html + * no longer requires null check on the objectref for null restricted value type class */ - @Test(priority=1, expectedExceptions=NullPointerException.class) + @Test(enabled=false, priority=1, expectedExceptions=NullPointerException.class) static public void testCheckCastValueTypeOnNull() throws Throwable { String fields[] = {"longField:J"}; Class valueClass = ValueTypeGenerator.generateValueClass("TestCheckCastValueTypeOnNull", fields);