Skip to content

Commit

Permalink
Move anchored instanceof to the correct position
Browse files Browse the repository at this point in the history
This commit includes the following two changes in ILGen:

1.
If the class is unresolved, `genInstanceof` anchors `instanceof`
under a `treetop` node. Then the anchored `instanceof` treetop
appears after the callTree and it commons with
the `instanceof` node under the `ZEROCHK` in `geninterface`.

This causes a problem because `expandUnresolvedClassInstanceof`
does not expect the treetop that has the anchored `instanceof`
to have already been evaluated when it appears under ZEROCHK.
The transformation in `expandUnresolvedClassInstanceof` will
not function correctly.

Therefore, the anchored `instanceof` treetop needs to be moved
up before ZEROCHK.

2.
If the receiver of the `instanceof` is non-null, there is no need
to transform the `instanceof` into two cases: the null case and
the non-null cases in `expandUnresolvedClassInstanceof`.

Fixes: #18748

Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
  • Loading branch information
a7ehuo committed Jun 6, 2024
1 parent b4dda4b commit c3f186f
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 1 deletion.
14 changes: 14 additions & 0 deletions runtime/compiler/ilgen/IlGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2652,6 +2652,20 @@ void TR_J9ByteCodeIlGenerator::expandUnresolvedClassInstanceof(TR::TreeTop *tree
TR_ASSERT(origClassNode->getSymbolReference()->isUnresolved(), "unresolved class instanceof n%un: expected symref of class child n%un to be unresolved\n", instanceofNode->getGlobalIndex(), origClassNode->getGlobalIndex());

bool trace = comp()->getOption(TR_TraceILGen);

// If the receiver of the instanceof is non-null, there is no need of the null case
if(instanceofNode->isReferenceNonNull() || objNode->isNonNull())
{
TR::Node *resolveCheckNode = genResolveCheck(origClassNode);
resolveCheckNode->copyByteCodeInfo(instanceofNode);
tree->insertBefore(TR::TreeTop::create(comp(), resolveCheckNode));

if (trace)
traceMsg(comp(), "%s: emit ResolveCHK n%dn before the unresolved class instanceof n%un in block_%d\n", __FUNCTION__,
resolveCheckNode->getGlobalIndex(), instanceofNode->getGlobalIndex(), tree->getEnclosingBlock()->getNumber());
return;
}

if (trace)
traceMsg(comp(), "expanding unresolved class instanceof n%un in block_%d\n", instanceofNode->getGlobalIndex(), tree->getEnclosingBlock()->getNumber());

Expand Down
61 changes: 60 additions & 1 deletion runtime/compiler/ilgen/Walker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3048,6 +3048,15 @@ TR_J9ByteCodeIlGenerator::genInvokeInterface(int32_t cpIndex)
}
else
{
/*
* The sequence of the nodes being generated below matters for the following reasons:
* - If OSR generates the call, it can also generate some pending push stores,
* and especially for involuntary OSR. The ZEROCHK needs to be between the
* pending push stores and the call
*
* - The generated instanceof depends on the NULLCHK on the receiver generated through
* the call node
*/
_methodSymbol->setMayHaveInlineableCall(true);
TR::TreeTop *prevLastTree = _block->getExit()->getPrevTreeTop();
TR::Node *callNode = NULL;
Expand Down Expand Up @@ -3092,13 +3101,63 @@ TR_J9ByteCodeIlGenerator::genInvokeInterface(int32_t cpIndex)
genInstanceof(interfaceCPIndex);
TR::Node *instanceof = pop();

// The receiver should not be NULL at this point because there should already be a NULLCHK
// before the call or there is no need of NULLCHK
instanceof->setReferenceIsNonNull(true);

TR::SymbolReference *icce =
symRefTab()->findOrCreateIncompatibleClassChangeErrorSymbolRef(_methodSymbol);

TR::Node *check =
TR::Node::createWithSymRef(TR::ZEROCHK, 1, 1, instanceof, icce);

callTree->insertBefore(TR::TreeTop::create(comp(), check));
TR::TreeTop *zeroCHKTT = callTree->insertBefore(TR::TreeTop::create(comp(), check));

/*
* If the class is unresolved, genInstanceof anchors instanceof under a treetop node.
* Then the anchored instanceof treetop appears after the callTree and it commons with
* the previous instanceof node under the ZEROCHK.
* This causes a problem because expandUnresolvedClassInstanceof does not expect the
* treetop that has the anchored instanceof to have already been evaluated when it appears
* under ZEROCHK. The transformation in expandUnresolvedClassInstanceof will not function correctly.
* Therefore, the anchored instanceof treetop needs to be moved up before ZEROCHK.
*
* preTT
* ZEROCHKTT
* instanceof
* callTree
* treetop
* ==>instanceof
* nextTT
*
* ||
* ||
* \/
*
* preTT
* treetop
* instanceof
* ZEROCHKTT
* ==>instanceof
* callTree
* nextTT
*
*/
TR::TreeTop *instanceOfTT = callTree->getNextTreeTop();
if (instanceOfTT &&
instanceOfTT->getNode()->getOpCodeValue() == TR::treetop &&
instanceOfTT->getNode()->getFirstChild() &&
instanceOfTT->getNode()->getFirstChild() == instanceof)
{
callTree->join(instanceOfTT->getNextTreeTop());

zeroCHKTT->insertBefore(instanceOfTT);

if (comp()->getOption(TR_TraceILGen))
{
traceMsg(comp(), "%s: move the anchored instanceof n%dn before ZEROCHK n%dn\n", __FUNCTION__, instanceOfTT->getNode()->getGlobalIndex(), zeroCHKTT->getNode()->getGlobalIndex());
}
}
}
}

Expand Down

0 comments on commit c3f186f

Please sign in to comment.