Merge pull request #9139 from hzongaro/avoid-value-type-inline-alloca…

…tions

Avoid value type inline allocations

runtime/compiler/compile/J9Compilation.cpp

@@ -617,6 +617,8 @@ J9::Compilation::canAllocateInline(TR::Node* node, TR_OpaqueClassBlock* &classIn
 
    bool generateArraylets = self()->generateArraylets();
 
+   const bool areValueTypesEnabled = TR::Compiler->om.areValueTypesEnabled();
+
    if (node->getOpCodeValue() == TR::New)
       {
 
@@ -665,11 +667,26 @@ J9::Compilation::canAllocateInline(TR::Node* node, TR_OpaqueClassBlock* &classIn
       {
       classRef      = node->getSecondChild();
 
-      // In the case of dynamic array allocation, return 0 indicating variable dynamic array allocation
+      // In the case of dynamic array allocation, return 0 indicating variable dynamic array allocation,
+      // unless value types are enabled, in which case return -1 to prevent inline allocation
       if (classRef->getOpCodeValue() != TR::loadaddr)
          {
          classInfo = NULL;
-         return 0;
+         if (areValueTypesEnabled)
+            {
+            if (self()->getOption(TR_TraceCG))
+               {
+               traceMsg(self(), "cannot inline array allocation @ node %p because value types are enabled\n", node);
+               }
+            const char *signature = self()->signature();
+
+            TR::DebugCounter::incStaticDebugCounter(self(), TR::DebugCounter::debugCounterName(self(), "inlineAllocation/dynamicArray/failed/valueTypes/(%s)", signature));
+            return -1;
+            }
+         else
+            {
+            return 0;
+            }
          }
 
       classSymRef   = classRef->getSymbolReference();
@@ -679,8 +696,17 @@ J9::Compilation::canAllocateInline(TR::Node* node, TR_OpaqueClassBlock* &classIn
       if (clazz == NULL)
          return -1;
 
+      // Arrays of value type classes must have all their elements initialized with the
+      // default value of the component type.  For now, prevent inline allocation of them.
+      //
+      if (areValueTypesEnabled && TR::Compiler->cls.isValueTypeClass(reinterpret_cast<TR_OpaqueClassBlock*>(clazz)))
+         {
+         return -1;
+         }
+
       auto classOffset = self()->fej9()->getArrayClassFromComponentClass(TR::Compiler->cls.convertClassPtrToClassOffset(clazz));
       clazz = TR::Compiler->cls.convertClassOffsetToClassPtr(classOffset);
+
       if (!clazz)
          return -1;
 

runtime/compiler/env/VMJ9.cpp

@@ -5973,8 +5973,10 @@ TR_J9VMBase::canAllocateInlineClass(TR_OpaqueClassBlock *clazzOffset)
    if (clazz->initializeStatus != 1)
       return false;
 
-   // Can not inline the allocation if the class is an interface or abstract
-   if (clazz->romClass->modifiers & (J9AccAbstract | J9AccInterface))
+   // Can not inline the allocation if the class is an interface, abstract or identityless
+   // (a value type) or if it is a class with identityless fields
+   if ((clazz->romClass->modifiers & (J9AccAbstract | J9AccInterface | J9AccValueType))
+       || (clazz->classFlags & J9ClassContainsUnflattenedFlattenables))
       return false;
    return true;
    }

runtime/compiler/env/VMJ9Server.cpp

@@ -876,6 +876,7 @@ bool
 TR_J9ServerVM::canAllocateInlineClass(TR_OpaqueClassBlock *clazz)
    {
    uint32_t modifiers = 0;
+   uintptr_t classFlags = 0;
    bool isClassInitialized = false;
    JITServer::ServerStream *stream = _compInfoPT->getMethodBeingCompiled()->_stream;
    JITServerHelpers::getAndCacheRAMClassInfo((J9Class *)clazz, _compInfoPT->getClientData(), stream, JITServerHelpers::CLASSINFO_CLASS_INITIALIZED, (void *)&isClassInitialized, JITServerHelpers::CLASSINFO_ROMCLASS_MODIFIERS, (void *)&modifiers);
@@ -894,10 +895,13 @@ TR_J9ServerVM::canAllocateInlineClass(TR_OpaqueClassBlock *clazz)
            it->second._classInitialized = isClassInitialized;
            }
         }
+
+        classFlags = TR_J9ServerVM::getClassFlagsValue(clazz);
      }
 
    if (isClassInitialized)
-      return ((modifiers & (J9AccAbstract | J9AccInterface ))?false:true);
+      return ((modifiers & (J9AccAbstract | J9AccInterface | J9AccValueType)
+              || (classFlags & J9ClassContainsUnflattenedFlattenables)) ? false : true);
 
    return false;
    }

runtime/compiler/optimizer/EscapeAnalysis.cpp

@@ -1076,6 +1076,24 @@ int32_t TR_EscapeAnalysis::performAnalysisOnce()
                traceMsg(comp(), "   Make [%p] non-local because we can't have locking when candidate escapes in cold blocks\n", candidate->_node);
             }
 
