Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw IdentityException if operand of monent is a value type object #21218

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+162 −10
Draft

Throw IdentityException if operand of monent is a value type object #21218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions runtime/codert_vm/arm64nathelp.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ EXCEPTION_THROW_HELPER(jitThrowInstantiationException,0)
EXCEPTION_THROW_HELPER(jitThrowNullPointerException,0)
EXCEPTION_THROW_HELPER(jitThrowWrongMethodTypeException,0)
EXCEPTION_THROW_HELPER(jitThrowIncompatibleReceiver,2)
EXCEPTION_THROW_HELPER(jitThrowIdentityException,0)

dnl Write barrier helpers

Expand Down
1 change: 1 addition & 0 deletions runtime/codert_vm/armnathelp.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ EXCEPTION_THROW_HELPER(jitThrowInstantiationException,0)
EXCEPTION_THROW_HELPER(jitThrowNullPointerException,0)
EXCEPTION_THROW_HELPER(jitThrowWrongMethodTypeException,0)
EXCEPTION_THROW_HELPER(jitThrowIncompatibleReceiver,2)
EXCEPTION_THROW_HELPER(jitThrowIdentityException,0)

dnl Write barrier helpers

Expand Down
15 changes: 15 additions & 0 deletions runtime/codert_vm/cnathelp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2731,6 +2731,20 @@ old_slow_jitThrowArrayIndexOutOfBounds(J9VMThread *currentThread)
return setCurrentExceptionFromJIT(currentThread, J9VMCONSTANTPOOL_JAVALANGARRAYINDEXOUTOFBOUNDSEXCEPTION, NULL);
}

void* J9FASTCALL
old_slow_jitThrowIdentityException(J9VMThread *currentThread)
{
void *exception = NULL;
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
OLD_JIT_HELPER_PROLOGUE(0);
buildJITResolveFrameForRuntimeCheck(currentThread);
exception = setCurrentExceptionFromJIT(currentThread, J9VMCONSTANTPOOL_JAVALANGIDENTITYEXCEPTION, NULL);
#else /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
exception = (void *)-1;
#endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
return exception;
}

void* J9FASTCALL
impl_jitReferenceArrayCopy(J9VMThread *currentThread, UDATA lengthInBytes)
{
Expand Down Expand Up @@ -4000,6 +4014,7 @@ initPureCFunctionTable(J9JavaVM *vm)
jitConfig->old_slow_jitThrowInstantiationException = (void*)old_slow_jitThrowInstantiationException;
jitConfig->old_slow_jitThrowNullPointerException = (void*)old_slow_jitThrowNullPointerException;
jitConfig->old_slow_jitThrowWrongMethodTypeException = (void*)old_slow_jitThrowWrongMethodTypeException;
jitConfig->old_slow_jitThrowIdentityException = (void*)old_slow_jitThrowIdentityException;
jitConfig->old_fast_jitTypeCheckArrayStoreWithNullCheck = (void*)old_fast_jitTypeCheckArrayStoreWithNullCheck;
jitConfig->old_slow_jitTypeCheckArrayStoreWithNullCheck = (void*)old_slow_jitTypeCheckArrayStoreWithNullCheck;
jitConfig->old_fast_jitTypeCheckArrayStore = (void*)old_fast_jitTypeCheckArrayStore;
Expand Down
1 change: 1 addition & 0 deletions runtime/codert_vm/pnathelp.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ EXCEPTION_THROW_HELPER(jitThrowInstantiationException,0)
EXCEPTION_THROW_HELPER(jitThrowNullPointerException,0)
EXCEPTION_THROW_HELPER(jitThrowWrongMethodTypeException,0)
EXCEPTION_THROW_HELPER(jitThrowIncompatibleReceiver,2)
EXCEPTION_THROW_HELPER(jitThrowIdentityException,0)

dnl Write barrier helpers

Expand Down
1 change: 1 addition & 0 deletions runtime/codert_vm/riscvnathelp.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ EXCEPTION_THROW_HELPER(jitThrowIncompatibleClassChangeError,0)
EXCEPTION_THROW_HELPER(jitThrowInstantiationException,0)
EXCEPTION_THROW_HELPER(jitThrowNullPointerException,0)
EXCEPTION_THROW_HELPER(jitThrowWrongMethodTypeException,0)
EXCEPTION_THROW_HELPER(jitThrowIdentityException,0)
EXCEPTION_THROW_HELPER(jitThrowIncompatibleReceiver,2)

dnl Write barrier helpers
Expand Down
1 change: 1 addition & 0 deletions runtime/codert_vm/xnathelp.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ EXCEPTION_THROW_HELPER(jitThrowInstantiationException,0)
EXCEPTION_THROW_HELPER(jitThrowNullPointerException,0)
EXCEPTION_THROW_HELPER(jitThrowWrongMethodTypeException,0)
EXCEPTION_THROW_HELPER(jitThrowIncompatibleReceiver,2)
EXCEPTION_THROW_HELPER(jitThrowIdentityException,0)

dnl Write barrier helpers

Expand Down
1 change: 1 addition & 0 deletions runtime/codert_vm/znathelp.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ EXCEPTION_THROW_HELPER(jitThrowInstantiationException,0)
EXCEPTION_THROW_HELPER(jitThrowNullPointerException,0)
EXCEPTION_THROW_HELPER(jitThrowWrongMethodTypeException,0)
EXCEPTION_THROW_HELPER(jitThrowIncompatibleReceiver,2)
EXCEPTION_THROW_HELPER(jitThrowIdentityException,0)

dnl Write barrier helpers

Expand Down
3 changes: 2 additions & 1 deletion runtime/compiler/compile/J9AliasBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,8 @@ J9::AliasBuilder::createAliasInfo()
TR_methodTypeCheck,
TR_incompatibleReceiver,
TR_IncompatibleClassChangeError,
TR_multiANewArray
TR_multiANewArray,
TR_identityException
};

