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 <[email protected]>
eclipse-openj9 · Jun 5, 2023 · 156b344 · 156b344
1 parent 178ee93
commit 156b344
Show file tree

Hide file tree

Showing 9 changed files with 135 additions and 88 deletions.
diff --git 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
@@ -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
@@ -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,15 +9301,15 @@ 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;
 
    J9VMThread *vmThread = fej9()->vmThread();
    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<J9Class *>(containingClass), fieldShape);
    }
diff --git 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
@@ -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
@@ -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<J9::AOTNoSupportForAOTFailure>("NO support for AOT in field watch");
 
    TR_ResolvedJ9Method * owningMethod = static_cast<TR_ResolvedJ9Method*>(_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<TR_ResolvedJ9Method*>(_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<J9::AOTNoSupportForAOTFailure>("NO support for AOT in field watch");
 
    TR_ResolvedJ9Method * owningMethod = static_cast<TR_ResolvedJ9Method*>(_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();