+         // Value type fields of objects created with a NEW bytecode must be initialized
+         // with their default values.  EA is not yet set up to perform such iniitialization
+         // if the value type's own fields have not been inlined into the class that
+         // has a field of that type, so remove the candidate from consideration.
+         if (candidate->_kind == TR::New)
+            {
+            TR_OpaqueClassBlock *clazz = (TR_OpaqueClassBlock *)candidate->_node->getFirstChild()->getSymbol()->getStaticSymbol()->getStaticAddress();
+
+            if (!TR::Compiler->cls.isZeroInitializable(clazz))
+               {
+               if (trace())
+                  traceMsg(comp(), "   Fail [%p] because the candidate is not zero initializable (that is, it has a field of a value type whose fields have not been inlined into this candidate's class)\n", candidate->_node);
+               rememoize(candidate);
+               _candidates.remove(candidate);
+               continue;
+               }
+            }
+
          // If a contiguous candidate has reference slots, then stack-allocating it means putting
          // stores in the first block of the method.  If the first block is really hot, those stores
          // are expensive, and stack-allocation is probably not worthwhile.
@@ -1156,7 +1174,8 @@ int32_t TR_EscapeAnalysis::performAnalysisOnce()
          {
          // Array Candidates for contiguous allocation that have unresolved
          // base classes must be rejected, since we cannot initialize the array
-         // header
+         // header.  If the component type is a value type, reject the array
+         // as we can't initialize the elements to the default value yet.
          //
          if (candidate->isContiguousAllocation())
             {
@@ -1169,6 +1188,18 @@ int32_t TR_EscapeAnalysis::performAnalysisOnce()
                rememoize(candidate);
                _candidates.remove(candidate);
                }
+            else
+               {
+               TR_OpaqueClassBlock *clazz = (TR_OpaqueClassBlock*)classNode->getSymbol()->castToStaticSymbol()->getStaticAddress();
+
+               if (TR::Compiler->cls.isValueTypeClass(clazz))
+                  {
+                  if (trace())
+                     traceMsg(comp(), "   Fail [%p] because array has value type elements\n", candidate->_node);
+                  rememoize(candidate);
+                  _candidates.remove(candidate);
+                  }
+               }
             }
          }
 

runtime/oti/VMHelpers.hpp

@@ -771,7 +771,7 @@ class VM_VMHelpers
 #if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
 		if (J9_IS_J9CLASS_FLATTENED(arrayClass)) {
 			instance = objectAllocate->inlineAllocateIndexableValueTypeObject(currentThread, arrayClass, size, initializeSlots, memoryBarrier, sizeCheck);
-		} else
+		} else if (J9_ARE_NO_BITS_SET(arrayClass->classFlags, J9ClassContainsUnflattenedFlattenables))
 #endif /* J9VM_OPT_VALHALLA_VALUE_TYPES */
 		{
 			instance = objectAllocate->inlineAllocateIndexableObject(currentThread, arrayClass, size, initializeSlots, memoryBarrier, sizeCheck);