for (i = 0; i < (sizeof(helpersThatMayThrow) / 4); ++i)
Expand Down
19 changes: 19 additions & 0 deletions runtime/compiler/compile/J9SymbolReferenceTable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1318,6 +1318,12 @@ J9::SymbolReferenceTable::findOrCreateIncompatibleReceiverSymbolRef(TR::Resolved
return findOrCreateRuntimeHelper(TR_incompatibleReceiver, false, true, true);
}

TR::SymbolReference *
J9::SymbolReferenceTable::findOrCreateIdentityExceptionSymbolRef(TR::ResolvedMethodSymbol *)
{
return findOrCreateRuntimeHelper(TR_identityException, false, true, true);
}

TR::SymbolReference *
J9::SymbolReferenceTable::findOrCreateIncompatibleClassChangeErrorSymbolRef(TR::ResolvedMethodSymbol *)
{
Expand Down Expand Up @@ -2712,6 +2718,19 @@ J9::SymbolReferenceTable::findOrCreateStoreFlattenableArrayElementNonHelperSymbo
return symRef;
}

TR::SymbolReference *
J9::SymbolReferenceTable::findOrCreateIsIdentityObjectNonHelperSymbolRef()
{
TR::SymbolReference *symRef = element(isIdentityObjectNonHelperSymbol);
if (symRef != NULL)
{
return symRef;
}

symRef = self()->findOrCreateCodeGenInlinedHelper(isIdentityObjectNonHelperSymbol);
return symRef;
}

