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 <[email protected]>
eclipse-openj9 · Dec 6, 2023 · 066df6b · 066df6b
1 parent 744bf55
commit 066df6b
Show file tree

Hide file tree

Showing 6 changed files with 172 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 <nonNullableArrayNullStoreCheck> 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);
                      }
                   }