From c3f186f96b8b1210063f423f490b8c7508a5114a Mon Sep 17 00:00:00 2001 From: Annabelle Huo Date: Thu, 30 May 2024 15:32:27 -0400 Subject: [PATCH] Move anchored instanceof to the correct position 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 --- runtime/compiler/ilgen/IlGenerator.cpp | 14 ++++++ runtime/compiler/ilgen/Walker.cpp | 61 +++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/runtime/compiler/ilgen/IlGenerator.cpp b/runtime/compiler/ilgen/IlGenerator.cpp index 175896a81e1..0305caca197 100644 --- a/runtime/compiler/ilgen/IlGenerator.cpp +++ b/runtime/compiler/ilgen/IlGenerator.cpp @@ -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()); diff --git a/runtime/compiler/ilgen/Walker.cpp b/runtime/compiler/ilgen/Walker.cpp index d359a353e1c..7a714f3c0d2 100644 --- a/runtime/compiler/ilgen/Walker.cpp +++ b/runtime/compiler/ilgen/Walker.cpp @@ -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; @@ -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()); + } + } } }