From b06541552da888d31a748eefefd6045e62704cfc Mon Sep 17 00:00:00 2001 From: Irwin D'Souza Date: Mon, 8 Jan 2024 14:13:41 -0500 Subject: [PATCH] Ignore adjustment heuristics if size in weighCallSite is negative The estimated code size of methods can be negative; this is to indicate that the method is a good candidate for inlinling. However, the logic in weighCallSite, which contains several heuristics that adjust the size of a candidate method, used a variable of type uint32_t. The first issue is that the code that compares the size of a method to the inlining budget results in promoting the inlining budge value from a signed 32-bit integer to an unsigned 32-bit integer, because the variable that represents the size of the candidate method is an unsigned 32-bit integer. This means that any negative size, which is supposed to represent a good candidate method, will actually cause the inliner to make the determination that it is a very bad candidate method. The second issue is that even if if the comparison was fixed to cast the size variable to be a signed 32-bit integer, the heuristics prior to that point that adjust the size can result in an overflow that now leaves the size variable with a very large signed positive number, which again, causes the inliner to make the determination that the candidate method is very bad for inlining. This commit fixes both issues by 1. Changing the size variable to be of type int32_t 2. Only allowing the heuristics to adjust the size if it is positive. Signed-off-by: Irwin D'Souza --- .../compiler/optimizer/InlinerTempForJ9.cpp | 94 ++++++++++--------- 1 file changed, 51 insertions(+), 43 deletions(-) diff --git a/runtime/compiler/optimizer/InlinerTempForJ9.cpp b/runtime/compiler/optimizer/InlinerTempForJ9.cpp index b84ac35144e..d664d891adc 100644 --- a/runtime/compiler/optimizer/InlinerTempForJ9.cpp +++ b/runtime/compiler/optimizer/InlinerTempForJ9.cpp @@ -3354,7 +3354,7 @@ void TR_MultipleCallTargetInliner::weighCallSite( TR_CallStack * callStack , TR_ for (int32_t k = 0; k < callsite->numTargets(); k++) { - uint32_t size = 0; + int32_t size = 0; TR_EstimateCodeSize::raiiWrapper ecsWrapper(this, tracer(), _maxRecursiveCallByteCodeSizeEstimate); TR_EstimateCodeSize *ecs = ecsWrapper.getCodeEstimator(); @@ -3452,7 +3452,7 @@ void TR_MultipleCallTargetInliner::weighCallSite( TR_CallStack * callStack , TR_ heuristicTrace(tracer(),"WeighCallSite: For Target %p node %p signature %s, estimation returned a size of %d", calltarget,calltarget->_myCallSite->_callNode,tracer()->traceSignature(calltarget),size); - if (currentBlockHasExceptionSuccessors && ecs->aggressivelyInlineThrows()) + if (currentBlockHasExceptionSuccessors && ecs->aggressivelyInlineThrows() && size > 0) { _maxRecursiveCallByteCodeSizeEstimate >>= 3; size >>= 3; @@ -3469,7 +3469,7 @@ void TR_MultipleCallTargetInliner::weighCallSite( TR_CallStack * callStack , TR_ (size > _maxRecursiveCallByteCodeSizeEstimate/2)) possiblyVeryHotLargeCallee = true; - if (calltarget->_calleeSymbol->isSynchronised()) + if (calltarget->_calleeSymbol->isSynchronised() && size > 0) { size >>= 1; // could help gvp heuristicTrace(tracer(),"Setting size to %d because call is Synchronized",size); @@ -3481,13 +3481,13 @@ void TR_MultipleCallTargetInliner::weighCallSite( TR_CallStack * callStack , TR_ wouldBenefitFromInlining = true; } - if (strstr(calltarget->_calleeSymbol->signature(trMemory()),"BigDecimal.add(")) + if (strstr(calltarget->_calleeSymbol->signature(trMemory()),"BigDecimal.add(") && size > 0) { size >>=2; heuristicTrace(tracer(),"Setting size to %d because call is BigDecimal.add",size); } - if (isHot(comp())) + if (isHot(comp()) && size > 0) { TR_ResolvedMethod *m = calltarget->_calleeSymbol->getResolvedMethod(); const char *sig = "toString"; @@ -3587,53 +3587,61 @@ void TR_MultipleCallTargetInliner::weighCallSite( TR_CallStack * callStack , TR_ if (comp()->trace(OMR::inlining)) heuristicTrace(tracer(),"WeighCallSite: Considering shrinking call %p with frequency %d\n", callsite->_callNode, frequency2); - bool largeCompiledCallee = !comp()->getOption(TR_InlineVeryLargeCompiledMethods) && - isLargeCompiledMethod(calltarget->_calleeMethod, size, frequency2); - if (largeCompiledCallee) - { - size = size*TR::Options::_inlinerVeryLargeCompiledMethodAdjustFactor; - } - else if (frequency2 > borderFrequency) + if (size > 0) { - float factor = (float)(maxFrequency-frequency2)/(float)maxFrequency; - factor = std::max(factor, 0.4f); + bool largeCompiledCallee = !comp()->getOption(TR_InlineVeryLargeCompiledMethods) && + isLargeCompiledMethod(calltarget->_calleeMethod, size, frequency2); + if (largeCompiledCallee) + { + size = size*TR::Options::_inlinerVeryLargeCompiledMethodAdjustFactor; + } + else if (frequency2 > borderFrequency) + { + float factor = (float)(maxFrequency-frequency2)/(float)maxFrequency; + factor = std::max(factor, 0.4f); - float avgMethodSize = (float)size/(float)ecs->getNumOfEstimatedCalls(); - float numCallsFactor = (float)(avgMethodSize)/110.0f; + float avgMethodSize = (float)size/(float)ecs->getNumOfEstimatedCalls(); + float numCallsFactor = (float)(avgMethodSize)/110.0f; - numCallsFactor = std::max(factor, 0.1f); + numCallsFactor = std::max(factor, 0.1f); - if (size > 100) - { - size = (int)((float)size * factor * numCallsFactor); - if (size < 100) size = 100; + if (size > 100) + { + size = (int)((float)size * factor * numCallsFactor); + if (size < 100) size = 100; + } + else + { + size = (int)((float)size * factor * numCallsFactor); + } + if (comp()->trace(OMR::inlining)) + heuristicTrace(tracer(), "WeighCallSite: Adjusted call-graph size for call node %p, from %d to %d\n", callsite->_callNode, origSize, size); } - else + else if ((frequency2 > 0) && (frequency2 < veryColdBorderFrequency)) // luke-warm block { - size = (int)((float)size * factor * numCallsFactor); + float factor = (float)frequency2 / (float)maxFrequency; + //factor = std::max(factor, 0.1f); + size = (int)((float)size / (factor*factor)); // make the size look bigger to inline less + if (comp()->trace(OMR::inlining)) + heuristicTrace(tracer(), "WeighCallSite: Adjusted call-graph size for call node %p, from %d to %d\n", callsite->_callNode, origSize, size); } - if (comp()->trace(OMR::inlining)) - heuristicTrace(tracer(), "WeighCallSite: Adjusted call-graph size for call node %p, from %d to %d\n", callsite->_callNode, origSize, size); - } - else if ((frequency2 > 0) && (frequency2 < veryColdBorderFrequency)) // luke-warm block - { - float factor = (float)frequency2 / (float)maxFrequency; - //factor = std::max(factor, 0.1f); - size = (int)((float)size / (factor*factor)); // make the size look bigger to inline less - if (comp()->trace(OMR::inlining)) - heuristicTrace(tracer(), "WeighCallSite: Adjusted call-graph size for call node %p, from %d to %d\n", callsite->_callNode, origSize, size); - } - else if ((frequency2 >= 0) && (frequency2 < coldBorderFrequency)) // very cold block - { - //to avoid division by zero crash. Semantically freqs of 0 and 1 should be pretty close given maxFrequency of 10K - int adjFrequency2 = frequency2 ? frequency2 : 1; - float factor = (float)adjFrequency2 / (float)maxFrequency; - //factor = std::max(factor, 0.1f); - size = (int)((float)size / factor); + else if ((frequency2 >= 0) && (frequency2 < coldBorderFrequency)) // very cold block + { + //to avoid division by zero crash. Semantically freqs of 0 and 1 should be pretty close given maxFrequency of 10K + int adjFrequency2 = frequency2 ? frequency2 : 1; + float factor = (float)adjFrequency2 / (float)maxFrequency; + //factor = std::max(factor, 0.1f); + size = (int)((float)size / factor); - if (comp()->trace(OMR::inlining)) - heuristicTrace(tracer(),"WeighCallSite: Adjusted call-graph size for call node %p, from %d to %d\n", callsite->_callNode, origSize, size); + if (comp()->trace(OMR::inlining)) + heuristicTrace(tracer(),"WeighCallSite: Adjusted call-graph size for call node %p, from %d to %d\n", callsite->_callNode, origSize, size); + } + else + { + if (comp()->trace(OMR::inlining)) + heuristicTrace(tracer(),"WeighCallSite: Not adjusted call-graph size for call node %p, size %d\n", callsite->_callNode, origSize); + } } else {