Skip to content

Commit

Permalink
Remove isPrimitiveValueTypeClass
Browse files Browse the repository at this point in the history
Also updated code comments where the old
primitive value type is referenced

Related: eclipse-openj9#18157
Signed-off-by: Annabelle Huo <[email protected]>
  • Loading branch information
a7ehuo committed Oct 24, 2024
1 parent f165fd4 commit ab38690
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 56 deletions.
13 changes: 4 additions & 9 deletions runtime/compiler/compile/J9Compilation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ J9::Compilation::canAllocateInline(TR::Node* node, TR_OpaqueClassBlock* &classIn
}
else if (node->getOpCodeValue() == TR::anewarray)
{
classRef = node->getSecondChild();
classRef = node->getSecondChild();

// 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
Expand All @@ -727,20 +727,15 @@ J9::Compilation::canAllocateInline(TR::Node* node, TR_OpaqueClassBlock* &classIn
}
}

classSymRef = classRef->getSymbolReference();
classSymRef = classRef->getSymbolReference();
// Can't skip the allocation if the class is unresolved
//
clazz = self()->fej9vm()->getClassForAllocationInlining(self(), classSymRef);
if (clazz == NULL)
return -1;

// Arrays of null-restricted (a.k.a, primitive 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.isPrimitiveValueTypeClass(reinterpret_cast<TR_OpaqueClassBlock*>(clazz)))
{
return -1;
}
// TODO-VALUETYPE: If null-restricted arrays are ever allocated using TR::anewarray,
// the JIT will need to handle the inline initialization or prevent inline allocation.

auto classOffset = self()->fej9()->getArrayClassFromComponentClass(TR::Compiler->cls.convertClassPtrToClassOffset(clazz));
clazz = TR::Compiler->cls.convertClassOffsetToClassPtr(classOffset);
Expand Down
23 changes: 0 additions & 23 deletions runtime/compiler/env/J9ClassEnv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -833,29 +833,6 @@ J9::ClassEnv::isValueTypeClass(TR_OpaqueClassBlock *clazz)
return J9_IS_J9CLASS_VALUETYPE(j9class);
}

bool
J9::ClassEnv::isPrimitiveValueTypeClass(TR_OpaqueClassBlock *clazz)
{
#if defined(J9VM_OPT_JITSERVER)
if (auto stream = TR::CompilationInfo::getStream())
{
uintptr_t classFlags = 0;
JITServerHelpers::getAndCacheRAMClassInfo((J9Class *)clazz, TR::compInfoPT->getClientData(), stream, JITServerHelpers::CLASSINFO_CLASS_FLAGS, (void *)&classFlags);
#ifdef DEBUG
stream->write(JITServer::MessageType::ClassEnv_classFlagsValue, clazz);
uintptr_t classFlagsRemote = std::get<0>(stream->read<uintptr_t>());
// Check that class flags from remote call is equal to the cached ones
classFlags = classFlags & J9ClassIsPrimitiveValueType;
classFlagsRemote = classFlagsRemote & J9ClassIsPrimitiveValueType;
TR_ASSERT(classFlags == classFlagsRemote, "remote call class flags is not equal to cached class flags");
#endif
return classFlags & J9ClassIsPrimitiveValueType;
}
#endif /* defined(J9VM_OPT_JITSERVER) */
J9Class *j9class = reinterpret_cast<J9Class*>(clazz);
return J9_IS_J9CLASS_PRIMITIVE_VALUETYPE(j9class);
}

bool
J9::ClassEnv::isValueTypeClassFlattened(TR_OpaqueClassBlock *clazz)
{
Expand Down
3 changes: 1 addition & 2 deletions runtime/compiler/env/J9ClassEnv.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ class OMR_EXTENSIBLE ClassEnv : public OMR::ClassEnvConnector
bool isInterfaceClass(TR::Compilation *comp, TR_OpaqueClassBlock *clazzPointer);
bool isConcreteClass(TR::Compilation *comp, TR_OpaqueClassBlock * clazzPointer);
bool isValueTypeClass(TR_OpaqueClassBlock *);
bool isPrimitiveValueTypeClass(TR_OpaqueClassBlock *);
bool isValueTypeClassFlattened(TR_OpaqueClassBlock *clazz);
bool isValueBasedOrValueTypeClass(TR_OpaqueClassBlock *);
bool isArrayNullRestricted(TR::Compilation *comp, TR_OpaqueClassBlock *arrayClass);
Expand Down Expand Up @@ -145,7 +144,7 @@ class OMR_EXTENSIBLE ClassEnv : public OMR::ClassEnvConnector
* \brief
* Checks whether instances of the specified class can be trivially initialized by
* "zeroing" their fields.
* In the case of OpenJ9, this tests whether any field is of a primitive value type that
* In the case of OpenJ9, this tests whether any field is of null-restricted type that
* has not been "flattened" (that is, had the value type's fields inlined into this class).
* Such a value type field must be initialized with the default value of the type.
*
Expand Down
2 changes: 1 addition & 1 deletion runtime/compiler/env/VMJ9.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6324,7 +6324,7 @@ TR_J9VMBase::canAllocateInlineClass(TR_OpaqueClassBlock *clazzOffset)
return false;

// Cannot inline the allocation if the class is an interface, abstract,
// or if it is a class with identityless primitive value type fields that
// or if it is a class with identityless null-restricted fields that
// aren't flattened, because they have to be made to refer to their type's
// default values
if ((clazz->romClass->modifiers & (J9AccAbstract | J9AccInterface))
Expand Down
18 changes: 4 additions & 14 deletions runtime/compiler/optimizer/EscapeAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1187,12 +1187,14 @@ int32_t TR_EscapeAnalysis::performAnalysisOnce()
continue;
}

// TODO-VALUETYPE: Need to handle allocation of null-restricted arrays if
// null-restricted arrays are ever allocated using TR::anewarray and we
// want to allow for stack allocation of null-restricted arrays
if (candidate->_kind == TR::anewarray)
{
// Array Candidates for contiguous allocation that have unresolved
// base classes must be rejected, since we cannot initialize the array
// header. If the component type is a primitive value type, reject the array
// as we can't initialize the elements to the default value yet.
// header.
//
if (candidate->isContiguousAllocation())
{
Expand All @@ -1205,18 +1207,6 @@ int32_t TR_EscapeAnalysis::performAnalysisOnce()
rememoize(candidate);
_candidates.remove(candidate);
}
else
{
TR_OpaqueClassBlock *clazz = (TR_OpaqueClassBlock*)classNode->getSymbol()->castToStaticSymbol()->getStaticAddress();

if (TR::Compiler->cls.isPrimitiveValueTypeClass(clazz))
{
if (trace())
traceMsg(comp(), " Fail [%p] because array has primitive value type elements\n", candidate->_node);
rememoize(candidate);
_candidates.remove(candidate);
}
}
}
}

Expand Down
7 changes: 4 additions & 3 deletions runtime/compiler/optimizer/J9ValuePropagation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1096,7 +1096,7 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node)
}

