Skip to content

JIT: Remove a TODO comment#128836

Merged
jakobbotsch merged 1 commit into
dotnet:mainfrom
jakobbotsch:fix-comment
Jun 1, 2026
Merged

JIT: Remove a TODO comment#128836
jakobbotsch merged 1 commit into
dotnet:mainfrom
jakobbotsch:fix-comment

Conversation

@jakobbotsch
Copy link
Copy Markdown
Member

This case is legitimate. A reduced repro looks like:

public class Program
{
    public static async Task Main(string[] args)
    {
        Program p = GetP();
        int x = 0;
        await Aaa();
        if (args.Length > 1)
        {
            x = 456;
            await p.Foo(ref x);
        }
        else
        {
            await p.Bar();
        }
        Console.WriteLine(x);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static Program GetP() => new Program();

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static async Task Aaa()
    {
    }

    public virtual Task Foo(ref int x)
    {
        return Task.CompletedTask;
    }

    public virtual Task Bar()
    {
        return Task.CompletedTask;
    }
}

In this case we need to save x only in the first path even though x is live in both cases. The layout check is needed to catch this case.
Note that ref x is necessary to the Foo call, but only because otherwise the previous save-set check filters the case.

Copilot AI review requested due to automatic review settings June 1, 2026 09:55
@github-actions github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 1, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

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.

Pull request overview

This PR updates the tail-merging/reuse check for async suspension points in AsyncTransformation::IsReusableSuspension by removing an open-ended TODO and replacing it with an explanatory comment describing why a final continuation layout equality check is still required.

Changes:

  • Replace a TODO about unexpected liveness mismatches with a concrete explanation of an observed mismatch case.
  • Keep the existing final ContinuationLayoutBuilder::Equals guard, now documented as the catch-all for remaining save/layout disagreements.

@jakobbotsch jakobbotsch merged commit 3a811a4 into dotnet:main Jun 1, 2026
139 of 141 checks passed
@jakobbotsch jakobbotsch deleted the fix-comment branch June 1, 2026 13:01
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.

3 participants