ILC fails to detect and trim some dependency cycles (interface -> attribute -> [DAM] -> type impl -> new())

[Sergio0694]

Description

Found a possible miss by ILC when working on microsoft/CsWinRT#1907. This causes IReferenceArray<T> to be kept, which then produces a bunch of IL3050 warnings when I try to publish the Microsoft Store with a preview of CsWinRT from that PR. We can rework IReferenceArray<T> to be more AOT friendly, but this feels also feels like something that ILC should be able to handle.

cc. @MichalStrehovsky @sbomer

Reproduction Steps

using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;

namespace NaotTrimmingDamTest
{
    internal class Program
    {
        static void Main()
        {
            Console.WriteLine(M(new object()));
        }

        [MethodImpl(MethodImplOptions.NoInlining)]
        public static bool M(object obj)
        {
            Type type = obj.GetType();

            return type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IFoo<>);
        }
    }

    [Keep(typeof(ABI.NaotTrimmingDamTest.IFoo<>))]
    internal interface IFoo<T>
    {
    }

    public sealed class KeepAttribute : Attribute
    {
        public KeepAttribute([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] Type type)
        {
        }
    }
}

namespace ABI.NaotTrimmingDamTest
{
    internal sealed class IFoo<T> : global::NaotTrimmingDamTest.IFoo<T>
    {
        public static object Create()
        {
            return new IFoo<T>();
        }
    }
}

Expected behavior

• All IFoo<T> types should be completely trimmed.
• I also expect M to be optimized to just return false, basically.
• KeepAttribute should also be removed.

There is no type that can possibly be instantiated, that implements IFoo<T>.

Actual behavior

Both IFoo<T> types and [Keep] are kept. Not sure if M is being optimized (don't see the codegen from here).

Additional context

I tried changing the Create method to this:

public static object Create()
{
    if (!RuntimeFeature.IsDynamicCodeCompiled)
    {
        throw new NotSupportedException("");
    }

    return new IFoo<T>();
}

Thinking maybe if this just throws on AOT, ILC will see the type never being constructed and will trim. Nope, no changes.

Regression?

No.

Configuration

• .NET 9.0.102

[dotnet-policy-service]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

[jkoritzinsky]

The problem is that types referenced by typeof(X) in attributes are marked as "Reflected Type"s here:

runtime/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs

Line 269 in 55e9ab5

dependencies.Add(factory.ReflectedType(type), "Custom attribute blob");

And reflected types are marked as Maximally Constructable here:

runtime/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedTypeNode.cs

Line 39 in 55e9ab5

new DependencyListEntry(factory.MaximallyConstructableType(_type), "Reflection target"),

We would need some way to represent "here's a System.Type that has reflectible but cannot be constructed".

[MichalStrehovsky]

We'd normally cut this off much earlier than at the custom attribute analysis spot, but the uninstantiated generic type part makes it harder.

Normally when there is a typeof somewhere (as in the return type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IFoo<>); line) the compiler needs to assume the type is reflected. One can get the name, or even read custom attributes on the result of typeof; the custom attribute reading APIs are not RequiresUnreferencedCode and we don't do any extra analysis on them because it would be mostly futile given real-world code.

There is an optimization though for the case when the typeof is used in a type equality comparison like here.

The compiler can represent types in two forms within the dependency graph: "constructed type" and "unconstructed type" (called just "type"). Constructed type is modeled as something that could be allocated on the GC heap, has a valid vtable, etc. The unconstructed form is mostly used for casting so we can get away with less stuff on it. For example, casting doesn't need reflection metadata so we don't generate it (that includes the custom attributes).

The optimization for typeof used with type equality will downgrade constructed form (that we'd normally hand out) to unconstructed form (since we know equality check really doesn't need the full type data structure).

The problem is that uninstantiated generic types don't have a constructed form. So while the typeof() == ... optimization does kick in, it doesn't actually do a downgrade from constructed to unconstructed form. We can never have a constructed form of an uninstantiated type. So for analysis purposes this typeof that is used in a comparison is no different from a typeof that was e.g. stored in some local variable and used for purposes the analysis doesn't track.

The fix will probably require distinguishing between typeof of an uninstantiated generic type that can be reflected on vs one that cannot.

You can check this yourself - if you make the NaotTrimmingDamTest.IFoo interface non-generic, the other IFoo and Create method will disappear.

[Sergio0694]

Oh I see, that makes sense. I do remember a similar issue coming up a while back and that you had made some optimizations for this scenario, so I was wondering why this would be different. Thanks you for the explanation!

We do have a fair amount of type checks with unconstructed generic types in CsWinRT, so optimizing this would be cool to see, assuming it's doable 😄

[hez2010]

I'm wondering if this one can be addressed by using RyuJIT as the IL scanner.

[MichalStrehovsky]

I'm wondering if this one can be addressed by using RyuJIT as the IL scanner.

RyuJIT as scanner helps for optimizations that RyuJIT can sometimes do but we don't want to risk predicting them happening when building whole program view (we need guarantees that optimizations performed when building whole program view also happen during codegen - otherwise the whole program view is wrong).

The type equality optimization can be done in the existing scanner and we do it. It is actually one of the things that will be more difficult to do in the RyuJIT codebase when RyuJIT is scanner. RyuJIT codebase expects that a type is always backed by a single symbol. For this optimizations, we potentially have two symbols for the same type (the constructed and unconstructed one). RyuJIT as it is right now cannot handle this. We actually need to do work to hide that from RyuJIT (dotnet/runtimelab#1754).