if (!canTransformIdentityArrayElementLoadStoreUseTypeHint &&
TR::Compiler->cls.isPrimitiveValueTypeClass(hintComponentClass))
TR::Compiler->cls.isArrayNullRestricted(comp(), typeHintClass)) //TODO-VALUETYPE: typeHintClass eventually should be the null-restricted array class
{
if (TR::Compiler->cls.isValueTypeClassFlattened(hintComponentClass))
{
Expand Down Expand Up @@ -1719,8 +1719,9 @@ J9::ValuePropagation::constrainRecognizedMethod(TR::Node *node)
// (ii) The operand is definitely a non-null instance of java.lang.Class. In that case, load the
// J9Class from the java.lang.Class by way of <classFromJavaLangClass>
//
// The result of Class.isValueType(), Class.isPrimitiveValueType() or Class.isIdentity() can then
// be determined by checking the corresponding bit in the classFlags field.
// The result of Class.isValueType() and Class.isIdentity() can then be determined by checking
// the corresponding bit in the classFlags field.
//
TR::SymbolReference *symRef = classChild->getOpCode().hasSymbolReference() ? classChild->getSymbolReference() : NULL;
TR::Node *classOperand = NULL;

Expand Down
8 changes: 4 additions & 4 deletions runtime/compiler/optimizer/TreeLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@ static bool skipBoundChecks(TR::Compilation *comp, TR::Node *node)
* LoadArrayElementTransformer transforms the block that contains the jitLoadFlattenableArrayElement helper call into three blocks:
* 1. The merge block (blockAfterHelperCall) that contains the tree tops after the helper call
* 2. The helper call block (helperCallBlock) that contains the helper call and is moved to the end of the tree top list
* 3. The new non-primitive VT array load block (arrayElementLoadBlock) which is an extended block of the original block
* 3. The new nullable array load block (arrayElementLoadBlock) which is an extended block of the original block
*
* originalBlock----------+
* arrayElementLoadBlock |
Expand Down Expand Up @@ -996,7 +996,7 @@ LoadArrayElementTransformer::lower(TR::Node* const node, TR::TreeTop* const tt)
// 1. Anchor helper call node after the helper call
// Anchor elementIndex and arrayBaseAddress before the helper call

// Anchoring the helper call ensures that the return value from the helper call or from the non-primitive VT array element load
// Anchoring the helper call ensures that the return value from the helper call or from the nullable array element load
// will be saved to a register or a temp.
TR::TreeTop *anchoredCallTT = TR::TreeTop::create(comp, tt, TR::Node::create(TR::treetop, 1, node));
TR::TreeTop *anchoredElementIndexTT = TR::TreeTop::create(comp, tt->getPrevTreeTop(), TR::Node::create(TR::treetop, 1, elementIndexNode));
Expand Down Expand Up @@ -1206,7 +1206,7 @@ static bool skipArrayStoreChecks(TR::Compilation *comp, TR::Node *node)
* StoreArrayElementTransformer transforms the block that contains the jitStoreFlattenableArrayElement helper call into three blocks:
* 1. The merge block that contains the tree tops after the helper call
* 2. The helper call block that contains the helper call and is moved to the end of the tree top list
* 3. The new non-primitive VT array store block which is an extended block of the original block
* 3. The new nullable array store block which is an extended block of the original block
*
* originalBlock ----------+
* arrayElementStoreBlock |
Expand Down Expand Up @@ -1372,7 +1372,7 @@ StoreArrayElementTransformer::lower(TR::Node* const node, TR::TreeTop* const tt)
///////////////////////////////////////
// 5. Split (2) at the helper call node into its own helperCallBlock

// Insert NULLCHK for Primitive VT
// Insert NULLCHK for null-restricted VT
TR::Node *anchoredValueNode = anchoredValueTT->getNode()->getFirstChild();
TR::TreeTop *ttForHelperCallBlock = tt;

Expand Down

0 comments on commit ab38690

Please sign in to comment.