TR::ParameterSymbol *
J9::SymbolReferenceTable::createParameterSymbol(
TR::ResolvedMethodSymbol *owningMethodSymbol,
Expand Down
24 changes: 24 additions & 0 deletions runtime/compiler/compile/J9SymbolReferenceTable.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,18 @@ class SymbolReferenceTable : public OMR::SymbolReferenceTableConnector
TR::SymbolReference * findOrCreateArrayComponentTypeAsPrimitiveSymbolRef();
TR::SymbolReference * findOrCreateMethodTypeCheckSymbolRef(TR::ResolvedMethodSymbol * owningMethodSymbol);
TR::SymbolReference * findOrCreateIncompatibleReceiverSymbolRef(TR::ResolvedMethodSymbol * owningMethodSymbol);

/**
* Used to find the symbol reference for \c java/lang/IdentityException. If it does not already exist,
* it will be created.
*
* \param owningMethodSymbol
* The method in which the IdentityException symbol reference needs to be created.
*
* \returns
* A symbol reference for \c java/lang/IdentityException
*/
TR::SymbolReference * findOrCreateIdentityExceptionSymbolRef(TR::ResolvedMethodSymbol * owningMethodSymbol);
TR::SymbolReference * findOrCreateIncompatibleClassChangeErrorSymbolRef(TR::ResolvedMethodSymbol * owningMethodSymbol);
TR::SymbolReference * findOrCreateReportStaticMethodEnterSymbolRef(TR::ResolvedMethodSymbol * owningMethodSymbol);
TR::SymbolReference * findOrCreateReportMethodExitSymbolRef(TR::ResolvedMethodSymbol * owningMethodSymbol);
Expand Down Expand Up @@ -415,6 +427,18 @@ class SymbolReferenceTable : public OMR::SymbolReferenceTableConnector
*/
TR::SymbolReference *findOrCreateStoreFlattenableArrayElementNonHelperSymbolRef();

/**
* \brief
* Finds the <isIdentityObject> "non-helper" symbol reference, creating it if
* necessary. The non-helper is used to test whether an object is an instance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An extra space in necessary. The

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was intentional. I always use two spaces after the period at the end of sentence, or after a colon. It's a style that was common using typewriters that printed using monospace fonts like Courier, and as we usually look at our code in Courier font, I prefer to continue to follow that convention.

* of an identity class, in which case it returns the value one, or a value type
* class, in which case it returns the value zero.
*
* \return
* The <isIdentityObject> symbol reference
*/
TR::SymbolReference *findOrCreateIsIdentityObjectNonHelperSymbolRef();

/**
* \brief
* Creates a new symbol for a parameter within the supplied owning method of the
Expand Down
28 changes: 24 additions & 4 deletions runtime/compiler/ilgen/Walker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6047,11 +6047,31 @@ TR_J9ByteCodeIlGenerator::genMonitorEnter()
}
*/

node = TR::Node::createWithSymRef(TR::monent, 1, 1, node, monitorEnterSymbolRef);
if (isStatic)
node->setStaticMonitor(true);
static const bool disableMonentIdentityException = (feGetEnv("TR_disableMonentIdentityException") != NULL);

genTreeTop(genNullCheck(node));
if (disableMonentIdentityException || !TR::Compiler->om.areValueTypesEnabled())
{
node = TR::Node::createWithSymRef(TR::monent, 1, 1, node, monitorEnterSymbolRef);
if (isStatic)
node->setStaticMonitor(true);

genTreeTop(genNullCheck(node));
}
else
{
genTreeTop(genNullCheck(TR::Node::create(TR::PassThrough, 1, node)));

TR::SymbolReference *isIdentitySymRef = comp()->getSymRefTab()->findOrCreateIsIdentityObjectNonHelperSymbolRef();
TR::Node *isIdentityObjectTestNode = TR::Node::createWithSymRef(TR::icall, 1, 1, node, isIdentitySymRef);
TR::SymbolReference *identityExceptionSymRef = comp()->getSymRefTab()->findOrCreateIdentityExceptionSymbolRef(_methodSymbol);
genTreeTop(TR::Node::createWithSymRef(TR::ZEROCHK, 1, 1, isIdentityObjectTestNode, identityExceptionSymRef));

node = TR::Node::createWithSymRef(TR::monent, 1, 1, node, monitorEnterSymbolRef);
if (isStatic)
node->setStaticMonitor(true);

genTreeTop(node);
}

if (!comp()->getOption(TR_DisableLiveMonitorMetadata))
{
Expand Down
58 changes: 58 additions & 0 deletions runtime/compiler/optimizer/TreeLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1536,6 +1536,60 @@ StoreArrayElementTransformer::lower(TR::Node* const node, TR::TreeTop* const tt)
}
}

class IsIdentityObjectTransformer: public TR::TreeLowering::Transformer
{
public:
explicit IsIdentityObjectTransformer(TR::TreeLowering* opt)
: TR::TreeLowering::Transformer(opt)
{}

void lower(TR::Node* const node, TR::TreeTop* const tt);
};


/**
* @brief Perform lowering of calls to the <isIdentityObject> non-helper function
*
* A call like the following
*
* @verbatim
n88n icall <isIdentityObject>
n77n aload x
* @endverbatim
*
* will be transformed into
*
* @verbatim
n88n PassThrough
n99n iand
n98n iloadi <isClassFlags>
n97n aloadi <vft-symbol>
n77n aload x
n96n iconst 0x80000 // Test whether class is an identity class
* @endverbatim
Comment on lines +1562 to +1569
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iconst node doesn't seem indented correctly to align with the iloadi node. I also wonder if putting the comment at the iand node could make it clearer.

 * @verbatim
   n88n  PassThrough
   n99n    iand                     // Test whether class is an identity class
   n98n      iloadi  <isClassFlags>
   n97n        aloadi  <vft-symbol>
   n77n          aload x
   n96n      iconst 0x80000
 * @endverbatim

*/
void
IsIdentityObjectTransformer::lower(TR::Node* const node, TR::TreeTop* const tt)
{
// If the argument to the call of the <isIdentityObject> non-helper is the
// object of a NULLCHK, pull the NULLCHK into a separate tree before
// transforming the call to <isIdentityObject>. Otherwise, we'll end up
// with the NULLCHK operating on something meaningless.
//
if (tt->getNode()->getOpCode().isNullCheck() && tt->getNode()->getFirstChild() == node)
{
J9::TransformUtil::separateNullCheck(comp(), tt, trace());
}

TR::SymbolReference *vftSymRef = comp()->getSymRefTab()->findOrCreateVftSymbolRef();
TR::Node *objNode = node->getFirstChild();
TR::Node *vftNode = TR::Node::createWithSymRef(TR::aloadi, 1, 1, objNode, vftSymRef);
TR::Node *testFlagsNode = comp()->fej9()->testIsClassIdentityType(vftNode);
TR::Node::recreate(node, TR::PassThrough);
objNode->decReferenceCount();
node->setAndIncChild(0, testFlagsNode);
}

