-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Avoid reusing temps whose refs might be captured #76009
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
Merged
Merged
Changes from 17 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
80aaae6
Avoid reusing temps whose refs might be captured
jjonescz 4672a11
Revert some changes
jjonescz 4dc8f82
Simplify the heuristic
jjonescz 60dec1e
Avoid reusing any local whose address has been taken
jjonescz 1d6b786
Update tests
jjonescz 6ccc2ad
Inline a function
jjonescz 402376f
Revert unrelated change
jjonescz 689cc3e
Remove non-reusable locals after checking for them
jjonescz 0428e09
Keep ref count for addressed locals list
jjonescz 1558dbc
Extend an assert
jjonescz 910d711
Add high-level comment to MightEscapeTemporaryRefs
jjonescz 583015d
Filter non-reusable locals
jjonescz 45dcef5
Simplify `int`s to `bool`s
jjonescz 0fca536
Replace coalesce with an assert
jjonescz 0f30460
Mark nested calls as always used
jjonescz cd7f4bc
Fix this parameter of nint methods
jjonescz 50accc5
Test chained call
jjonescz 6718681
Simplify by using this parameter symbol
jjonescz d01890f
Fix readonly targets
jjonescz 42ffdc8
Fixup a test
jjonescz 0669bce
Remove an unused parameter
jjonescz bc9fd0c
Merge branch 'main' into 67435-RefTempReuse-2
jjonescz bf95dba
Fix nullability after merge
jjonescz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
126 changes: 126 additions & 0 deletions
126
src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System.Collections.Immutable; | ||
| using System.Diagnostics; | ||
| using Microsoft.CodeAnalysis.CSharp.Symbols; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.CSharp.CodeGen; | ||
|
|
||
| internal partial class CodeGenerator | ||
| { | ||
| private static bool MightEscapeTemporaryRefs(BoundCall node, bool used, AddressKind? receiverAddressKind) | ||
| { | ||
| return MightEscapeTemporaryRefs( | ||
| used: used, | ||
| returnType: node.Type, | ||
| returnRefKind: node.Method.RefKind, | ||
| receiverType: !node.Method.RequiresInstanceReceiver ? null : node.ReceiverOpt?.Type, | ||
| receiverScope: node.Method.TryGetThisParameter(out var thisParameter) ? thisParameter?.EffectiveScope : null, | ||
| receiverAddressKind: receiverAddressKind, | ||
| isReceiverReadOnly: node.Method.IsEffectivelyReadOnly, | ||
| parameters: node.Method.Parameters); | ||
| } | ||
|
|
||
| private static bool MightEscapeTemporaryRefs(BoundObjectCreationExpression node, bool used) | ||
| { | ||
| return MightEscapeTemporaryRefs( | ||
| used: used, | ||
| returnType: node.Type, | ||
| returnRefKind: RefKind.None, | ||
| receiverType: null, | ||
| receiverScope: null, | ||
| receiverAddressKind: null, | ||
| isReceiverReadOnly: false, | ||
| parameters: node.Constructor.Parameters); | ||
| } | ||
|
|
||
| private static bool MightEscapeTemporaryRefs(BoundFunctionPointerInvocation node, bool used) | ||
| { | ||
| FunctionPointerMethodSymbol method = node.FunctionPointer.Signature; | ||
| return MightEscapeTemporaryRefs( | ||
| used: used, | ||
| returnType: node.Type, | ||
| returnRefKind: method.RefKind, | ||
| receiverType: null, | ||
| receiverScope: null, | ||
| receiverAddressKind: null, | ||
| isReceiverReadOnly: false, | ||
| parameters: method.Parameters); | ||
| } | ||
|
|
||
| private static bool MightEscapeTemporaryRefs( | ||
RikkiGibson marked this conversation as resolved.
Show resolved
Hide resolved
jjonescz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| bool used, | ||
| TypeSymbol returnType, | ||
| RefKind returnRefKind, | ||
| TypeSymbol? receiverType, | ||
| ScopedKind? receiverScope, | ||
jjonescz marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| AddressKind? receiverAddressKind, | ||
| bool isReceiverReadOnly, | ||
| ImmutableArray<ParameterSymbol> parameters) | ||
| { | ||
| Debug.Assert(receiverAddressKind is null || receiverType is not null); | ||
|
|
||
| // We check the signature of the method, counting potential `ref` sources and destinations | ||
| // to determine whether a `ref` can be captured by the method. | ||
| // The emit layer then uses this information to avoid reusing temporaries that are passed by ref to such methods. | ||
|
|
||
| // whether we have any outputs that can capture `ref`s | ||
| bool anyRefTargets = false; | ||
| // whether we have any inputs that can contain `ref`s | ||
| bool anyRefSources = false; | ||
| // NOTE: If there is at least one output and at least one input, a `ref` can be captured. | ||
|
|
||
| if (used && (returnRefKind != RefKind.None || returnType.IsRefLikeOrAllowsRefLikeType())) | ||
| { | ||
| // If returning by ref or returning a ref struct, the result might capture `ref`s. | ||
| anyRefTargets = true; | ||
| } | ||
|
|
||
| if (receiverType is not null) | ||
| { | ||
| Debug.Assert(receiverScope != null); | ||
| if (receiverType.IsRefLikeOrAllowsRefLikeType() && receiverScope != ScopedKind.ScopedValue) | ||
| { | ||
| anyRefSources = true; | ||
| if (!isReceiverReadOnly && !receiverType.IsReadOnly) | ||
| { | ||
| anyRefTargets = true; | ||
| } | ||
| } | ||
| else if (receiverAddressKind != null && receiverScope == ScopedKind.None) | ||
| { | ||
| anyRefSources = true; | ||
| } | ||
| } | ||
|
|
||
| if (anyRefTargets && anyRefSources) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| foreach (var parameter in parameters) | ||
| { | ||
| if (parameter.Type.IsRefLikeOrAllowsRefLikeType() && parameter.EffectiveScope != ScopedKind.ScopedValue) | ||
| { | ||
| anyRefSources = true; | ||
| if (!parameter.Type.IsReadOnly && parameter.RefKind.IsWritableReference()) | ||
jjonescz marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { | ||
| anyRefTargets = true; | ||
| } | ||
| } | ||
jjonescz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| else if (parameter.RefKind != RefKind.None && parameter.EffectiveScope == ScopedKind.None) | ||
| { | ||
| anyRefSources = true; | ||
| } | ||
|
|
||
| if (anyRefTargets && anyRefSources) | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.