Skip to content

Commit

Permalink
Inliner: fix assert for multiple failure reasons when inlining
Browse files Browse the repository at this point in the history 
Change dotnet/coreclr#3436 added asserts that checked that the LegacyPolicy would not
see multiple failing observations when inlining because we expect the
inliner to fail fast when it hits the first bad observation.

This change adds a test case that shows one example where we could get
two failing observations, along with code changes to fix the issue.

When an inlinee has invalid IL, an exception is raised and caught in
`jitNativeCode`. The catch there sets the inline result to the never
state. A failure code is propagated back to `fgInvokeInlineeCompiler`
which redundantly sets the inline result to a failing state with a
generic "something bad happened" observation. This second observation
triggers an assert since the jit is inlining and the result is already
known to be a failure.

The fix is to check in `fgInvokeInlineeCompiler` if the inline result
is already known to be in a failing state, and if so, forgo the new
observation.


Commit migrated from dotnet/coreclr@eec9185
  • Loading branch information
@AndyAyersMS
AndyAyersMS committed Mar 2, 2016
1 parent 51d2bf1 commit 48d47ca
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 2 deletions.
17 changes: 15 additions & 2 deletions src/coreclr/src/jit/flowgraph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21949,7 +21949,14 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call,

if (result != CORJIT_OK)
{
pParam->inlineInfo->inlineResult->noteFatal(InlineObservation::CALLSITE_COMPILATION_FAILURE);
// If we haven't yet determined why this inline fails, use
// a catch-all something bad happened observation.
InlineResult* innerInlineResult = pParam->inlineInfo->inlineResult;

if (!innerInlineResult->isFailure())
{
innerInlineResult->noteFatal(InlineObservation::CALLSITE_COMPILATION_FAILURE);
}
}
}
}
Expand All @@ -21962,7 +21969,13 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call,
eeGetMethodFullName(fncHandle));
}
#endif // DEBUG
inlineResult->noteFatal(InlineObservation::CALLSITE_COMPILATION_ERROR);

// If we haven't yet determined why this inline fails, use
// a catch-all something bad happened observation.
if (!inlineResult->isFailure())
{
inlineResult->noteFatal(InlineObservation::CALLSITE_COMPILATION_ERROR);
}
}
endErrorTrap();

Expand Down
27 changes: 27 additions & 0 deletions src/coreclr/tests/src/JIT/opt/Inline/regression/badcallee/app.config
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?xml version="1.0" encoding="utf-8"?>
<configuration>
<runtime>
<assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
<dependentAssembly>
<assemblyIdentity name="System.Runtime" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
<bindingRedirect oldVersion="0.0.0.0-4.0.20.0" newVersion="4.0.20.0" />
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="System.Text.Encoding" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
<bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="System.Threading.Tasks" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
<bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="System.IO" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
<bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="System.Reflection" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
<bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
</dependentAssembly>
</assemblyBinding>
</runtime>
</configuration>
52 changes: 52 additions & 0 deletions src/coreclr/tests/src/JIT/opt/Inline/regression/badcallee/badcallee.il
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// 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.

.assembly extern mscorlib
{
.publickeytoken = (B7 7A 5C 56 19 34 E0 89 )
.ver 4:0:0:0
}

.assembly extern System.Console
{
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A )
.ver 4:0:0:0
}

.assembly BadCallee
{
}

.module BadCallee.exe

.class public auto ansi beforefieldinit P
extends [mscorlib]System.Object
{
.method public hidebysig static int32
A1() cil managed aggressiveinlining
{
// Code size 55 (0x37)
.maxstack 2
ldc.i4.0
br.s 100
ret
} // end of method P::A1

.method public hidebysig static int32 Main(string[] x) cil managed
{
.entrypoint
ldarg.0
ldlen
dup
xor
ldc.i4.0
ceq
brtrue.s OK
call int32 P::A1()
pop
OK: ldc.i4.s 100
ret
} // end of method P::Main

} // end of class P
43 changes: 43 additions & 0 deletions src/coreclr/tests/src/JIT/opt/Inline/regression/badcallee/badcallee.ilproj
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
<PropertyGroup>
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
<AssemblyName>$(MSBuildProjectName)</AssemblyName>
<SchemaVersion>2.0</SchemaVersion>
<ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
<OutputType>Exe</OutputType>
<AppDesignerFolder>Properties</AppDesignerFolder>
<FileAlignment>512</FileAlignment>
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
<ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages</ReferencePath>
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
<NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
</PropertyGroup>
<!-- Default configurations to help VS understand the configurations -->
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
</PropertyGroup>
<ItemGroup>
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
<Visible>False</Visible>
</CodeAnalysisDependentAssemblyPaths>
</ItemGroup>
<PropertyGroup>
<DebugType>PdbOnly</DebugType>
</PropertyGroup>
<ItemGroup>
<Compile Include="badcallee.il" />
</ItemGroup>
<ItemGroup>
<None Include="app.config" />
</ItemGroup>
<ItemGroup>
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
</ItemGroup>
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
</PropertyGroup>
</Project>

0 comments on commit 48d47ca

Please sign in to comment.