Skip to content

Commit

Permalink
When QuickJit is disabled, fix assertion failures
Browse files Browse the repository at this point in the history
- When QuickJit is disabled, the initial tier is Optimized instead of the correct Tier0. This causes assertion failures as tiering tries to count calls and promote the method to Tier1.
  - Does not appear to be an issue in release builds, as the methods are still call-counted and promoted despite the incorrect tier
- Add some basic tiering tests for config modes that are exposed and supported through <app>.runtimeconfig.json, QuickJit and QuickJitForLoops, when on and off
- Removed an invalid and redundant assertion that was causing a profiler rejit test to fail, see dotnet#33492 (comment). What the assertion was intending to verify is already verified by an assertion above it that checks the tier, which also covers the default native code version case.
  • Loading branch information
kouvel committed Mar 12, 2020
1 parent f30ea37 commit 72f909e
Show file tree
Hide file tree
Showing 13 changed files with 267 additions and 11 deletions.
6 changes: 0 additions & 6 deletions src/coreclr/src/vm/prestub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1048,12 +1048,6 @@ PCODE MethodDesc::JitCompileCodeLocked(PrepareCodeConfig* pConfig, JitListLockEn
{
_ASSERTE(pConfig->GetCodeVersion().GetOptimizationTier() == NativeCodeVersion::OptimizationTier0);
_ASSERTE(pConfig->GetMethodDesc()->IsEligibleForTieredCompilation());
_ASSERTE(
pConfig
->GetMethodDesc()
->GetLoaderAllocator()
->GetCallCountingManager()
->IsCallCountingEnabled(pConfig->GetCodeVersion()));

if (pConfig->JitSwitchedToOptimized())
{
Expand Down
15 changes: 10 additions & 5 deletions src/coreclr/src/vm/tieredcompilation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,7 @@ NativeCodeVersion::OptimizationTier TieredCompilationManager::GetInitialOptimiza
return NativeCodeVersion::OptimizationTier1;
}

if (!g_pConfig->TieredCompilation_QuickJit() ||
!pMethodDesc->GetLoaderAllocator()->GetCallCountingManager()->IsCallCountingEnabled(NativeCodeVersion(pMethodDesc)))
if (!pMethodDesc->GetLoaderAllocator()->GetCallCountingManager()->IsCallCountingEnabled(NativeCodeVersion(pMethodDesc)))
{
// Tier 0 call counting may have been disabled for several reasons, the intention is to start with and stay at an
// optimized tier
Expand Down Expand Up @@ -920,15 +919,21 @@ CORJIT_FLAGS TieredCompilationManager::GetJitFlags(NativeCodeVersion nativeCodeV
switch (nativeCodeVersion.GetOptimizationTier())
{
case NativeCodeVersion::OptimizationTier0:
_ASSERTE(g_pConfig->TieredCompilation_QuickJit());
flags.Set(CORJIT_FLAGS::CORJIT_FLAG_TIER0);
break;
if (g_pConfig->TieredCompilation_QuickJit())
{
flags.Set(CORJIT_FLAGS::CORJIT_FLAG_TIER0);
break;
}

nativeCodeVersion.SetOptimizationTier(NativeCodeVersion::OptimizationTierOptimized);
goto Optimized;

case NativeCodeVersion::OptimizationTier1:
flags.Set(CORJIT_FLAGS::CORJIT_FLAG_TIER1);
// fall through

case NativeCodeVersion::OptimizationTierOptimized:
Optimized:
#ifdef FEATURE_INTERPRETER
flags.Set(CORJIT_FLAGS::CORJIT_FLAG_MAKEFINALCODE);
#endif
Expand Down
49 changes: 49 additions & 0 deletions src/coreclr/tests/src/baseservices/TieredCompilation/BasicTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Runtime.CompilerServices;
using System.Threading;

public static class BasicTest
{
private static int Main()
{
const int Pass = 100;

PromoteToTier1(Foo);
Foo();

return Pass;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void Foo()
{
Foo2();
}

private static void Foo2()
{
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void PromoteToTier1(Action action)
{
// Call the method once to register a call for call counting
action();

// Allow time for call counting to begin
Thread.Sleep(500);

// Call the method enough times to trigger tier 1 promotion
for (int i = 0; i < 100; i++)
{
action();
}

// Allow time for the method to be jitted at tier 1
Thread.Sleep(500);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Optimize>true</Optimize>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="BasicTest.cs" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Optimize>true</Optimize>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="BasicTest.cs" />
</ItemGroup>
<PropertyGroup>
<CLRTestBatchPreCommands><![CDATA[
$(CLRTestBatchPreCommands)
"%CORE_ROOT%\crossgen" -ReadyToRun -Platform_Assemblies_Paths "%CORE_ROOT%" -out $(MSBuildProjectName).ni.dll $(MSBuildProjectName).dll
]]></CLRTestBatchPreCommands>
<BashCLRTestPreCommands><![CDATA[
$(BashCLRTestPreCommands)
"$CORE_ROOT/crossgen" -ReadyToRun -Platform_Assemblies_Paths "$CORE_ROOT" -out $(MSBuildProjectName).ni.dll $(MSBuildProjectName).dll
]]></BashCLRTestPreCommands>
</PropertyGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Optimize>true</Optimize>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="BasicTest.cs" />
</ItemGroup>
<PropertyGroup>
<CLRTestBatchPreCommands><![CDATA[
$(CLRTestBatchPreCommands)
set COMPlus_TC_QuickJitForLoops=0
]]></CLRTestBatchPreCommands>
<BashCLRTestPreCommands><![CDATA[
$(BashCLRTestPreCommands)
export COMPlus_TC_QuickJitForLoops=0
]]></BashCLRTestPreCommands>
</PropertyGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Optimize>true</Optimize>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="BasicTest.cs" />
</ItemGroup>
<PropertyGroup>
<CLRTestBatchPreCommands><![CDATA[
$(CLRTestBatchPreCommands)
"%CORE_ROOT%\crossgen" -ReadyToRun -Platform_Assemblies_Paths "%CORE_ROOT%" -out $(MSBuildProjectName).ni.dll $(MSBuildProjectName).dll
set COMPlus_TC_QuickJitForLoops=0
]]></CLRTestBatchPreCommands>
<BashCLRTestPreCommands><![CDATA[
$(BashCLRTestPreCommands)
"$CORE_ROOT/crossgen" -ReadyToRun -Platform_Assemblies_Paths "$CORE_ROOT" -out $(MSBuildProjectName).ni.dll $(MSBuildProjectName).dll
export COMPlus_TC_QuickJitForLoops=0
]]></BashCLRTestPreCommands>
</PropertyGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Optimize>true</Optimize>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="BasicTest.cs" />
</ItemGroup>
<PropertyGroup>
<CLRTestBatchPreCommands><![CDATA[
$(CLRTestBatchPreCommands)
set COMPlus_TC_QuickJitForLoops=1
]]></CLRTestBatchPreCommands>
<BashCLRTestPreCommands><![CDATA[
$(BashCLRTestPreCommands)
export COMPlus_TC_QuickJitForLoops=1
]]></BashCLRTestPreCommands>
</PropertyGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Optimize>true</Optimize>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="BasicTest.cs" />
</ItemGroup>
<PropertyGroup>
<CLRTestBatchPreCommands><![CDATA[
$(CLRTestBatchPreCommands)
"%CORE_ROOT%\crossgen" -ReadyToRun -Platform_Assemblies_Paths "%CORE_ROOT%" -out $(MSBuildProjectName).ni.dll $(MSBuildProjectName).dll
set COMPlus_TC_QuickJitForLoops=1
]]></CLRTestBatchPreCommands>
<BashCLRTestPreCommands><![CDATA[
$(BashCLRTestPreCommands)
"$CORE_ROOT/crossgen" -ReadyToRun -Platform_Assemblies_Paths "$CORE_ROOT" -out $(MSBuildProjectName).ni.dll $(MSBuildProjectName).dll
export COMPlus_TC_QuickJitForLoops=1
]]></BashCLRTestPreCommands>
</PropertyGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Optimize>true</Optimize>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="BasicTest.cs" />
</ItemGroup>
<PropertyGroup>
<CLRTestBatchPreCommands><![CDATA[
$(CLRTestBatchPreCommands)
set COMPlus_TC_QuickJit=0
]]></CLRTestBatchPreCommands>
<BashCLRTestPreCommands><![CDATA[
$(BashCLRTestPreCommands)
export COMPlus_TC_QuickJit=0
]]></BashCLRTestPreCommands>
</PropertyGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Optimize>true</Optimize>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="BasicTest.cs" />
</ItemGroup>
<PropertyGroup>
<CLRTestBatchPreCommands><![CDATA[
$(CLRTestBatchPreCommands)
"%CORE_ROOT%\crossgen" -ReadyToRun -Platform_Assemblies_Paths "%CORE_ROOT%" -out $(MSBuildProjectName).ni.dll $(MSBuildProjectName).dll
set COMPlus_TC_QuickJit=0
]]></CLRTestBatchPreCommands>
<BashCLRTestPreCommands><![CDATA[
$(BashCLRTestPreCommands)
"$CORE_ROOT/crossgen" -ReadyToRun -Platform_Assemblies_Paths "$CORE_ROOT" -out $(MSBuildProjectName).ni.dll $(MSBuildProjectName).dll
export COMPlus_TC_QuickJit=0
]]></BashCLRTestPreCommands>
</PropertyGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Optimize>true</Optimize>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="BasicTest.cs" />
</ItemGroup>
<PropertyGroup>
<CLRTestBatchPreCommands><![CDATA[
$(CLRTestBatchPreCommands)
set COMPlus_TC_QuickJit=1
]]></CLRTestBatchPreCommands>
<BashCLRTestPreCommands><![CDATA[
$(BashCLRTestPreCommands)
export COMPlus_TC_QuickJit=1
]]></BashCLRTestPreCommands>
</PropertyGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Optimize>true</Optimize>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="BasicTest.cs" />
</ItemGroup>
<PropertyGroup>
<CLRTestBatchPreCommands><![CDATA[
$(CLRTestBatchPreCommands)
"%CORE_ROOT%\crossgen" -ReadyToRun -Platform_Assemblies_Paths "%CORE_ROOT%" -out $(MSBuildProjectName).ni.dll $(MSBuildProjectName).dll
set COMPlus_TC_QuickJit=1
]]></CLRTestBatchPreCommands>
<BashCLRTestPreCommands><![CDATA[
$(BashCLRTestPreCommands)
"$CORE_ROOT/crossgen" -ReadyToRun -Platform_Assemblies_Paths "$CORE_ROOT" -out $(MSBuildProjectName).ni.dll $(MSBuildProjectName).dll
export COMPlus_TC_QuickJit=1
]]></BashCLRTestPreCommands>
</PropertyGroup>
</Project>

0 comments on commit 72f909e

Please sign in to comment.