-
Notifications
You must be signed in to change notification settings - Fork 397
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.
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
Improvements for Relocatable Compilations #2978
Changes from all commits
e3f3e09
6eaccdb
5e15900
5f2e1cc
2347110
7ba167c
7df29e2
bf3d1ec
a18a64a
1520ed0
34f7416
482c102
e5c1d42
ace92d2
d96f3ba
e0bc346
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8058,7 +8058,9 @@ TR_ColdBlockMarker::hasNotYetRun(TR::Node *node) | |
TR::SymbolReference *symRef = node->getSymbolReference(); | ||
bool isUnresolved; | ||
|
||
if (comp()->compileRelocatableCode() && !comp()->getOption(TR_DisablePeekAOTResolutions)) | ||
if (comp()->compileRelocatableCode() && | ||
!comp()->getOption(TR_UseSymbolValidationManager) && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you just trying to preserve the old behaviour here by overloading on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah that's exactly it. I wanted there to be a way for the old code to run exactly as it did without all the new changes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although... the |
||
!comp()->getOption(TR_DisablePeekAOTResolutions)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need a more detailed commit message to cover these changes. |
||
isUnresolved = symRef->isUnresolvedMethodInCP(comp()); | ||
else | ||
isUnresolved = symRef->isUnresolved(); | ||
|
@@ -8090,7 +8092,9 @@ TR_ColdBlockMarker::hasNotYetRun(TR::Node *node) | |
} | ||
else | ||
{ | ||
if (comp()->compileRelocatableCode() && !comp()->getOption(TR_DisablePeekAOTResolutions)) | ||
if (comp()->compileRelocatableCode() && | ||
!comp()->getOption(TR_UseSymbolValidationManager) && | ||
!comp()->getOption(TR_DisablePeekAOTResolutions)) | ||
{ | ||
bool isUnresolved = node->getSymbolReference()->isUnresolvedFieldInCP(comp()); | ||
//currentely node->hasUnresolvedSymbolReference() returns true more often for AOT than non-AOT beacause of | ||
|
@@ -8104,7 +8108,15 @@ TR_ColdBlockMarker::hasNotYetRun(TR::Node *node) | |
return isUnresolved; | ||
} | ||
else | ||
{ | ||
if (comp()->compileRelocatableCode() | ||
&& comp()->getOption(TR_UseSymbolValidationManager) | ||
&& node->getSymbol()->isConstString()) | ||
{ | ||
return false; | ||
} | ||
return true; | ||
} | ||
} | ||
} | ||
return false; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1215,8 +1215,9 @@ TR::VPClassType *TR::VPClassType::create(OMR::ValuePropagation *vp, const char * | |
if (classObject) | ||
{ | ||
bool isClassInitialized = false; | ||
bool allowForAOT = vp->comp()->getOption(TR_UseSymbolValidationManager); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. another commit that should have a more meaningful commit title and message |
||
TR_PersistentClassInfo * classInfo = | ||
vp->comp()->getPersistentInfo()->getPersistentCHTable()->findClassInfoAfterLocking(classObject, vp->comp()); | ||
vp->comp()->getPersistentInfo()->getPersistentCHTable()->findClassInfoAfterLocking(classObject, vp->comp(), allowForAOT); | ||
if (classInfo && classInfo->isInitialized()) | ||
{ | ||
if (isFixed) | ||
|
@@ -1293,6 +1294,8 @@ TR::VPKnownObject *TR::VPKnownObject::create(OMR::ValuePropagation *vp, TR::Know | |
{ | ||
TR::KnownObjectTable *knot = vp->comp()->getKnownObjectTable(); | ||
TR_ASSERT(knot, "Can't create a TR::VPKnownObject without a known-object table"); | ||
if (!knot) | ||
return NULL; | ||
if (knot->isNull(index)) // No point in worrying about the NULL case because existing constraints handle that optimally | ||
return NULL; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1599,6 +1599,7 @@ TR::Node *constrainAload(OMR::ValuePropagation *vp, TR::Node *node) | |
sym->isFinal())) | ||
haveClassLookaheadInfo = true; | ||
|
||
bool allowForAOT = vp->comp()->getOption(TR_UseSymbolValidationManager); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given only classes are actually covered properly by the symbol validation manager (methods still use the "old" approach)...have you convinced yourself that the enhanced knowledge about classes doesn't end up enabling an incorrect runtime state due to the methods derived from those classes? Or will that be protected because the queries on the resulting method won't let things get out of hand? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From what I've seen,
since I can't think of any good reason why the "new" approach shouldn't working, Towards the end of this work, I did find a bug in this code that I fixed, so perhaps I won't see that crash now. |
||
if (haveClassLookaheadInfo) | ||
{ | ||
bool foundInfo = false; | ||
|
@@ -1623,13 +1624,12 @@ TR::Node *constrainAload(OMR::ValuePropagation *vp, TR::Node *node) | |
} | ||
bool isClassInitialized = false; | ||
TR_PersistentClassInfo * classInfo = | ||
vp->comp()->getPersistentInfo()->getPersistentCHTable()->findClassInfoAfterLocking(classOfStatic, vp->comp()); | ||
vp->comp()->getPersistentInfo()->getPersistentCHTable()->findClassInfoAfterLocking(classOfStatic, vp->comp(), allowForAOT); | ||
if (classInfo && classInfo->isInitialized()) | ||
isClassInitialized = true; | ||
|
||
if (classOfStatic != vp->comp()->getSystemClassPointer() && | ||
isClassInitialized && | ||
!vp->comp()->getOption(TR_AOT) && | ||
(type == TR::Address)) | ||
{ | ||
TR::VMAccessCriticalSection constrainAloadCriticalSection(vp->comp(), | ||
|
@@ -1663,7 +1663,7 @@ TR::Node *constrainAload(OMR::ValuePropagation *vp, TR::Node *node) | |
if (!foundInfo) | ||
{ | ||
TR_PersistentClassInfo * classInfo = | ||
vp->comp()->getPersistentInfo()->getPersistentCHTable()->findClassInfoAfterLocking(symRef->getOwningMethod(vp->comp())->classOfStatic(symRef->getCPIndex()), vp->comp()); | ||
vp->comp()->getPersistentInfo()->getPersistentCHTable()->findClassInfoAfterLocking(symRef->getOwningMethod(vp->comp())->classOfStatic(symRef->getCPIndex()), vp->comp(), allowForAOT); | ||
if (classInfo && classInfo->getFieldInfo()) | ||
{ | ||
TR_PersistentFieldInfo * fieldInfo = classInfo->getFieldInfo()->findFieldInfo(vp->comp(), node, false); | ||
|
@@ -2366,6 +2366,7 @@ TR::Node *constrainIaload(OMR::ValuePropagation *vp, TR::Node *node) | |
sym->isFinal())) | ||
haveClassLookaheadInfo = true; | ||
|
||
bool allowForAOT = vp->comp()->getOption(TR_UseSymbolValidationManager); | ||
if (haveClassLookaheadInfo) | ||
{ | ||
if (sym->isStatic() && sym->isFinal()) | ||
|
@@ -2375,13 +2376,12 @@ TR::Node *constrainIaload(OMR::ValuePropagation *vp, TR::Node *node) | |
TR_OpaqueClassBlock * classOfStatic = symRef->getOwningMethod(vp->comp())->classOfStatic(symRef->getCPIndex()); | ||
bool isClassInitialized = false; | ||
TR_PersistentClassInfo * classInfo = | ||
vp->comp()->getPersistentInfo()->getPersistentCHTable()->findClassInfoAfterLocking(classOfStatic, vp->comp()); | ||
vp->comp()->getPersistentInfo()->getPersistentCHTable()->findClassInfoAfterLocking(classOfStatic, vp->comp(), allowForAOT); | ||
if (classInfo && classInfo->isInitialized()) | ||
isClassInitialized = true; | ||
|
||
if ((classOfStatic != vp->comp()->getSystemClassPointer() && | ||
isClassInitialized && | ||
!vp->comp()->getOption(TR_AOT) && | ||
(type == TR::Address))) | ||
{ | ||
TR::VMAccessCriticalSection constrainIaloadCriticalSection(vp->comp(), | ||
|
@@ -4870,6 +4870,9 @@ static void devirtualizeCall(OMR::ValuePropagation *vp, TR::Node *node) | |
return; | ||
} | ||
|
||
if (!vp->comp()->fe()->canDevirtualizeDispatch()) | ||
return; | ||
|
||
int32_t firstArgIndex = node->getFirstArgumentIndex(); | ||
bool isGlobal; | ||
TR::VPConstraint *constraint = vp->getConstraint(node->getChild(firstArgIndex), isGlobal); | ||
|
@@ -9463,7 +9466,8 @@ static TR::Node *constrainIfcmpeqne(OMR::ValuePropagation *vp, TR::Node *node, b | |
} | ||
|
||
#ifdef J9_PROJECT_SPECIFIC | ||
if (!vp->comp()->compileRelocatableCode() && | ||
|
||
if ((!vp->comp()->compileRelocatableCode() || vp->comp()->getOption(TR_UseSymbolValidationManager)) && | ||
vp->lastTimeThrough() && | ||
vp->comp()->performVirtualGuardNOPing() && | ||
!vp->_curBlock->isCold() && | ||
|
@@ -9487,12 +9491,13 @@ static TR::Node *constrainIfcmpeqne(OMR::ValuePropagation *vp, TR::Node *node, b | |
if (typeConstraint) | ||
{ | ||
TR::VPConstraint *resolvedTypeConstraint = typeConstraint->asResolvedClass(); | ||
bool allowForAOT = vp->comp()->getOption(TR_UseSymbolValidationManager); | ||
if (resolvedTypeConstraint) | ||
{ | ||
TR_OpaqueClassBlock *clazz = resolvedTypeConstraint->getClass(); | ||
|
||
TR_PersistentClassInfo * classInfo = | ||
vp->comp()->getPersistentInfo()->getPersistentCHTable()->findClassInfoAfterLocking(clazz, vp->comp()); | ||
vp->comp()->getPersistentInfo()->getPersistentCHTable()->findClassInfoAfterLocking(clazz, vp->comp(), allowForAOT); | ||
|
||
if (vp->trace()) | ||
traceMsg(vp->comp(), "MyDebug: clazz %p classInfo %p classInfo->isInitialized() %d\n", | ||
|
@@ -9525,7 +9530,7 @@ static TR::Node *constrainIfcmpeqne(OMR::ValuePropagation *vp, TR::Node *node, b | |
else | ||
{ | ||
TR_PersistentClassInfo * classInfo = | ||
vp->comp()->getPersistentInfo()->getPersistentCHTable()->findClassInfoAfterLocking(clazz, vp->comp()); | ||
vp->comp()->getPersistentInfo()->getPersistentCHTable()->findClassInfoAfterLocking(clazz, vp->comp(), allowForAOT); | ||
|
||
if (vp->trace()) | ||
traceMsg(vp->comp(), "MyDebug: clazz %p classInfo %p classInfo->isInitialized() %d\n", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...makes me wonder if
ForHeuristics
(or its other state) is a concept worth promoting more broadly...For example, the
isUnresolved()
change you had to make in another commit...