Propagate delegate loads during inlining

[MichalPetryka]

Propagate delegate loads from static readonly by value when inlining to avoid spilling them.

Alternative of #108579, needed for #109679.

[dotnet-policy-service]

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

[MichalPetryka]

@MihuBot -dependsOn 109679

[MichalPetryka]

@MihuBot -dependsOn 109679

[MichalPetryka]

@MihuBot

[EgorBo]

Can you explain why this limits static readonly propagation to only delegates?

[MichalPetryka]

Can you explain why this limits static readonly propagation to only delegates?

As seen in #108579, propagating everything leads to regressions due to CSE not being able to deduplicate all the new loads. This is not as much of an issue for delegates since those are usually single use and the benefit from better inlining of them outweighs any missed cases of duplicated loads.

[EgorBo]

As seen in #108579, propagating everything leads to regressions due to CSE not being able to deduplicate all the new loa

I don't see how that couldn't be an issue with delegates, e.g.

void Foo(delegate o, bool condition)
{
    if (condition)
    {
        use1(o);
    }
    else
    {
        use2(o);
    }
}

in this case CSE won't be able to handle it, so you'd create two indirs here, not sure it's a big problem just FYI.

I am not against this change, just that the diffs are quite big and we need to inspect them rather than blaming inliner. Let's see the SPMI diffs.

Also, didn't we agreed that we shouldn't touch impIsInvariant in Discord? And use the whatever way typeof() is propagated?

[MichalPetryka]

I don't see how that couldn't be an issue with delegates

The idea is that all nullchecks will be folded away while calls will be removed by #109679 so only other operations might end up with duplicated loads but those should be more rare.

Also, didn't we agreed that we shouldn't touch impIsInvariant in Discord? And use the whatever way typeof() is propagated?

We agreed on only putting it in impIsInvariant and not GenTree::IsInvariant. impIsInvariant itself is only used in one place in code currently, the arg propagation.