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

Collection expression IL could be simplified for single span element #72566

Closed
stephentoub opened this issue Mar 15, 2024 · 1 comment · Fixed by #73086
Closed

Collection expression IL could be simplified for single span element #72566

stephentoub opened this issue Mar 15, 2024 · 1 comment · Fixed by #73086

Comments

@stephentoub
Copy link
Member

Version Used:
d8d7886

Steps to Reproduce:

using System;
using System.Runtime.CompilerServices;
public class C
{
    public void M1(int i) => Use([i]);
    public void M2(int i) => Use(new ReadOnlySpan<int>(in i));
    
    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void Use<T>(ReadOnlySpan<T> span){}
}

SharpLab

Expected Behavior:
M1 and M2 could produce identical IL.

Actual Behavior:
The IL generated is more akin to:

    public void M1(int i)
    {
        <>y__InlineArray1<int> buffer = default(<>y__InlineArray1<int>);
        <PrivateImplementationDetails>.InlineArrayElementRef<<>y__InlineArray1<int>, int>(ref buffer, 0) = i;
        Use(<PrivateImplementationDetails>.InlineArrayAsReadOnlySpan<<>y__InlineArray1<int>, int>(ref buffer, 1));
    }

    public void M2(int i)
    {
        Use(new ReadOnlySpan<int>(ref i));
    }

The JIT is able to produce exactly the same thing for both, but the IL for the former is more complicated and requires the compiler synthesize the <>y__InlineArray1<T> type if it wouldn't otherwise be necessary.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 15, 2024
@jaredpar jaredpar added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 19, 2024
@jaredpar jaredpar added this to the 17.10 milestone Mar 19, 2024
@jaredpar jaredpar modified the milestones: 17.10, 17.11 Mar 26, 2024
@RikkiGibson
Copy link
Contributor

RikkiGibson commented Apr 17, 2024

I think the compiler should still create a temp here to pass by-ref to the span constructor. The ReadOnlySpan in the original sample should not hold a reference to x. Otherwise, the behavior is in violation of the spec, when x is reassigned before the ReadOnlySpan is used.

The JIT can likely eliminate the temp when it can see that the original variable is not reassigned while the ReadOnlySpan is live.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants