Fix GC hole on arm32 #50759
Merged
Fix GC hole on arm32 #50759
Conversation
This file contains 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
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
fyi, original code to handle the LCL_VAR_ADDR on arm64 was from #38316 |
|
@sandreenko @dotnet/jit-contrib PTAL |
sandreenko
reviewed
Apr 6, 2021
| @@ -13510,6 +13510,20 @@ void emitter::emitInsLoadStoreOp(instruction ins, emitAttr attr, regNumber dataR | |||
| } | |||
| else // addr is not contained, so we evaluate it into a register | |||
| { | |||
| #ifdef DEBUG | |||
There was a problem hiding this comment.
Nit: maybe void Lowering::CheckNode(Compiler* compiler, GenTree* node) is a better place for such check:
- it is earlier;
- it is one place for all platforms
There was a problem hiding this comment.
I'm worried about moving the assert to CheckNode:
- It's pretty far upstream. Am I guaranteed no other phases are going to introduce the bad pattern after that is run and before this?
- I'd have to check for any IND-like thing that might end up getting to this code path, then looking at the IND's address. Might I miss something doing that? It feels perhaps like doing too much anticipating what downstream phases and codegen are actually going to do. E.g., it looks like for STOREIND(LCL_VAR_ADDR) of a SIMD12 type, the addr isn't contained. I'd have to check that.
Comments?
There was a problem hiding this comment.
It's pretty far upstream. Am I guaranteed no other phases are going to introduce the bad pattern after that is run and before this?
We are adding "tracked" before Lowering and "contained" during Lowering, so after Lowering both should be stable (there is one exception for of ClearContained for bitcast but it is not important here)
I'd have to check for any IND-like thing that might end up getting to this code path, then looking at the IND's address. Might I miss something doing that? It feels perhaps like doing too much anticipating what downstream phases and codegen are actually going to do. E.g., it looks like for STOREIND(LCL_VAR_ADDR) of a SIMD12 type, the addr isn't contained. I'd have to check that.
I was thinking about adding something like:
case GT_LCL_VAR_ADDR:
case GT_LCL_FLD_ADDR:
{
const GenTreeLclVarCommon* varNode = tree->AsLclVarCommon();
const LclVarDsc* varDsc = emitComp->lvaGetDesc(varNode);
assert(varNode->isContained() || !varDsc->lvTracked);
break;
}
to Lowering::CheckNode.
would not it be enough?
There was a problem hiding this comment.
That doesn't work. There are cases where the LCL_VAR_ADDR is a not required to be contained. I see such a case with STORE_OBJ(LCL_VAR_ADDR, LCL_VAR). I also see STOREIND-simd12(LCL_FLD_ADDR, LCLVAR-simd16).
If I add || !varTypeIsGC(varDsc) that's still not enough, as I see cases where we have not-contained tracked address loads, which is fine because genCodeForLclAddr does the right liveness updates for uses. The issue is for stores. So, I'd need to look at the context (parent node). And it's not clear that just STOREIND is necessary to check.
Bottom-line: does this change really need this more general assert in a common location? Or can I leave it as-is?
On arm32 only, in cases where we have STOREIND(GT_LCL_VAR_ADDR) of
a GC var, the JIT was generating (e.g.):
```
add r1, sp, 48 // [V27 tmp23]
str r0, [r1]
```
If this was a local variable birth, codegen would correctly process the lifetime. However,
we would have created an instrDesc that knows nothing about the local var, so can't emit
any GC transitions during emission.
It turns out this is also sub-optimal code; we don't need to create a byref of the
address of a local when we can encode the address directly in an addressing mode, as:
```
str r0, [sp+48] // [V27 tmp23]
```
There was already code on arm64 to handle this case, by making the LCL_VAR_ADDR contained
and using the emitIns_S_R / emitIns_R_S functions, which create instrDescs with the necessary
local tracking info.
I added an assert, on both arm32 and arm64, that non-contained local var address nodes must
refer to untracked locals. Otherwise, we could potentially create addresses where we don't
properly track the GC info.
There are quite a few diffs, avoiding creating the local var address. There are several cases
I noted in the Microsoft.CodeAnalysis assemblies of bad GC info now fixed (this is where the
initial failure came from, that the Runtime_45557 test was derived from).
From libraries crossgen:
```
Summary of Code Size diffs:
(Lower is better)
Total bytes of base: 255542
Total bytes of diff: 251924
Total bytes of delta: -3618 (-1.42% of base)
diff is an improvement.
```
<details>
<summary>Detail diffs</summary>
```
Top file improvements (bytes):
-18 : 133274.dasm (-0.95% of base)
-16 : 136993.dasm (-1.44% of base)
-14 : 127453.dasm (-1.35% of base)
-12 : 126915.dasm (-1.15% of base)
-12 : 63852.dasm (-1.71% of base)
-12 : 137357.dasm (-3.59% of base)
-12 : 7927.dasm (-2.29% of base)
-12 : 39850.dasm (-1.11% of base)
-10 : 132884.dasm (-2.11% of base)
-10 : 10262.dasm (-0.95% of base)
-10 : 131112.dasm (-7.35% of base)
-10 : 74924.dasm (-4.50% of base)
-8 : 10344.dasm (-5.48% of base)
-8 : 10435.dasm (-6.67% of base)
-8 : 56301.dasm (-1.08% of base)
-8 : 56683.dasm (-0.31% of base)
-8 : 56686.dasm (-0.49% of base)
-8 : 10341.dasm (-5.48% of base)
-8 : 10346.dasm (-5.06% of base)
-8 : 126900.dasm (-0.43% of base)
1107 total files with Code Size differences (1107 improved, 0 regressed), 0 unchanged.
Top method improvements (bytes):
-18 (-0.95% of base) : 133274.dasm - Microsoft.CodeAnalysis.CSharp.Binder:CreateUserDefinedConversion(Microsoft.CodeAnalysis.CSharp.CSharpSyntaxNode,Microsoft.CodeAnalysis.CSharp.BoundExpression,Microsoft.CodeAnalysis.CSharp.Conversion,bool,Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol,Microsoft.CodeAnalysis.DiagnosticBag):Microsoft.CodeAnalysis.CSharp.BoundExpression:this
-16 (-1.44% of base) : 136993.dasm - Enumerator:MoveNext():bool:this
-14 (-1.35% of base) : 127453.dasm - Microsoft.CodeAnalysis.CSharp.OverloadResolution:IsApplicable(Microsoft.CodeAnalysis.CSharp.Symbol,EffectiveParameters,Microsoft.CodeAnalysis.CSharp.AnalyzedArguments,System.Collections.Immutable.ImmutableArray`1[Int32],bool,bool,bool,byref):Microsoft.CodeAnalysis.CSharp.MemberAnalysisResult:this
-12 (-1.15% of base) : 126915.dasm - Microsoft.CodeAnalysis.CSharp.OverloadResolution:IsMemberApplicableInExpandedForm(System.__Canon,System.__Canon,Microsoft.CodeAnalysis.ArrayBuilder`1[[Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol, Microsoft.CodeAnalysis.CSharp, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]],Microsoft.CodeAnalysis.CSharp.AnalyzedArguments,bool,byref):Microsoft.CodeAnalysis.CSharp.MemberResolutionResult`1[__Canon]:this
-12 (-1.71% of base) : 63852.dasm - System.Security.Cryptography.RSAKeyFormatHelper:FromPkcs1PrivateKey(System.ReadOnlyMemory`1[Byte],byref,byref)
-12 (-3.59% of base) : 137357.dasm - System.Reflection.Metadata.Ecma335.ControlFlowBuilder:AddExceptionRegion(ushort,System.Reflection.Metadata.Ecma335.LabelHandle,System.Reflection.Metadata.Ecma335.LabelHandle,System.Reflection.Metadata.Ecma335.LabelHandle,System.Reflection.Metadata.Ecma335.LabelHandle,System.Reflection.Metadata.Ecma335.LabelHandle,System.Reflection.Metadata.EntityHandle):this
-12 (-2.29% of base) : 7927.dasm - Microsoft.CodeAnalysis.Emit.AddedOrChangedMethodInfo:MapTypes(Microsoft.CodeAnalysis.Emit.SymbolMatcher):Microsoft.CodeAnalysis.Emit.AddedOrChangedMethodInfo:this
-12 (-1.11% of base) : 39850.dasm - Internal.Cryptography.Pal.OpenSslX509CertificateReader:CopyWithPrivateKey(System.Security.Cryptography.RSA):Internal.Cryptography.ICertificatePal:this
-10 (-2.11% of base) : 132884.dasm - Microsoft.CodeAnalysis.CSharp.OverloadResolution:IsConstructorApplicableInExpandedForm(Microsoft.CodeAnalysis.CSharp.Symbols.MethodSymbol,Microsoft.CodeAnalysis.CSharp.AnalyzedArguments,byref):Microsoft.CodeAnalysis.CSharp.MemberAnalysisResult:this
-10 (-0.95% of base) : 10262.dasm - Microsoft.CodeAnalysis.Emit.DeltaMetadataWriter:SerializeLocalVariablesSignature(Microsoft.Cci.IMethodBody):int:this
-10 (-7.35% of base) : 131112.dasm - Microsoft.CodeAnalysis.CSharp.MemberAnalysisResult:BadArgumentConversions(System.Collections.Immutable.ImmutableArray`1[Int32],System.Collections.Immutable.ImmutableArray`1[Int32],System.Collections.Immutable.ImmutableArray`1[Conversion]):Microsoft.CodeAnalysis.CSharp.MemberAnalysisResult
-10 (-4.50% of base) : 74924.dasm - System.MemoryExtensions:AsMemory(System.__Canon[],System.Range):System.Memory`1[__Canon]
-8 (-5.48% of base) : 10344.dasm - Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers:GetAnalyzerSyntaxDiagnosticsCoreAsync(Microsoft.CodeAnalysis.SyntaxTree,System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer, Microsoft.CodeAnalysis, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]],System.Threading.CancellationToken):System.Threading.Tasks.Task`1[[System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.Diagnostic, Microsoft.CodeAnalysis, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]], System.Collections.Immutable, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]]:this
-8 (-6.67% of base) : 10435.dasm - Microsoft.CodeAnalysis.Diagnostics.AnalysisState:OnCompilationEventsGeneratedAsync(System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.Diagnostics.CompilationEvent, Microsoft.CodeAnalysis, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]],Microsoft.CodeAnalysis.Diagnostics.AnalyzerDriver,System.Threading.CancellationToken):System.Threading.Tasks.Task:this
-8 (-1.08% of base) : 56301.dasm - Microsoft.CodeAnalysis.VisualBasic.Symbols.MethodSignatureComparer:HaveSameParameterTypes(System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.VisualBasic.Symbols.ParameterSymbol, Microsoft.CodeAnalysis.VisualBasic, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]],Microsoft.CodeAnalysis.VisualBasic.Symbols.TypeSubstitution,System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.VisualBasic.Symbols.ParameterSymbol, Microsoft.CodeAnalysis.VisualBasic, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]],Microsoft.CodeAnalysis.VisualBasic.Symbols.TypeSubstitution,bool,bool):bool
-8 (-0.31% of base) : 56683.dasm - Microsoft.CodeAnalysis.VisualBasic.Symbols.ConstraintsHelper:ReportIndirectConstraintConflicts(Microsoft.CodeAnalysis.VisualBasic.Symbols.SourceTypeParameterSymbol,Microsoft.CodeAnalysis.ArrayBuilder`1[TypeParameterDiagnosticInfo],byref)
-8 (-0.49% of base) : 56686.dasm - Microsoft.CodeAnalysis.VisualBasic.Symbols.MethodSignatureComparer:DetailedParameterCompare(System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.VisualBasic.Symbols.ParameterSymbol, Microsoft.CodeAnalysis.VisualBasic, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]],byref,System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.VisualBasic.Symbols.ParameterSymbol, Microsoft.CodeAnalysis.VisualBasic, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]],byref,int,int):int
-8 (-5.48% of base) : 10341.dasm - Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers:GetAnalyzerCompilationDiagnosticsCoreAsync(System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer, Microsoft.CodeAnalysis, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]],System.Threading.CancellationToken):System.Threading.Tasks.Task`1[[System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.Diagnostic, Microsoft.CodeAnalysis, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]], System.Collections.Immutable, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]]:this
-8 (-5.06% of base) : 10346.dasm - Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers:GetAnalyzerSemanticDiagnosticsCoreAsync(Microsoft.CodeAnalysis.SemanticModel,System.Nullable`1[TextSpan],System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer, Microsoft.CodeAnalysis, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]],System.Threading.CancellationToken):System.Threading.Tasks.Task`1[[System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.Diagnostic, Microsoft.CodeAnalysis, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]], System.Collections.Immutable, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]]:this
-8 (-0.43% of base) : 126900.dasm - Microsoft.CodeAnalysis.CSharp.OverloadResolutionResult`1[__Canon][System.__Canon]:ReportDiagnostics(Microsoft.CodeAnalysis.CSharp.Binder,Microsoft.CodeAnalysis.Location,Microsoft.CodeAnalysis.DiagnosticBag,System.String,Microsoft.CodeAnalysis.CSharp.BoundExpression,Microsoft.CodeAnalysis.CSharp.AnalyzedArguments,System.Collections.Immutable.ImmutableArray`1[__Canon],Microsoft.CodeAnalysis.CSharp.Symbols.NamedTypeSymbol,Microsoft.CodeAnalysis.CSharp.Symbols.NamedTypeSymbol,Microsoft.CodeAnalysis.CSharp.CSharpSyntaxNode,bool):this
Top method improvements (percentages):
-6 (-13.64% of base) : 135707.dasm - System.Reflection.Metadata.BlobBuilder:WriteBytes(System.Collections.Immutable.ImmutableArray`1[Byte],int,int):this
-4 (-13.33% of base) : 17244.dasm - System.Collections.Immutable.ImmutableArray`1[__Canon][System.__Canon]:op_Equality(System.Nullable`1[ImmutableArray`1],System.Nullable`1[ImmutableArray`1]):bool
-4 (-13.33% of base) : 17245.dasm - System.Collections.Immutable.ImmutableArray`1[__Canon][System.__Canon]:op_Inequality(System.Nullable`1[ImmutableArray`1],System.Nullable`1[ImmutableArray`1]):bool
-2 (-10.00% of base) : 138130.dasm - System.Reflection.Internal.ImmutableByteArrayInterop:DangerousGetUnderlyingArray(System.Collections.Immutable.ImmutableArray`1[Byte]):System.Byte[]
-4 (-8.33% of base) : 75404.dasm - System.Range:Equals(System.Range):bool:this
-10 (-7.35% of base) : 131112.dasm - Microsoft.CodeAnalysis.CSharp.MemberAnalysisResult:BadArgumentConversions(System.Collections.Immutable.ImmutableArray`1[Int32],System.Collections.Immutable.ImmutableArray`1[Int32],System.Collections.Immutable.ImmutableArray`1[Conversion]):Microsoft.CodeAnalysis.CSharp.MemberAnalysisResult
-8 (-6.67% of base) : 10435.dasm - Microsoft.CodeAnalysis.Diagnostics.AnalysisState:OnCompilationEventsGeneratedAsync(System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.Diagnostics.CompilationEvent, Microsoft.CodeAnalysis, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]],Microsoft.CodeAnalysis.Diagnostics.AnalyzerDriver,System.Threading.CancellationToken):System.Threading.Tasks.Task:this
-8 (-5.80% of base) : 130961.dasm - Microsoft.CodeAnalysis.CSharp.MemberAnalysisResult:NormalForm(System.Collections.Immutable.ImmutableArray`1[Int32],System.Collections.Immutable.ImmutableArray`1[Conversion],bool):Microsoft.CodeAnalysis.CSharp.MemberAnalysisResult
-8 (-5.56% of base) : 10340.dasm - Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers:GetAnalyzerCompilationDiagnosticsAsync(System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer, Microsoft.CodeAnalysis, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]],System.Threading.CancellationToken):System.Threading.Tasks.Task`1[[System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.Diagnostic, Microsoft.CodeAnalysis, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]], System.Collections.Immutable, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]]:this
-8 (-5.48% of base) : 10344.dasm - Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers:GetAnalyzerSyntaxDiagnosticsCoreAsync(Microsoft.CodeAnalysis.SyntaxTree,System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer, Microsoft.CodeAnalysis, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]],System.Threading.CancellationToken):System.Threading.Tasks.Task`1[[System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.Diagnostic, Microsoft.CodeAnalysis, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]], System.Collections.Immutable, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]]:this
-8 (-5.48% of base) : 10341.dasm - Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers:GetAnalyzerCompilationDiagnosticsCoreAsync(System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer, Microsoft.CodeAnalysis, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]],System.Threading.CancellationToken):System.Threading.Tasks.Task`1[[System.Collections.Immutable.ImmutableArray`1[[Microsoft.CodeAnalysis.Diagnostic, Microsoft.CodeAnalysis, Version=1.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]], System.Collections.Immutable, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]]:this
-6 (-5.45% of base) : 166017.dasm - Newtonsoft.Json.Bson.BsonDataReader:DoReadAsDecimalAsync(System.Threading.CancellationToken):System.Threading.Tasks.Task`1[Nullable`1]:this
-6 (-5.45% of base) : 166019.dasm - Newtonsoft.Json.Bson.BsonDataReader:DoReadAsDoubleAsync(System.Threading.CancellationToken):System.Threading.Tasks.Task`1[Nullable`1]:this
-6 (-5.45% of base) : 166021.dasm - Newtonsoft.Json.Bson.BsonDataReader:DoReadAsInt32Async(System.Threading.CancellationToken):System.Threading.Tasks.Task`1[Nullable`1]:this
-6 (-5.45% of base) : 166024.dasm - Newtonsoft.Json.Bson.BsonDataReader:ReaderReadAndAssertAsync(System.Threading.CancellationToken):System.Threading.Tasks.Task:this
-6 (-5.45% of base) : 166067.dasm - Newtonsoft.Json.Bson.BsonDataWriter:CompleteAndCloseOutputAsync(System.Threading.CancellationToken):System.Threading.Tasks.Task:this
-6 (-5.45% of base) : 93077.dasm - Newtonsoft.Json.JsonTextReader:ParseConstructorAsync(System.Threading.CancellationToken):System.Threading.Tasks.Task:this
-6 (-5.45% of base) : 93084.dasm - Newtonsoft.Json.JsonTextReader:ReadNumberIntoBufferAsync(System.Threading.CancellationToken):System.Threading.Tasks.Task:this
-6 (-5.45% of base) : 93085.dasm - Newtonsoft.Json.JsonTextReader:ParseUnquotedPropertyAsync(System.Threading.CancellationToken):System.Threading.Tasks.Task:this
-6 (-5.45% of base) : 93086.dasm - Newtonsoft.Json.JsonTextReader:ReadNullCharAsync(System.Threading.CancellationToken):System.Threading.Tasks.Task`1[Boolean]:this
1107 total methods with Code Size differences (1107 improved, 0 regressed), 0 unchanged.
```
</details>
--------------------------------------------------------------------------------
Fixes dotnet#46023
BruceForstall
force-pushed
the
Fix45557Arm32
branch
from
85f8763 to
efb19b0
Compare
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp list |
|
/azp run runtime-coreclr jitstress |
|
Azure Pipelines successfully started running 1 pipeline(s). |
sandreenko
approved these changes
Apr 9, 2021
|
@sandreenko Thanks for the detailed look. |
|
The triggered job failures are all known. |
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
On arm32 only, in cases where we have STOREIND(GT_LCL_VAR_ADDR) of
a GC var, the JIT was generating (e.g.):
If this was a local variable birth, codegen would correctly process the lifetime. However,
we would have created an instrDesc that knows nothing about the local var, so can't emit
any GC transitions during emission.
It turns out this is also sub-optimal code; we don't need to create a byref of the
address of a local when we can encode the address directly in an addressing mode, as:
There was already code on arm64 to handle this case, by making the LCL_VAR_ADDR contained
and using the emitIns_S_R / emitIns_R_S functions, which create instrDescs with the necessary
local tracking info.
I added an assert, on both arm32 and arm64, that non-contained local var address nodes must
refer to untracked locals. Otherwise, we could potentially create addresses where we don't
properly track the GC info.
There are quite a few diffs, avoiding creating the local var address. There are several cases
I noted in the Microsoft.CodeAnalysis assemblies of bad GC info now fixed (this is where the
initial failure came from, that the Runtime_45557 test was derived from).
From libraries crossgen:
Detail diffs
Fixes #46023