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 expressions: use constant indices when populating the span for a List<T> #71183

Open
cston opened this issue Dec 8, 2023 · 1 comment
Assignees
Labels
Area-Compilers Bug Feature - Collection Expressions help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Milestone

Comments

@cston
Copy link
Member

cston commented Dec 8, 2023

Originally posted by @Sergio0694 in #70656 (comment)

Just noticed this (sharplab):

public static IList<int> M() => [1, 2, 8, 4];
List<int> list = new List<int>();
CollectionsMarshal.SetCount(list, 4);
Span<int> span = CollectionsMarshal.AsSpan(list);
int num = 0;
span[num] = 1;
num++;
span[num] = 2;
num++;
span[num] = 8;
num++;
span[num] = 4;
num++;
return list;

It seems there's a fair amount of extra IL just to keep track of that index that's incremented every time, which also makes it not a constant (which might be less useful for the JIT). Is there any specific reason why Roslyn is doing this rather than just emitting:

List<int> list = new List<int>();
CollectionsMarshal.SetCount(list, 4);
Span<int> span = CollectionsMarshal.AsSpan(list);
span[0] = 1;
span[1] = 2;
span[2] = 8;
span[3] = 4;
return list;

The latter might even allow the JIT to vectorize some of these assignments, in theory (also the IL seems smaller too?).

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 8, 2023
@cston
Copy link
Member Author

cston commented Dec 8, 2023

There is a similar issue when populating an array or span from a collection expression with spread elements of known length. In those cases, a local is used for all elements, even though we could use constants for the elements before the first spread (for instance, for assigning x and y in the following - see sharplab.io).

static void CreateArray(int x, int y, int[] a, int z)
{
    int[] b = [x, y, ..a, z];
}

@jaredpar jaredpar added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 2, 2024
@jaredpar jaredpar added this to the 17.10 milestone Jan 2, 2024
@jaredpar jaredpar added the help wanted The issue is "up for grabs" - add a comment if you are interested in working on it label Mar 27, 2024
@jaredpar jaredpar modified the milestones: 17.10, Backlog Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug Feature - Collection Expressions help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Projects
None yet
Development

No branches or pull requests

2 participants