Merge pull request #8722 from cathyzhyi/v0.19.0-release

(0.19.0) Fix method handle inlining and small improvements
eclipse-openj9 · Mar 2, 2020 · 07afcbd · 07afcbd
2 parents 7852213 + ad5d190
commit 07afcbd
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 22 deletions.
diff --git a/runtime/compiler/optimizer/EstimateCodeSize.hpp b/runtime/compiler/optimizer/EstimateCodeSize.hpp
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2019 IBM Corp. and others
+ * Copyright (c) 2000, 2020 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -99,7 +99,13 @@ class TR_EstimateCodeSize
    virtual bool estimateCodeSize(TR_CallTarget *, TR_CallStack * , bool recurseDown = true) = 0;
 
 
-   bool returnCleanup(int32_t);      // common tasks requiring completion before returning from estimation
+   /*
+    *  \brief common tasks requiring completion before returning from estimation
+    *
+    *  \param errorNumber
+    *       an unique number used to identify where estimate code size bailed out
+    */
+   bool returnCleanup(int32_t errorNumber );
 
    /* Fields */
 

diff --git a/runtime/compiler/optimizer/InterpreterEmulator.cpp b/runtime/compiler/optimizer/InterpreterEmulator.cpp
@@ -141,7 +141,7 @@ InterpreterEmulator::initializeIteratorWithState()
    this->setIndex(0);
    }
 
-void
+bool
 InterpreterEmulator::maintainStack(TR_J9ByteCode bc)
    {
    TR_ASSERT_FATAL(_iteratorWithState, "has to be called when the iterator has state!");
@@ -216,23 +216,42 @@ InterpreterEmulator::maintainStack(TR_J9ByteCode bc)
       case J9BCinvokestaticsplit:
          break;
       default:
-         TR_ASSERT_FATAL(0, "unexpected bytecode in thunk archetype %p at bcIndex %d %s (%d)\n", _calltarget, bcIndex(), comp()->fej9()->getByteCodeName(nextByte(0)), bc);
+         static const bool assertfatal = feGetEnv("TR_AssertFatalForUnexpectedBytecodeInMethodHandleThunk") ? true: false;
+         if (!assertfatal)
+            debugTrace(tracer(), "unexpected bytecode in thunk archetype %s (%p) at bcIndex %d %s (%d)\n", _calltarget->_calleeMethod->signature(comp()->trMemory()), _calltarget, bcIndex(), comp()->fej9()->getByteCodeName(nextByte(0)), bc);
+         else
+            TR_ASSERT_FATAL(0, "unexpected bytecode in thunk archetype %s (%p) at bcIndex %d %s (%d)\n", _calltarget->_calleeMethod->signature(comp()->trMemory()), _calltarget, bcIndex(), comp()->fej9()->getByteCodeName(nextByte(0)), bc);
+
+         TR::DebugCounter::incStaticDebugCounter(comp(),
+            TR::DebugCounter::debugCounterName(comp(),
+               "InterpreterEmulator.unexpectedBytecode/(root=%s)/(%s)/bc=%d/%s",
+               comp()->signature(),
+               _calltarget->_calleeMethod->signature(comp()->trMemory()),
+               _bcIndex,
+               comp()->fej9()->getByteCodeName(nextByte(0)))
+            );
+         return false;
       }
+   return true;
    }
 
 void
 InterpreterEmulator::maintainStackForAload(int slotIndex)
    {
    TR_ASSERT_FATAL(_iteratorWithState, "has to be called when the iterator has state!");
    TR_PrexArgInfo *argInfo = _calltarget->_ecsPrexArgInfo;
-   TR_PrexArgument *prexArgument = argInfo ? argInfo->get(slotIndex): NULL;
    TR_ASSERT_FATAL(argInfo, "thunk archetype target doesn't have _ecsPrexArgInfo %p\n", _calltarget);
-   if (prexArgument && TR_PrexArgument::knowledgeLevel(prexArgument) == KNOWN_OBJECT)
+   if (slotIndex < argInfo->getNumArgs())
       {
-      debugTrace(tracer(), "aload known obj%d from slot %d\n", prexArgument->getKnownObjectIndex(), slotIndex);
-      push(new (trStackMemory()) KnownObjOperand(prexArgument->getKnownObjectIndex()));
+      TR_PrexArgument *prexArgument = argInfo->get(slotIndex);
+      if (prexArgument && TR_PrexArgument::knowledgeLevel(prexArgument) == KNOWN_OBJECT)
+         {
+         debugTrace(tracer(), "aload known obj%d from slot %d\n", prexArgument->getKnownObjectIndex(), slotIndex);
+         push(new (trStackMemory()) KnownObjOperand(prexArgument->getKnownObjectIndex()));
+         return;
+         }
       }
-   else pushUnknownOperand();
+   pushUnknownOperand();
    }
 
 void
@@ -405,7 +424,7 @@ InterpreterEmulator::refineResolvedCalleeForInvokestatic(TR_ResolvedMethod *&cal
       }
    }
 
-void
+bool
 InterpreterEmulator::findAndCreateCallsitesFromBytecodes(bool wasPeekingSuccessfull, bool withState)
    {
    TR::Region findCallsitesRegion(comp()->region());
@@ -439,13 +458,16 @@ InterpreterEmulator::findAndCreateCallsitesFromBytecodes(bool wasPeekingSuccessf
 
       if (_iteratorWithState)
          {
-         maintainStack(bc);
-         dumpStack();
+         if (maintainStack(bc))
+            dumpStack();
+         else
+            return false;
          }
 
       _pca.updateArg(bc);
       bc = findNextByteCodeToVisit();
       }
+   return true;
    }
 
 TR_J9ByteCode

