Merge pull request #19655 from a7ehuo/fix-ilgen-instanceof-3-release-46

(0.46) Move anchored instanceof to the correct position
eclipse-openj9 · Jun 8, 2024 · da980bf · da980bf
2 parents 618a1d6 + c3f186f
commit da980bf
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 1 deletion.
diff --git 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
@@ -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());
+            }
+         }
       }
    }