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.

Sign up for GitHub

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

Use the List(int) constructor if possible when lowering collection expressions #72427

Merged
merged 9 commits into from
Mar 12, 2024
Merged

Use the List(int) constructor if possible when lowering collection expressions #72427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
96e6511
Use the List(int) constructor if available when lowering collection e…
jbevain Mar 7, 2024
0e29029
Update tests
jbevain Mar 7, 2024
82390e8
Update test
jbevain Mar 7, 2024
cdd8f75
Fix comment
jbevain Mar 7, 2024
18b3599
Add test ensuring Length is only called once
jbevain Mar 7, 2024
95098c5
Remove typo
jbevain Mar 7, 2024
404401c
Fix the scenario where we call List.ctor(in32) but we don't use optim…
jbevain Mar 11, 2024
723fb69
Update async test where we're not generating the known length temp
jbevain Mar 11, 2024
aacca9c
Extract knownLength expression
jbevain Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CollectionExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -883,12 +883,33 @@ private BoundExpression CreateAndPopulateList(BoundCollectionExpression node, Ty
}
}

// Create a temp for the knownLength
BoundAssignmentOperator assignmentToTemp;
BoundLocal? knownLengthTemp = null;

BoundObjectCreationExpression rewrittenReceiver;
if (useKnownLength && elements.Length > 0 && !useOptimizations)
if (useKnownLength && elements.Length > 0)
{
// List<ElementType> list = new(N + s1.Length + ...);
var constructor = ((MethodSymbol)_factory.WellKnownMember(WellKnownMember.System_Collections_Generic_List_T__ctorInt32)).AsMember(collectionType);
rewrittenReceiver = _factory.New(constructor, ImmutableArray.Create(GetKnownLengthExpression(elements, numberIncludingLastSpread, localsBuilder)));
var knownLengthExpression = GetKnownLengthExpression(elements, numberIncludingLastSpread, localsBuilder);

if (useOptimizations)
{
// If we use optimizations, we know the length of the resulting list, and we store it in a temp so we can pass it to List.ctor(int32) and to CollectionsMarshal.SetCount

// int knownLengthTemp = N + s1.Length + ...;
knownLengthTemp = _factory.StoreToTemp(knownLengthExpression, out assignmentToTemp, isKnownToReferToTempIfReferenceType: true);
localsBuilder.Add(knownLengthTemp);
sideEffects.Add(assignmentToTemp);

// List<ElementType> list = new(knownLengthTemp);
rewrittenReceiver = _factory.New(constructor, ImmutableArray.Create<BoundExpression>(knownLengthTemp));
}
else
{
// List<ElementType> list = new(N + s1.Length + ...)
rewrittenReceiver = _factory.New(constructor, ImmutableArray.Create(knownLengthExpression));
}
}
else
{
Expand All @@ -898,7 +919,6 @@ private BoundExpression CreateAndPopulateList(BoundCollectionExpression node, Ty
}

// Create a temp for the list.
BoundAssignmentOperator assignmentToTemp;
BoundLocal listTemp = _factory.StoreToTemp(rewrittenReceiver, out assignmentToTemp, isKnownToReferToTempIfReferenceType: true);
localsBuilder.Add(listTemp);
sideEffects.Add(assignmentToTemp);
Expand All @@ -909,9 +929,10 @@ private BoundExpression CreateAndPopulateList(BoundCollectionExpression node, Ty
Debug.Assert(useKnownLength);
Debug.Assert(setCount is { });
Debug.Assert(asSpan is { });
Debug.Assert(knownLengthTemp is { });
jbevain marked this conversation as resolved.
Show resolved Hide resolved

// CollectionsMarshal.SetCount<ElementType>(list, N + s1.Length + ...);
sideEffects.Add(_factory.Call(receiver: null, setCount, listTemp, GetKnownLengthExpression(elements, numberIncludingLastSpread, localsBuilder)));
// CollectionsMarshal.SetCount<ElementType>(list, knownLengthTemp);
sideEffects.Add(_factory.Call(receiver: null, setCount, listTemp, knownLengthTemp));

// var span = CollectionsMarshal.AsSpan<ElementType(list);
BoundLocal spanTemp = _factory.StoreToTemp(_factory.Call(receiver: null, asSpan, listTemp), out assignmentToTemp, isKnownToReferToTempIfReferenceType: true);
Expand Down
Loading