Skip to content

Encourage inlining of il throw test #114420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

21 changes: 21 additions & 0 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1258,6 +1258,23 @@ private CorInfoInline canInline(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHO
MethodDesc callerMethod = HandleToObject(callerHnd);
MethodDesc calleeMethod = HandleToObject(calleeHnd);

EcmaModule rootModule = (MethodBeingCompiled.OwningType as MetadataType)?.Module as EcmaModule;
EcmaModule calleeModule = (calleeMethod.OwningType as MetadataType)?.Module as EcmaModule;

// If this inline crosses module boundaries, ensure the modules agree on exception wrapping behavior.
if ((rootModule != calleeModule) && (rootModule != null) && (calleeModule != null))
{
if (rootModule.IsWrapNonExceptionThrows != calleeModule.IsWrapNonExceptionThrows)
{
var calleeIL = _compilation.GetMethodIL(calleeMethod);
if (calleeIL.GetExceptionRegions().Length != 0)
{
// Fail inlining if root method and callee have different exception wrapping behavior
return CorInfoInline.INLINE_FAIL;
}
}
}

if (_compilation.CanInline(callerMethod, calleeMethod))
{
// No restrictions on inlining
Expand All @@ -1266,6 +1283,10 @@ private CorInfoInline canInline(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHO
else
{
// Call may not be inlined
//
// Note despite returning INLINE_NEVER here, in compilations where jitting is possible
// the jit may still be able to inline this method. So we rely on reportInliningDecision
// to not propagate this into metadata to short-circuit future inlining attempts.
return CorInfoInline.INLINE_NEVER;
}
}
Expand Down
38 changes: 38 additions & 0 deletions src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ public partial class EcmaModule : ModuleDesc
{
private readonly PEReader _peReader;
protected readonly MetadataReader _metadataReader;
private volatile bool _isWrapNonExceptionThrowsKnown;
private volatile bool _isWrapNonExceptionThrows;

internal interface IEntityHandleObject
{
Expand Down Expand Up @@ -690,5 +692,41 @@ public override string ToString()
ModuleDefinition moduleDefinition = _metadataReader.GetModuleDefinition();
return _metadataReader.GetString(moduleDefinition.Name);
}

public bool IsWrapNonExceptionThrows
{
get
{
if (!_isWrapNonExceptionThrowsKnown)
{
var reader = MetadataReader;
var c = reader.StringComparer;
foreach (var attr in reader.GetModuleDefinition().GetCustomAttributes())
{
if (reader.GetAttributeNamespaceAndName(attr, out var ns, out var n))
{
if (c.Equals(ns, "System.Runtime.CompilerServices") && c.Equals(n, "RuntimeCompatibilityAttribute"))
{
var dec = reader.GetCustomAttribute(attr).DecodeValue(new CustomAttributeTypeProvider(this));

foreach (var arg in dec.NamedArguments)
{
if (arg.Name == "WrapNonExceptionThrows")
{
if (!(arg.Value is bool))
ThrowHelper.ThrowBadImageFormatException();
_isWrapNonExceptionThrows = (bool)arg.Value;
break;
}
}
}
}
}

_isWrapNonExceptionThrowsKnown = true;
}
return _isWrapNonExceptionThrows;
}
}
}
}
62 changes: 62 additions & 0 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7890,6 +7890,68 @@ CorInfoInline CEEInfo::canInline (CORINFO_METHOD_HANDLE hCaller,
}
}

// If the root level caller and callee modules do not have the same runtime
// wrapped exception behavior, and the callee has EH, we cannot inline.
_ASSERTE(!pCallee->IsDynamicMethod());
{
Module* pCalleeModule = pCallee->GetModule();
Module* pRootModule = pOrigCaller->GetModule();

if (pRootModule->IsRuntimeWrapExceptions() != pCalleeModule->IsRuntimeWrapExceptions())
{
if (pCallee->HasILHeader())
{
COR_ILMETHOD_DECODER header(pCallee->GetILHeader(), pCallee->GetMDImport(), NULL);
if (header.EHCount() > 0)
{
result = INLINE_FAIL;
szFailReason = "Inlinee and root method have different wrapped exception behavior";
goto exit;
}
}
else if (pCallee->IsIL() && pCallee->GetRVA() == 0)
{
MethodInfoHelperContext cxt{ pCallee };
// We will either find or create transient method details.
_ASSERTE(!cxt.HasTransientMethodDetails());

// IL methods with no RVA indicate there is no implementation defined in metadata.
// Check if we previously generated details/implementation for this method.
TransientMethodDetails* detailsMaybe = NULL;
if (FindTransientMethodDetails(pCallee, &detailsMaybe))
cxt.UpdateWith(*detailsMaybe);

CORINFO_METHOD_INFO methodInfo;
getMethodInfoHelper(cxt, &methodInfo);

if (cxt.Header->EHCount() > 0)
{
result = INLINE_FAIL;
szFailReason = "Inlinee and root method have different wrapped exception behavior";
}

// If we have transient method details we need to handle
// the lifetime of the details.
if (cxt.HasTransientMethodDetails())
{
// If we didn't find transient details, but we have them
// after getting method info, store them for cleanup.
if (detailsMaybe == NULL)
AddTransientMethodDetails(cxt.CreateTransientMethodDetails());

// Reset the context because ownership has either been
// transferred or was found in this instance.
cxt.UpdateWith({});
}

if (result != INLINE_PASS)
{
goto exit;
}
}
}
}

#ifdef PROFILING_SUPPORTED
if (pOrigCallerModule->IsInliningDisabledByProfiler())
{
Expand Down
2 changes: 1 addition & 1 deletion src/tests/JIT/Directed/throwbox/filter.il
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
.class public auto ansi beforefieldinit Test
extends [mscorlib]System.Object
{
.method public hidebysig static int32 Main() cil managed
.method public hidebysig static int32 Main() cil managed aggressiveinlining
{
.custom instance void [xunit.core]Xunit.FactAttribute::.ctor() = (
01 00 00 00
Expand Down
17 changes: 17 additions & 0 deletions src/tests/JIT/Directed/throwbox/wrappedexception.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using JitTest;
using Xunit;

public class WrappedException
{
// C# wrapper method calling IL method
// that throws a non-exception based object
//
[Fact]
public static void Problem()
{
Test.Main();
}
}
21 changes: 21 additions & 0 deletions src/tests/JIT/Directed/throwbox/wrappedexception.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<CLRTestPriority>1</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="wrappedexception.cs" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="filter.ilproj" ReferenceOutputAssembly="true" />
</ItemGroup>

<PropertyGroup>
<FilterOutputPath>$(OutputPath)/../filter</FilterOutputPath>
</PropertyGroup>

<Target Name="CopySupportAssembliesToOutputSubfolders" AfterTargets="Build">
<Copy SourceFiles="$(FilterOutputPath)/filter.dll" DestinationFolder="$(OutputPath)" />
</Target>

</Project>
Loading