From 066df6b1a02fbb676543c4f065d945a7b16e1200 Mon Sep 17 00:00:00 2001 From: Annabelle Huo Date: Fri, 3 Nov 2023 13:38:46 -0400 Subject: [PATCH] Add and use a list of methods that can skip null value store check - Add a list recognized methods in j9method that can skip null check on the value stored to array elements. - ILGen: If array flattening is disabled and `skipNonNullableArrayNullStoreCheck` is true, avoid generating array element store non-helper calls. - VP: Avoid generating the `nonNullableArrayNullStoreCheck` if `skipNonNullableArrayNullStoreCheck` is true when transforming array element store non-helper calls. Signed-off-by: Annabelle Huo --- .../codegen/J9RecognizedMethodsEnum.hpp | 17 +++- runtime/compiler/env/j9method.cpp | 38 ++++++-- runtime/compiler/il/J9MethodSymbol.cpp | 90 +++++++++++++++++++ runtime/compiler/il/J9MethodSymbol.hpp | 1 + runtime/compiler/ilgen/Walker.cpp | 16 ++++ .../compiler/optimizer/J9ValuePropagation.cpp | 19 +++- 6 files changed, 172 insertions(+), 9 deletions(-) diff --git a/runtime/compiler/codegen/J9RecognizedMethodsEnum.hpp b/runtime/compiler/codegen/J9RecognizedMethodsEnum.hpp index 71c9afafdd8..67c6732e50c 100644 --- a/runtime/compiler/codegen/J9RecognizedMethodsEnum.hpp +++ b/runtime/compiler/codegen/J9RecognizedMethodsEnum.hpp @@ -68,6 +68,7 @@ java_lang_Class_isInstance, java_lang_Class_isInterface, java_lang_Class_cast, + java_lang_Class_all, java_lang_ClassLoader_callerClassLoader, java_lang_ClassLoader_getCallerClassLoader, java_lang_ClassLoader_getStackClassLoader, @@ -285,11 +286,23 @@ java_util_regex_Matcher_init, java_util_regex_Matcher_usePattern, - java_util_HashMap_all, + java_util_AbstractCollection_all, + java_util_ArrayDeque_all, java_util_ArrayList_all, - java_util_Hashtable_all, + java_util_ComparableTimSort_all, java_util_concurrent_ConcurrentHashMap_all, + java_util_HashMap_all, + java_util_Hashtable_all, + java_util_IdentityHashMap_all, + java_util_ImmutableCollections_all, + java_util_LinkedHashMap_all, + java_util_LinkedList_all, + java_util_Map_all, + java_util_regex_Pattern_all, + java_util_stream_Nodes_all, + java_util_TimSort_all, java_util_Vector_all, + java_util_WeakHashMap_all, java_util_Hashtable_get, java_util_Hashtable_put, diff --git a/runtime/compiler/env/j9method.cpp b/runtime/compiler/env/j9method.cpp index 2fbbd987b9c..36050d5491c 100644 --- a/runtime/compiler/env/j9method.cpp +++ b/runtime/compiler/env/j9method.cpp @@ -4376,6 +4376,10 @@ void TR_ResolvedJ9Method::construct() { // Cases where multiple method names all map to the same RecognizedMethod // + if ((classNameLen == 13) && !strncmp(className, "java/util/Map", 13)) + setRecognizedMethodInfo(TR::java_util_Map_all); + if ((classNameLen == 15) && !strncmp(className, "java/lang/Class", 15)) + setRecognizedMethodInfo(TR::java_lang_Class_all); if ((classNameLen == 17) && !strncmp(className, "java/util/TreeMap", 17)) setRecognizedMethodInfo(TR::java_util_TreeMap_all); else if ((classNameLen == 17) && !strncmp(className, "java/util/EnumMap", 17)) @@ -4391,14 +4395,32 @@ void TR_ResolvedJ9Method::construct() } else if ((classNameLen == 17) && !strncmp(className, "java/util/HashMap", 17)) setRecognizedMethodInfo(TR::java_util_HashMap_all); + else if ((classNameLen == 17) && !strncmp(className, "java/util/TimSort", 17)) + setRecognizedMethodInfo(TR::java_util_TimSort_all); else if ((classNameLen == 19) && !strncmp(className, "java/util/ArrayList", 19)) setRecognizedMethodInfo(TR::java_util_ArrayList_all); else if ((classNameLen == 19) && !strncmp(className, "java/util/Hashtable", 19)) setRecognizedMethodInfo(TR::java_util_Hashtable_all); + else if ((classNameLen == 20) && !strncmp(className, "java/util/LinkedList", 20)) + setRecognizedMethodInfo(TR::java_util_LinkedList_all); + else if ((classNameLen == 20) && !strncmp(className, "java/util/ArrayDeque", 20)) + setRecognizedMethodInfo(TR::java_util_ArrayDeque_all); + else if ((classNameLen == 21) && !strncmp(className, "java/util/WeakHashMap", 21)) + setRecognizedMethodInfo(TR::java_util_WeakHashMap_all); else if ((classNameLen == 38) && !strncmp(className, "java/util/concurrent/ConcurrentHashMap", 38)) setRecognizedMethodInfo(TR::java_util_concurrent_ConcurrentHashMap_all); else if ((classNameLen == 16) && !strncmp(className, "java/util/Vector", 16)) setRecognizedMethodInfo(TR::java_util_Vector_all); + else if ((classNameLen == 22) && !strncmp(className, "java/util/stream/Nodes", 22)) + setRecognizedMethodInfo(TR::java_util_stream_Nodes_all); + else if ((classNameLen == 23) && !strncmp(className, "java/util/LinkedHashMap", 23)) + setRecognizedMethodInfo(TR::java_util_LinkedHashMap_all); + else if ((classNameLen == 23) && !strncmp(className, "java/util/regex/Pattern", 23)) + setRecognizedMethodInfo(TR::java_util_regex_Pattern_all); + else if ((classNameLen == 25) && !strncmp(className, "java/util/IdentityHashMap", 25)) + setRecognizedMethodInfo(TR::java_util_IdentityHashMap_all); + else if ((classNameLen == 27) && !strncmp(className, "java/util/ComparableTimSort", 27)) + setRecognizedMethodInfo(TR::java_util_ComparableTimSort_all); else if ((classNameLen == 28) && !strncmp(className, "java/lang/invoke/ILGenMacros", 28)) { if (!strncmp(name, "invokeExact_", 12)) @@ -4408,6 +4430,8 @@ void TR_ResolvedJ9Method::construct() else if (!strncmp(name, "last_", 5)) setRecognizedMethodInfo(TR::java_lang_invoke_ILGenMacros_last); } + else if ((classNameLen == 28) && !strncmp(className, "java/util/AbstractCollection", 28)) + setRecognizedMethodInfo(TR::java_util_AbstractCollection_all); else if ((classNameLen == 29) && !strncmp(className, "java/lang/invoke/MethodHandle", 29)) { if (!strncmp(name, "asType", 6)) @@ -4434,11 +4458,6 @@ void TR_ResolvedJ9Method::construct() if (!strncmp(name, "invokeExact_thunkArchetype_", 27)) setRecognizedMethodInfo(TR::java_lang_invoke_DirectHandle_invokeExact); } - else if ((classNameLen == 32) && !strncmp(className, "java/lang/invoke/InterfaceHandle", 32)) - { - if (!strncmp(name, "invokeExact_thunkArchetype_", 27)) - setRecognizedMethodInfo(TR::java_lang_invoke_InterfaceHandle_invokeExact); - } else if ((classNameLen == 30) && !strncmp(className, "java/lang/invoke/VirtualHandle", 30)) { if (!strncmp(name, "virtualCall_", 12)) @@ -4455,6 +4474,15 @@ void TR_ResolvedJ9Method::construct() else if (!strncmp(name, "dispatchJ9Method_", 17)) setRecognizedMethodInfo(TR::java_lang_invoke_ComputedCalls_dispatchJ9Method); } + else if ((classNameLen == 30) && !strncmp(className, "java/util/ImmutableCollections", 30)) + { + setRecognizedMethodInfo(TR::java_util_ImmutableCollections_all); + } + else if ((classNameLen == 32) && !strncmp(className, "java/lang/invoke/InterfaceHandle", 32)) + { + if (!strncmp(name, "invokeExact_thunkArchetype_", 27)) + setRecognizedMethodInfo(TR::java_lang_invoke_InterfaceHandle_invokeExact); + } else if ((classNameLen >= 59 + 3 && classNameLen <= 59 + 7) && !strncmp(className, "java/lang/invoke/ArrayVarHandle$ArrayVarHandleOperations$Op", 59)) { setRecognizedMethodInfo(TR::java_lang_invoke_ArrayVarHandle_ArrayVarHandleOperations_OpMethod); diff --git a/runtime/compiler/il/J9MethodSymbol.cpp b/runtime/compiler/il/J9MethodSymbol.cpp index 03e98e13116..1e6bad8c82a 100644 --- a/runtime/compiler/il/J9MethodSymbol.cpp +++ b/runtime/compiler/il/J9MethodSymbol.cpp @@ -450,6 +450,96 @@ J9::MethodSymbol::safeToSkipArrayStoreChecks() return false; } +static TR::RecognizedMethod canSkipNonNullableArrayNullStoreCheck[] = { + TR::java_lang_invoke_CollectHandle_invokeExact, + TR::java_util_ArrayList_add, + TR::java_util_ArrayList_ensureCapacity, + TR::java_util_ArrayList_remove, + TR::java_util_ArrayList_set, + TR::java_util_Hashtable_get, + TR::java_util_Hashtable_put, + TR::java_util_Hashtable_clone, + TR::java_util_Hashtable_putAll, + TR::java_util_Hashtable_rehash, + TR::java_util_Hashtable_remove, + TR::java_util_Hashtable_contains, + TR::java_util_Hashtable_getEntry, + TR::java_util_Hashtable_getEnumeration, + TR::java_util_Hashtable_elements, + TR::java_util_HashtableHashEnumerator_hasMoreElements, + TR::java_util_HashtableHashEnumerator_nextElement, + TR::java_util_HashMap_rehash, + TR::java_util_HashMap_analyzeMap, + TR::java_util_HashMap_calculateCapacity, + TR::java_util_HashMap_findNullKeyEntry, + TR::java_util_HashMap_get, + TR::java_util_HashMap_getNode, + TR::java_util_HashMap_putImpl, + TR::java_util_HashMap_findNonNullKeyEntry, + TR::java_util_HashMap_resize, + TR::java_util_HashMap_prepareArray, + TR::java_util_HashMap_keysToArray, + TR::java_util_HashMap_valuesToArray, + TR::java_util_HashMapHashIterator_nextNode, + TR::java_util_HashMapHashIterator_init, + TR::java_util_Vector_addElement, + TR::java_util_Vector_contains, + TR::java_util_Vector_subList, + TR::java_util_concurrent_ConcurrentHashMap_addCount, + TR::java_util_concurrent_ConcurrentHashMap_tryPresize, + TR::java_util_concurrent_ConcurrentHashMap_transfer, + TR::java_util_concurrent_ConcurrentHashMap_fullAddCount, + TR::java_util_concurrent_ConcurrentHashMap_helpTransfer, + TR::java_util_concurrent_ConcurrentHashMap_initTable, + TR::java_util_concurrent_ConcurrentHashMap_tabAt, + TR::java_util_concurrent_ConcurrentHashMap_casTabAt, + TR::java_util_concurrent_ConcurrentHashMap_setTabAt, + TR::java_util_concurrent_ConcurrentHashMap_TreeBin_lockRoot, + TR::java_util_concurrent_ConcurrentHashMap_TreeBin_contendedLock, + TR::java_util_concurrent_ConcurrentHashMap_TreeBin_find, + TR::java_util_TreeMap_all, + TR::java_util_EnumMap_put, + TR::java_util_EnumMap_typeCheck, + TR::java_util_EnumMap__init_, + // TR::java_util_EnumMap__nec_, // Disable it for now because EnumMap.toArray explicitly stores null into array element + TR::java_util_HashMap_all, + // TR::java_util_ArrayList_all, // Disable it for now because ArrayList.toArray explicitly stores null into array element + TR::java_util_Hashtable_all, + // TR::java_util_concurrent_ConcurrentHashMap_all, // Disable it for now because ConcurrentHashMap.toArray explicitly stores null into array element + // TR::java_util_Vector_all, // Disable it for now because Vector.toArray explicitly stores null into array element + + // The following list is identified after running sanity.functional tests + // TR::java_util_AbstractCollection_all, // Disable it for now because AbstractCollection.toArray explicitly stores null into array element + // TR::java_util_ArrayDeque_all, // Disable it for now because ArrayDeque.toArray explicitly stores null into array element + TR::java_util_ComparableTimSort_all, + // TR::java_util_IdentityHashMap_all, // Disable it for now because IdentityHashMap.toArray explicitly stores null into array element + // TR::java_util_ImmutableCollections_all, // Disable it for now because ImmutableCollections.toArray explicitly stores null into array element + TR::java_util_LinkedHashMap_all, + // TR::java_util_LinkedList_all, // Disable it for now because LinkedList.toArray explicitly stores null into array element + TR::java_util_Map_all, + TR::java_util_regex_Pattern_all, + TR::java_util_stream_Nodes_all, + TR::java_util_TimSort_all, + TR::java_util_WeakHashMap_all, + + TR::java_lang_Class_all, + + TR::unknownMethod +}; + +bool +J9::MethodSymbol::safeToSkipNonNullableArrayNullStoreCheck() + { + TR::RecognizedMethod methodId = self()->getRecognizedMethod(); + if (methodId == TR::unknownMethod) + return false; + + for (int i = 0; canSkipNonNullableArrayNullStoreCheck[i] != TR::unknownMethod; ++i) + if (canSkipNonNullableArrayNullStoreCheck[i] == methodId) + return true; + + return false; + } // Which recognized methods are known to require no checking when lowering to TR::arraycopy // diff --git a/runtime/compiler/il/J9MethodSymbol.hpp b/runtime/compiler/il/J9MethodSymbol.hpp index 479d18ed3c0..298b207bd09 100644 --- a/runtime/compiler/il/J9MethodSymbol.hpp +++ b/runtime/compiler/il/J9MethodSymbol.hpp @@ -67,6 +67,7 @@ class OMR_EXTENSIBLE MethodSymbol : public OMR::MethodSymbolConnector bool safeToSkipDivChecks(); bool safeToSkipCheckCasts(); bool safeToSkipArrayStoreChecks(); + bool safeToSkipNonNullableArrayNullStoreCheck(); bool safeToSkipZeroInitializationOnNewarrays(); bool safeToSkipChecksOnArrayCopies(); diff --git a/runtime/compiler/ilgen/Walker.cpp b/runtime/compiler/ilgen/Walker.cpp index 72f23ee94fd..f568a8c0435 100644 --- a/runtime/compiler/ilgen/Walker.cpp +++ b/runtime/compiler/ilgen/Walker.cpp @@ -7471,10 +7471,26 @@ TR_J9ByteCodeIlGenerator::storeArrayElement(TR::DataType dataType, TR::ILOpCodes // we won't have flattening, so no call to flattenable array element access // helper is needed. // + + bool generateNonHelper = false; + if (TR::Compiler->om.areFlattenableValueTypesEnabled() && !TR::Compiler->om.canGenerateArraylets() && dataType == TR::Address) { + if (!TR::Compiler->om.isValueTypeArrayFlatteningEnabled() && + _methodSymbol->skipNonNullableArrayNullStoreCheck()) + { + generateNonHelper = false; + } + else + { + generateNonHelper = true; + } + } + + if (generateNonHelper) + { TR::Node* elementIndex = pop(); TR::Node* arrayBaseAddress = pop(); if (!arrayBaseAddress->isNonNull()) diff --git a/runtime/compiler/optimizer/J9ValuePropagation.cpp b/runtime/compiler/optimizer/J9ValuePropagation.cpp index a4f441df0a9..4ccddf1fa6e 100644 --- a/runtime/compiler/optimizer/J9ValuePropagation.cpp +++ b/runtime/compiler/optimizer/J9ValuePropagation.cpp @@ -647,6 +647,14 @@ static bool owningMethodDoesNotContainBoundChecks(OMR::ValuePropagation *vp, TR: return false; } +static bool owningMethodDoesNotContainNonNullableArrayNullStoreCheck(OMR::ValuePropagation *vp, TR::Node *node) + { + TR::ResolvedMethodSymbol *method = vp->comp()->getOwningMethodSymbol(node->getOwningMethod()); + if (method && method->skipNonNullableArrayNullStoreCheck()) + return true; + return false; + } + static TR::Node *getStoreValueBaseNode(TR::Node *storeValueNode, TR::SymbolReferenceTable *symRefTab) { TR::Node *storeValueBaseNode = NULL; @@ -1181,14 +1189,21 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node) flagsForTransform.set(ValueTypesHelperCallTransform::RequiresStoreCheck); } - //TODO: Require the non-helper if !canSkipNonNullableArrayNullValueChecks(...) // If the value being stored is NULL and the destination array component is null restricted in 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) && - (storeValueConstraint == NULL || !storeValueConstraint->isNonNullObject())) + (storeValueConstraint == NULL || !storeValueConstraint->isNonNullObject()) && + !owningMethodDoesNotContainNonNullableArrayNullStoreCheck(this, node)) { flagsForTransform.set(ValueTypesHelperCallTransform::RequiresNullValueCheck); + + const char *counterName = TR::DebugCounter::debugCounterName(comp(), "vt-helper/vp-nullvaluechk/aastore/(%s)/%s/block_%d", + comp()->signature(), + comp()->getHotnessName(comp()->getMethodHotness()), + _curTree->getEnclosingBlock()->getNumber()); + + TR::DebugCounter::prependDebugCounter(comp(), counterName, _curTree); } }