Fold "x ==/!= null" on NativeAOT for non-existent classes

[EgorBo]

Partially addresses #111256

This PR folds "obj ==/!= null" checks if VM tells JIT that obj is of type that has no real implementators (or they're trimmed out) so JIT can just fold such comparisons into true/false.

using System.Runtime.CompilerServices;

Foo(null);

[MethodImpl(MethodImplOptions.NoInlining)]
static void Foo(MyLogger? to = null) => to?.Log("hello");

abstract class MyLogger // has no real subclasses or they're trimmed out (like here)
{
    public abstract void Log(string s);
}

class MyLoggerImpl : MyLogger // trimmed out
{
    public override void Log(string s) => Console.WriteLine(s);
}

Codegen for Foo in Main:

; Assembly listing for method Program:<<Main>$>g__Foo|0_0(MyLogger) (FullOpts)
G_M59768_IG01:
       sub      rsp, 40
G_M59768_IG02:
       test     rcx, rcx
       je       SHORT G_M59768_IG04
G_M59768_IG03:
       lea      rdx, gword ptr [(reloc 0x4000000000420a40)]      ; '"hello"'
       mov      rax, qword ptr [rcx]
       call     [rax+0x30]MyLogger:Log(System.String):this
G_M59768_IG04:
       nop      
G_M59768_IG05:
       add      rsp, 40
       ret      
; Total bytes of code 28

Codegen for Foo in PR:

; Assembly listing for method Program:<<Main>$>g__Foo|0_0(MyLogger) (FullOpts)
G_M59768_IG01:
G_M59768_IG02:
       ret      
; Total bytes of code 1

Unfortunately, I was not able to make it work for interfaces without implementators like was asked in #111256. It seems to be an issue or NativeAOT side where getExactClasses returns -1 for interface case, because ScannedDevirtualizationManager.GetImplementingClasses returns null.

[MichalStrehovsky]

I'm looking at the failure - the fix will be on the JitInterface side. Looks like getExactClasses is not quite equipped to be able to properly answer this if the interface is never used for anything (besides typing storage location as the interface). Not sure what the fix will be yet...

[MichalStrehovsky]

I think what's in #111493 is going to fix this, but I want to spend a little bit more time on it to come up with the right tests. There's a couple more angles to it that I'm not sure I'm handling correctly in that PR. It does fix the one issue we're seeing in the trimming tests right now.

[EgorBo]

I think what's in #111493 is going to fix this, but I want to spend a little bit more time on it to come up with the right tests. There's a couple more angles to it that I'm not sure I'm handling correctly in that PR. It does fix the one issue we're seeing in the trimming tests right now.

feel free to integrate this change in your PR if it's simpler for you 🙂

[MichalStrehovsky]

/azp run runtime-nativeaot-outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[MichalStrehovsky]

I've been meditating on this for a while. Looks like the savings are not huge (0.5% in the best case if we do extra work in analysis that this PR doesn't do yet) and I'm not completely sure we can track this correctly. We only had one failing test while there was a glaring hole in the tracking (that I did not anticipate before I investigated the failing test) and I'm not sure if I was able to patch up all the holes.

The problem is that the compiler currently doesn't track types that are only used to type storage locations. So if there's code like:

IFoo x = new Foo();

And one asks the whole program analysis "did you hear about IFoo", it will mostly respond "never heard of it". The change I made in my commit here makes analysis consider all interfaces on allocated classes irrespective if there was or wasn't a cast or interface method call (which are the "interesting" events for whole program analysis before this PR). This new assumption possibly deoptimizes things in some spots, but probably not in practice.

But still, I'm not very convinced this would cover all the cases. It also complicates things quite a bit. Maybe this is not worth it. But thank you for entertaining the idea with me @EgorBo!

[EgorBo]

But thank you for entertaining the idea with me @EgorBo!

No problem! Happy to try new ideas if you have any 🙂