-
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 1 commit
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
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
158 changes: 158 additions & 0 deletions
158
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,158 @@ | ||
| // 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, | ||
| arguments: node.Arguments, | ||
| argsToParamsOpt: node.ArgsToParamsOpt, | ||
| expanded: node.Expanded); | ||
| } | ||
|
|
||
| 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, | ||
| arguments: node.Arguments, | ||
| argsToParamsOpt: node.ArgsToParamsOpt, | ||
| expanded: node.Expanded); | ||
| } | ||
|
|
||
| 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, | ||
| arguments: node.Arguments, | ||
| argsToParamsOpt: default, | ||
| expanded: false); | ||
| } | ||
|
|
||
| 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, | ||
| ImmutableArray<BoundExpression> arguments, | ||
| ImmutableArray<int> argsToParamsOpt, | ||
| bool expanded) | ||
jjonescz marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { | ||
| Debug.Assert(receiverAddressKind is null || receiverType is not null); | ||
|
|
||
| int writableRefs = 0; | ||
| int readonlyRefs = 0; | ||
|
|
||
| if (used && (returnRefKind != RefKind.None || returnType.IsRefLikeOrAllowsRefLikeType())) | ||
| { | ||
| writableRefs++; | ||
| } | ||
|
|
||
| if (receiverType is not null) | ||
| { | ||
| receiverScope ??= ScopedKind.None; | ||
jjonescz marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if (receiverAddressKind is { } a && !IsAnyReadOnly(a) && receiverScope == ScopedKind.None) | ||
| { | ||
| writableRefs++; | ||
| } | ||
| else if (receiverType.IsRefLikeOrAllowsRefLikeType() && receiverScope != ScopedKind.ScopedValue) | ||
| { | ||
| if (isReceiverReadOnly || receiverType.IsReadOnly) | ||
| { | ||
| readonlyRefs++; | ||
| } | ||
| else | ||
| { | ||
| writableRefs++; | ||
| } | ||
| } | ||
| else if (receiverAddressKind != null && receiverScope == ScopedKind.None) | ||
| { | ||
| readonlyRefs++; | ||
| } | ||
| } | ||
|
|
||
| if (shouldReturnTrue(writableRefs, readonlyRefs)) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| for (var arg = 0; arg < arguments.Length; arg++) | ||
| { | ||
| var parameter = Binder.GetCorrespondingParameter( | ||
jjonescz marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| arg, | ||
| parameters, | ||
| argsToParamsOpt, | ||
| expanded); | ||
|
|
||
| if (parameter is not null) | ||
| { | ||
| if (parameter.RefKind.IsWritableReference() && parameter.EffectiveScope == ScopedKind.None) | ||
| { | ||
| writableRefs++; | ||
| } | ||
| else if (parameter.Type.IsRefLikeOrAllowsRefLikeType() && parameter.EffectiveScope != ScopedKind.ScopedValue) | ||
| { | ||
| if (parameter.Type.IsReadOnly || !parameter.RefKind.IsWritableReference()) | ||
| { | ||
| readonlyRefs++; | ||
| } | ||
| else | ||
| { | ||
| writableRefs++; | ||
| } | ||
| } | ||
| else if (parameter.RefKind != RefKind.None && parameter.EffectiveScope == ScopedKind.None) | ||
| { | ||
| readonlyRefs++; | ||
| } | ||
| } | ||
jjonescz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if (shouldReturnTrue(writableRefs, readonlyRefs)) | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
|
|
||
| static bool shouldReturnTrue(int writableRefs, int readonlyRefs) | ||
| { | ||
| return writableRefs > 0 && (writableRefs + readonlyRefs) > 1; | ||
| } | ||
| } | ||
| } | ||
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.