diff --git a/src/coreclr/vm/rejit.cpp b/src/coreclr/vm/rejit.cpp index f0bdf78289f507..4f0b02c2416f75 100644 --- a/src/coreclr/vm/rejit.cpp +++ b/src/coreclr/vm/rejit.cpp @@ -548,7 +548,9 @@ HRESULT ReJitManager::UpdateActiveILVersions( if ((flags & COR_PRF_REJIT_BLOCK_INLINING) == COR_PRF_REJIT_BLOCK_INLINING) { - hr = UpdateNativeInlinerActiveILVersions(&mgrToCodeActivationBatch, pModule, rgMethodDefs[i], fIsRevert, flags); + _ASSERTE(!fIsRevert); + + hr = UpdateNativeInlinerActiveILVersions(&mgrToCodeActivationBatch, pModule, rgMethodDefs[i], flags); if (FAILED(hr)) { return hr; @@ -711,7 +713,6 @@ HRESULT ReJitManager::UpdateNativeInlinerActiveILVersions( SHash *pMgrToCodeActivationBatch, Module *pInlineeModule, mdMethodDef inlineeMethodDef, - BOOL fIsRevert, COR_PRF_REJIT_FLAGS flags) { CONTRACTL @@ -758,7 +759,7 @@ HRESULT ReJitManager::UpdateNativeInlinerActiveILVersions( } } - hr = UpdateActiveILVersion(pMgrToCodeActivationBatch, inliner.m_module, inliner.m_methodDef, fIsRevert, flags); + hr = UpdateActiveILVersion(pMgrToCodeActivationBatch, inliner.m_module, inliner.m_methodDef, FALSE /* fIsRevert */, flags); if (FAILED(hr)) { ReportReJITError(inliner.m_module, inliner.m_methodDef, NULL, hr); diff --git a/src/coreclr/vm/rejit.h b/src/coreclr/vm/rejit.h index a994f9d3455b5d..5dfb5697d12860 100644 --- a/src/coreclr/vm/rejit.h +++ b/src/coreclr/vm/rejit.h @@ -166,7 +166,6 @@ class ReJitManager SHash *pMgrToCodeActivationBatch, Module *pInlineeModule, mdMethodDef inlineeMethodDef, - BOOL fIsRevert, COR_PRF_REJIT_FLAGS flags); static HRESULT UpdateJitInlinerActiveILVersions( diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 5d91deeba59eb2..d6395f29fe18e3 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -570,9 +570,6 @@ https://github.com/dotnet/runtime/issues/108255 - - https://github.com/dotnet/runtime/issues/109310 - https://github.com/dotnet/runtime/issues/109311 diff --git a/src/tests/profiler/native/rejitprofiler/rejitprofiler.cpp b/src/tests/profiler/native/rejitprofiler/rejitprofiler.cpp index b93d3bc6926c9b..604d8cee30ceb4 100644 --- a/src/tests/profiler/native/rejitprofiler/rejitprofiler.cpp +++ b/src/tests/profiler/native/rejitprofiler/rejitprofiler.cpp @@ -95,13 +95,19 @@ HRESULT ReJITProfiler::Shutdown() _profInfo10 = nullptr; } - int expectedRejitCount = -1; + int expectedRejitCount; auto it = _inlinings.find(_targetFuncId); if (it != _inlinings.end()) { // The number of inliners are expected to ReJIT, plus the method itself expectedRejitCount = (int)((*it).second->size() + 1); } + else + { + // No inlinings happened, which can occur in composite R2R mode on some targets. + // This is fine as long as we rejitted the target method itself. + expectedRejitCount = 1; + } INFO(L" rejit count=" << _rejits << L" expected rejit count=" << expectedRejitCount); @@ -320,7 +326,8 @@ HRESULT STDMETHODCALLTYPE ReJITProfiler::GetReJITParameters(ModuleID moduleId, m { SHUTDOWNGUARD(); - INFO(L"Starting to build IL for method " << GetFunctionIDName(GetFunctionIDFromToken(moduleId, methodId, false))); + String functionName = GetFunctionIDName(GetFunctionIDFromToken(moduleId, methodId, false)); + INFO(L"Starting to build IL for method " << functionName); COMPtrHolder pUnk; HRESULT hr = _profInfo10->GetModuleMetaData(moduleId, ofWrite, IID_IMetaDataEmit2, &pUnk); if (FAILED(hr)) @@ -340,10 +347,18 @@ HRESULT STDMETHODCALLTYPE ReJITProfiler::GetReJITParameters(ModuleID moduleId, m } - const WCHAR *wszNewUserDefinedString = WCHAR("Hello from profiler rejit!"); + String newUserDefinedString = String(WCHAR("Hello from profiler rejit method '")); + newUserDefinedString += functionName; + newUserDefinedString += WCHAR("'! "); mdString tokmdsUserDefined = mdTokenNil; - hr = pTargetEmit->DefineUserString(wszNewUserDefinedString, - (ULONG)wcslen(wszNewUserDefinedString), + + // There's no portable way to convert a String to LPCWSTR so just make a manual copy on the stack. + char16_t buf[4096] = { 0 }; + for (size_t i = 0, c = newUserDefinedString.Length(); i < c; i++) + buf[i] = (char16_t)newUserDefinedString[i]; + + hr = pTargetEmit->DefineUserString((LPCWSTR)(void *)buf, + (ULONG)newUserDefinedString.Length(), &tokmdsUserDefined); if (FAILED(hr)) { @@ -427,28 +442,39 @@ HRESULT STDMETHODCALLTYPE ReJITProfiler::ReJITError(ModuleID moduleId, mdMethodD void ReJITProfiler::AddInlining(FunctionID inliner, FunctionID inlinee) { - shared_ptr> inliners; - auto result = _inlinings.find(inlinee); - if (result == _inlinings.end()) + String calleeName = GetFunctionIDName(inlinee); + String moduleName = GetModuleIDName(GetModuleIDForFunction(inlinee)); + + // Depending on various things it's possible the JIT will inline code during our test run that isn't part of the + // rejit test module. For example if part of the BCL didn't get crossgen'd or the crossgen'd code isn't used. + // We don't care about those inlinings, so we won't track them. This makes the test more reliable. + if (EndsWith(moduleName, String(WCHAR("rejit.dll")))) { - auto p = make_pair(inlinee, make_shared>()); - inliners = p.second; - _inlinings.insert(p); + shared_ptr> inliners; + auto result = _inlinings.find(inlinee); + if (result == _inlinings.end()) + { + auto p = make_pair(inlinee, make_shared>()); + inliners = p.second; + _inlinings.insert(p); + } + else + { + inliners = (*result).second; + } + + auto it = inliners->find(inliner); + if (it == inliners->end()) + { + inliners->insert(inliner); + } + + INFO(L"Inlining in test module! Inliner=" << GetFunctionIDName(inliner) << L" Inlinee=" << calleeName << L" module=" << moduleName); } else { - inliners = (*result).second; + INFO(L"Inlining in non-test module! Inliner=" << GetFunctionIDName(inliner) << L" Inlinee=" << calleeName << L" module=" << moduleName); } - - auto it = inliners->find(inliner); - if (it == inliners->end()) - { - inliners->insert(inliner); - } - - String calleeName = GetFunctionIDName(inlinee); - String moduleName = GetModuleIDName(GetModuleIDForFunction(inlinee)); - INFO(L"Inlining occurred! Inliner=" << GetFunctionIDName(inliner) << L" Inlinee=" << calleeName << L" Inlinee module name=" << moduleName); } FunctionID ReJITProfiler::GetFunctionIDFromToken(ModuleID module, mdMethodDef token, bool invalidArgNotFailure) diff --git a/src/tests/profiler/rejit/rejit.cs b/src/tests/profiler/rejit/rejit.cs index b9ea65ffccde8e..fc69720d270786 100644 --- a/src/tests/profiler/rejit/rejit.cs +++ b/src/tests/profiler/rejit/rejit.cs @@ -10,6 +10,15 @@ class RejitWithInlining //: ProfilerTest { static readonly Guid ReJitProfilerGuid = new Guid("66F7A9DF-8858-4A32-9CFF-3AD0787E0186"); + static System.Text.StringBuilder OutputBuilder = new (); + + [MethodImplAttribute(MethodImplOptions.NoInlining)] + public static void TestWriteLine(string s) + { + OutputBuilder.AppendLine(s); + Console.WriteLine(s); + } + [MethodImplAttribute(MethodImplOptions.NoInlining)] private static int MaxInlining() { @@ -20,18 +29,41 @@ private static int MaxInlining() TriggerReJIT(); + OutputBuilder.Clear(); + // TriggerInliningChain triggers a ReJIT, now this time we should call // in to the ReJITted code TriggerDirectInlining(); CallMethodWithoutInlining(); TriggerInliningChain(); + string matchString = "Hello from profiler rejit method 'InlineeTarget'!"; + int numRejittedTargets = OutputBuilder.ToString().Split(matchString).Length; + if (numRejittedTargets != 4) + { + Console.WriteLine("ReJIT did not update all instances of InlineeTarget!"); + return 1234; + } + TriggerRevert(); + OutputBuilder.Clear(); + TriggerDirectInlining(); CallMethodWithoutInlining(); TriggerInliningChain(); + if (OutputBuilder.ToString().Contains(matchString)) + { + Console.WriteLine("ReJIT revert of InlineeTarget was not complete!"); + return 1235; + } + + // Currently there will still be some 'Hello from profiler rejit method' messages left + // in the output in certain configurations. This is because the test profiler does not revert all + // the methods that it modified - reverts are not symmetric with rejits. + // See https://github.com/dotnet/runtime/issues/117823 + return 100; } @@ -50,7 +82,7 @@ private static void TriggerRevert() [MethodImplAttribute(MethodImplOptions.NoInlining)] private static void TriggerInliningChain() { - Console.WriteLine("TriggerInlining"); + TestWriteLine("TriggerInliningChain"); // Test Inlining through another method InlineeChain1(); } @@ -58,34 +90,40 @@ private static void TriggerInliningChain() [MethodImplAttribute(MethodImplOptions.NoInlining)] private static void TriggerDirectInlining() { - Console.WriteLine("TriggerDirectInlining"); + TestWriteLine("TriggerDirectInlining"); InlineeTarget(); } [MethodImplAttribute(MethodImplOptions.NoInlining)] private static void CallMethodWithoutInlining() { - Console.WriteLine("CallMethodWithoutInlining"); - Action callMethod = InlineeTarget; - callMethod(); + TestWriteLine("CallMethodWithoutInlining"); + Action callMethod = InlineeTarget; + callMethod("CallMethodWithoutInlining"); } [MethodImplAttribute(MethodImplOptions.AggressiveInlining)] private static void InlineeChain1() { - Console.WriteLine("Inline.InlineeChain1"); + TestWriteLine(" Inline.InlineeChain1"); InlineeTarget(); } [MethodImplAttribute(MethodImplOptions.AggressiveInlining)] - public static void InlineeTarget() + public static void InlineeTarget([CallerMemberName] string callerMemberName = null) { - Console.WriteLine("Inline.InlineeTarget"); + var sb = new System.Text.StringBuilder(); + sb.Append(" Inline.InlineeTarget"); + sb.Append(' '); + sb.Append('('); + sb.Append(callerMemberName); + sb.Append(')'); + TestWriteLine(sb.ToString()); } public static int RunTest(string[] args) { - Console.WriteLine("maxinlining"); + TestWriteLine("maxinlining"); return MaxInlining(); } @@ -108,7 +146,7 @@ public class SeparateClassNeverLoaded [MethodImplAttribute(MethodImplOptions.NoInlining)] private static void TriggerInliningChain() { - Console.WriteLine("TriggerInlining"); + RejitWithInlining.TestWriteLine("TriggerInlining"); // Test Inlining through another method InlineeChain1(); } @@ -116,14 +154,14 @@ private static void TriggerInliningChain() [MethodImplAttribute(MethodImplOptions.NoInlining)] private static void TriggerDirectInlining() { - Console.WriteLine("TriggerDirectInlining"); + RejitWithInlining.TestWriteLine("TriggerDirectInlining"); RejitWithInlining.InlineeTarget(); } [MethodImplAttribute(MethodImplOptions.AggressiveInlining)] private static void InlineeChain1() { - Console.WriteLine("Inline.InlineeChain1"); + RejitWithInlining.TestWriteLine("Inline.InlineeChain1"); RejitWithInlining.InlineeTarget(); } }