diff --git a/runtime/compiler/optimizer/InterpreterEmulator.hpp b/runtime/compiler/optimizer/InterpreterEmulator.hpp
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2019 IBM Corp. and others
+ * Copyright (c) 2000, 2020 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -200,8 +200,10 @@ class InterpreterEmulator : public TR_ByteCodeIteratorWithState<TR_J9ByteCode, J
        *
        * \param withState
        *       whether the bytecode iteration should be with or without state
+       *
+       * \return whether callsites are created successfully. Return false if failed for reasons like unexpected bytecodes etc.
        */
-      void findAndCreateCallsitesFromBytecodes(bool wasPeekingSuccessfull, bool withState);
+      bool findAndCreateCallsitesFromBytecodes(bool wasPeekingSuccessfull, bool withState);
       void setBlocks(TR::Block **blocks) { _blocks = blocks; }
       TR_StackMemory trStackMemory()            { return _trMemory; }
       bool _nonColdCallExists;
@@ -224,8 +226,10 @@ class InterpreterEmulator : public TR_ByteCodeIteratorWithState<TR_J9ByteCode, J
       void initializeIteratorWithState();
       /*
        * push and pop operands on stack according to given bytecode
+       *
+       * \return false if some error occurred such as unexpected bytecodes.
        */
-      void maintainStack(TR_J9ByteCode bc);
+      bool maintainStack(TR_J9ByteCode bc);
       void maintainStackForIf(TR_J9ByteCode bc);
       void maintainStackForGetField();
       void maintainStackForAload(int slotIndex);

diff --git a/runtime/compiler/optimizer/J9EstimateCodeSize.cpp b/runtime/compiler/optimizer/J9EstimateCodeSize.cpp
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2019 IBM Corp. and others
+ * Copyright (c) 2000, 2020 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -1144,9 +1144,9 @@ TR_J9EstimateCodeSize::realEstimateCodeSize(TR_CallTarget *calltarget, TR_CallSt
 
    const static bool debugMHInlineWithOutPeeking = feGetEnv("TR_DebugMHInlineWithOutPeeking") ? true: false;
    bool mhInlineWithPeeking =  comp()->getOption(TR_DisableMHInlineWithoutPeeking);
-   bool isCalleeMethodHandleThunk = calltarget->_calleeMethod->convertToMethod()->isArchetypeSpecimen();
+   bool isCalleeMethodHandleThunkInFirstPass = calltarget->_calleeMethod->convertToMethod()->isArchetypeSpecimen() && _inliner->firstPass();
    if (nph.doPeeking() && recurseDown ||
-       isCalleeMethodHandleThunk && mhInlineWithPeeking)
+       isCalleeMethodHandleThunkInFirstPass && mhInlineWithPeeking)
       {
 
       heuristicTrace(tracer(), "*** Depth %d: ECS CSI -- needsPeeking is true for calltarget %p",
@@ -1160,7 +1160,7 @@ TR_J9EstimateCodeSize::realEstimateCodeSize(TR_CallTarget *calltarget, TR_CallSt
          wasPeekingSuccessfull = true;
          }
       }
-   else if (isCalleeMethodHandleThunk && !mhInlineWithPeeking && debugMHInlineWithOutPeeking)
+   else if (isCalleeMethodHandleThunkInFirstPass && !mhInlineWithPeeking && debugMHInlineWithOutPeeking)
       {
       traceMsg(comp(), "printing out trees and bytecodes through peeking because DebugMHInlineWithOutPeeking is on\n");
       methodSymbol->getResolvedMethod()->genMethodILForPeekingEvenUnderMethodRedefinition(methodSymbol, comp(), false, NULL);
@@ -1178,7 +1178,7 @@ TR_J9EstimateCodeSize::realEstimateCodeSize(TR_CallTarget *calltarget, TR_CallSt
    //
    TR_ValueProfileInfoManager * profileManager = TR_ValueProfileInfoManager::get(comp());
    bool callGraphEnabled = !comp()->getOption(TR_DisableCallGraphInlining);//profileManager->isCallGraphProfilingEnabled(comp());
-   if (!_inliner->firstPass() || isCalleeMethodHandleThunk)
+   if (!_inliner->firstPass() || isCalleeMethodHandleThunkInFirstPass)
       callGraphEnabled = false; // TODO: Work out why this doesn't function properly on subsequent passes
    if (callGraphEnabled && recurseDown)
       {
@@ -1276,8 +1276,12 @@ TR_J9EstimateCodeSize::realEstimateCodeSize(TR_CallTarget *calltarget, TR_CallSt
    if (!callsitesAreCreatedFromTrees)
       {
       bci.prepareToFindAndCreateCallsites(blocks, flags, callSites, &cfg, &newBCInfo, _recursionDepth, &callStack);
-      bool iteratorWithState = isCalleeMethodHandleThunk && !mhInlineWithPeeking;
-      bci.findAndCreateCallsitesFromBytecodes(wasPeekingSuccessfull, iteratorWithState);
+      bool iteratorWithState = isCalleeMethodHandleThunkInFirstPass && !mhInlineWithPeeking;
+      if (!bci.findAndCreateCallsitesFromBytecodes(wasPeekingSuccessfull, iteratorWithState))
+         {
+         heuristicTrace(tracer(), "*** Depth %d: ECS end for target %p signature %s. bci.findAndCreateCallsitesFromBytecode failed", _recursionDepth, calltarget, callerName);
+         return returnCleanup(7);
+         }
       _hasNonColdCalls = bci._nonColdCallExists;
       }