/**
* @brief Perform lowering related to Valhalla value types
*
Expand Down Expand Up @@ -1601,5 +1655,9 @@ TR::TreeLowering::lowerValueTypeOperations(TransformationManager& transformation
transformations.addTransformation(getTransformer<StoreArrayElementTransformer>(), node, tt);
}
}
else if (symRefTab->isNonHelper(node->getSymbolReference(), TR::SymbolReferenceTable::isIdentityObjectNonHelperSymbol))
{
transformations.addTransformation(getTransformer<IsIdentityObjectTransformer>(), node, tt);
}
}
}
14 changes: 9 additions & 5 deletions runtime/compiler/runtime/Runtime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1052,12 +1052,12 @@ void initializeCodeRuntimeHelperTable(J9JITConfig *jitConfig, char isSMP)
SET(TR_newValueNoZeroInit, (void *)jitNewValueNoZeroInit, TR_CHelper);

SET(TR_getFlattenableField, (void *)jitGetFlattenableField, TR_Helper);
SET(TR_withFlattenableField, (void *)jitWithFlattenableField, TR_Helper);
SET(TR_withFlattenableField, (void *)jitWithFlattenableField, TR_Helper);
SET(TR_putFlattenableField, (void *)jitPutFlattenableField, TR_Helper);
SET(TR_getFlattenableStaticField, (void *)jitGetFlattenableStaticField, TR_Helper);
SET(TR_putFlattenableStaticField, (void *)jitPutFlattenableStaticField, TR_Helper);
SET(TR_ldFlattenableArrayElement, (void *)jitLoadFlattenableArrayElement, TR_Helper);
SET(TR_strFlattenableArrayElement, (void *)jitStoreFlattenableArrayElement, TR_Helper);
SET(TR_getFlattenableStaticField, (void *)jitGetFlattenableStaticField, TR_Helper);
SET(TR_putFlattenableStaticField, (void *)jitPutFlattenableStaticField, TR_Helper);
SET(TR_ldFlattenableArrayElement, (void *)jitLoadFlattenableArrayElement, TR_Helper);
SET(TR_strFlattenableArrayElement, (void *)jitStoreFlattenableArrayElement, TR_Helper);

SET(TR_acmpeqHelper, (void *)jitAcmpeqHelper, TR_Helper);
SET(TR_acmpneHelper, (void *)jitAcmpneHelper, TR_Helper);
Expand All @@ -1081,6 +1081,10 @@ void initializeCodeRuntimeHelperTable(J9JITConfig *jitConfig, char isSMP)
SET(TR_typeCheckArrayStore, (void *)jitTypeCheckArrayStoreWithNullCheck, TR_Helper);
#endif

#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
SET(TR_identityException, (void *)jitThrowIdentityException, TR_Helper);
#endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
Comment on lines +1084 to +1086
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if TR_identityException should be guarded with J9VM_OPT_VALHALLA_VALUE_TYPES in this file. First, the JIT code generally doesn't guard Valhalla specific change with build flags such as J9VM_OPT_VALHALLA_VALUE_TYPES. Secondly, anywhere else that references TR_identityException is not guarded with this flag either.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think at one point, I had the entire definition of old_slow_jitThrowIdentityException enclosed in an

#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)

and so I conditionally compiled these uses of jitThrowIdentityException. But then I ended up with link errors on a couple of platforms when building without valhalla enabled, so I adjusted the definition of old_slow_jitThrowIdentityException to allow it to compile even without value types enabled. But I forgot to remove these ifdefs.

Yes, I think they can safely be removed. Thanks for pointing that out!


#if defined(TR_HOST_X86) || defined(TR_HOST_POWER) || defined(TR_HOST_S390) || defined(TR_HOST_ARM64)
SET(TR_softwareReadBarrier, (void *)jitSoftwareReadBarrier, TR_Helper);
#endif
Expand Down
3 changes: 3 additions & 0 deletions runtime/compiler/runtime/asmprotos.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ JIT_HELPER(jitThrowCurrentException); // asm calling-convention helper
JIT_HELPER(jitThrowException); // asm calling-convention helper
JIT_HELPER(jitThrowUnreportedException); // asm calling-convention helper
JIT_HELPER(jitThrowExceptionInInitializerError); // asm calling-convention helper
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
JIT_HELPER(jitThrowIdentityException); // asm calling-convention helper
#endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
Comment on lines +146 to +148
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as the above regarding to J9VM_OPT_VALHALLA_VALUE_TYPES.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the previous comment - I think I had left this ifdef in because of the way I defined old_slow_jitThrowIdentityException in an earlier version of the code. I will remove it.

JIT_HELPER(jitThrowInstantiationException); // asm calling-convention helper
JIT_HELPER(jitThrowNullPointerException); // asm calling-convention helper
JIT_HELPER(jitThrowWrongMethodTypeException); // asm calling-convention helper
Expand Down
1 change: 1 addition & 0 deletions runtime/jilgen/jilconsts.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,7 @@ writeConstants(OMRPortLibrary *OMRPORTLIB, IDATA fd)
writeConstant(OMRPORTLIB, fd, "J9TR_JitConfig_old_slow_jitThrowInstantiationException", offsetof(J9JITConfig, old_slow_jitThrowInstantiationException)) |
writeConstant(OMRPORTLIB, fd, "J9TR_JitConfig_old_slow_jitThrowNullPointerException", offsetof(J9JITConfig, old_slow_jitThrowNullPointerException)) |
writeConstant(OMRPORTLIB, fd, "J9TR_JitConfig_old_slow_jitThrowWrongMethodTypeException", offsetof(J9JITConfig, old_slow_jitThrowWrongMethodTypeException)) |
writeConstant(OMRPORTLIB, fd, "J9TR_JitConfig_old_slow_jitThrowIdentityException", offsetof(J9JITConfig, old_slow_jitThrowIdentityException)) |
writeConstant(OMRPORTLIB, fd, "J9TR_JitConfig_old_fast_jitTypeCheckArrayStoreWithNullCheck", offsetof(J9JITConfig, old_fast_jitTypeCheckArrayStoreWithNullCheck)) |
writeConstant(OMRPORTLIB, fd, "J9TR_JitConfig_old_slow_jitTypeCheckArrayStoreWithNullCheck", offsetof(J9JITConfig, old_slow_jitTypeCheckArrayStoreWithNullCheck)) |
writeConstant(OMRPORTLIB, fd, "J9TR_JitConfig_old_fast_jitTypeCheckArrayStore", offsetof(J9JITConfig, old_fast_jitTypeCheckArrayStore)) |
Expand Down
1 change: 1 addition & 0 deletions runtime/oti/j9nonbuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4107,6 +4107,7 @@ typedef struct J9JITConfig {
void *old_slow_jitThrowInstantiationException;
void *old_slow_jitThrowNullPointerException;
void *old_slow_jitThrowWrongMethodTypeException;
void *old_slow_jitThrowIdentityException;
void *old_fast_jitTypeCheckArrayStoreWithNullCheck;
void *old_slow_jitTypeCheckArrayStoreWithNullCheck;
void *old_fast_jitTypeCheckArrayStore;
Expand Down