Skip to content
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

Block inlines with different exception wrapping behavior #113849

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AndyAyersMS
Copy link
Member

Fixes #113815

@Copilot Copilot bot review requested due to automatic review settings March 24, 2025 18:37
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)
  • src/coreclr/vm/jitinterface.cpp: Language not supported

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 24, 2025
@AndyAyersMS
Copy link
Member Author

@jkotas PTAL
cc @dotnet/jit-contrib

Module* pCalleeModule = pCallee->GetModule();
Module* pRootModule = pOrigCaller->GetModule();

if (pRootModule->IsRuntimeWrapExceptions() != pCalleeModule->IsRuntimeWrapExceptions())
Copy link
Member

@jkotas jkotas Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These checks are cached cheap bit tests. We should do them first, and run the more expensive IL header decoder only when the modes do not match.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also looks like there are still issues with (lack of) an IL header.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely I need to handle transient methods here too.

@jkotas
Copy link
Member

jkotas commented Mar 24, 2025

I think we need the same check in crossgen2 for R2R

@AndyAyersMS
Copy link
Member Author

I think we need the same check in crossgen2 for R2R

Would the check go into CrossModuleInlineableUncached?

@jkotas
Copy link
Member

jkotas commented Mar 24, 2025

Would the check go into CrossModuleInlineableUncached?

I do not see caller and callee in CrossModuleInlineableUncached. I think it needs to be earlier.

@AndyAyersMS
Copy link
Member Author

Would the check go into CrossModuleInlineableUncached?

I do not see caller and callee in CrossModuleInlineableUncached. I think it needs to be earlier.

@jkotas I started in on crossgen2 support, but I can't see where custom attributes on modules are recorded, so could use some help finishing it off.

Also I think it would be better to either always return INLINE_FAIL from canInline or else distinguish between INLINE_FAIL and INLINE_NEVER and use the latter to short-circuit some work... at least to remember context-free decisions within the current process.

@AndyAyersMS
Copy link
Member Author

@davidwrighton maybe you can give me some pointers? How do we find module attributes in crossgen2? I see support for assembly attributes but nothing for modules.

@MichalStrehovsky
Copy link
Member

@davidwrighton maybe you can give me some pointers? How do we find module attributes in crossgen2? I see support for assembly attributes but nothing for modules.

We don't have higher level facilities for this in crossgen2, this will have to use System.Reflection.Metadata APIs, something along the lines of:

var reader = rootMod.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(mod));
            // Go over dec.NamedArguments and find WrapNonExceptionThrows
        }
    }
}

We'll probably want to cache this on EcmaModule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test failure: JIT/Directed/throwbox/filter/filter.dll
3 participants