-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[release/8.0-staging] Fix SysV first/second return register GC info mismatch #116208
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,8 +49,21 @@ ReturnKind GCInfo::getReturnKind() | |
case 1: | ||
return VarTypeToReturnKind(retTypeDesc.GetReturnRegType(0)); | ||
case 2: | ||
return GetStructReturnKind(VarTypeToReturnKind(retTypeDesc.GetReturnRegType(0)), | ||
VarTypeToReturnKind(retTypeDesc.GetReturnRegType(1))); | ||
{ | ||
var_types first = retTypeDesc.GetReturnRegType(0); | ||
var_types second = retTypeDesc.GetReturnRegType(1); | ||
#ifdef UNIX_AMD64_ABI | ||
if (varTypeUsesFloatReg(first)) | ||
{ | ||
// first does not consume an int register in this case so an obj/ref | ||
// in the second ReturnKind would actually be found in the first int reg. | ||
first = second; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be helpful to include a brief comment clarifying why setting 'first' to 'second' and then assigning TYP_UNDEF to 'second' is necessary under UNIX_AMD64_ABI. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||
second = TYP_UNDEF; | ||
} | ||
#endif // UNIX_AMD64_ABI | ||
return GetStructReturnKind(VarTypeToReturnKind(first), | ||
VarTypeToReturnKind(second)); | ||
} | ||
default: | ||
#ifdef DEBUG | ||
for (unsigned i = 0; i < regCount; i++) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1168,6 +1168,15 @@ ReturnKind MethodDesc::ParseReturnKindFromSig(INDEBUG(bool supportStringConstruc | |
regKinds[i] = RT_Scalar; | ||
} | ||
} | ||
|
||
if (eeClass->GetEightByteClassification(0) == SystemVClassificationTypeSSE) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider expanding the comment to clearly explain why swapping regKinds[0] and regKinds[1] corrects the GC info mismatch when the first eight-byte is classified as SSE. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||
{ | ||
// Skip over SSE types since they do not consume integer registers. | ||
// An obj/byref in the 2nd eight bytes will be in the first integer register. | ||
regKinds[0] = regKinds[1]; | ||
regKinds[1] = RT_Scalar; | ||
} | ||
|
||
ReturnKind structReturnKind = GetStructReturnKind(regKinds[0], regKinds[1]); | ||
return structReturnKind; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.Threading; | ||
using System.Collections.Generic; | ||
using System.Runtime.CompilerServices; | ||
using Xunit; | ||
|
||
public class Runtime_115815 | ||
{ | ||
[Fact] | ||
public static void TestEntryPoint() | ||
{ | ||
var destination = new KeyValuePair<Container, double>[1_000]; | ||
|
||
// loop to make this method fully interruptible + to get into OSR version | ||
for (int i = 0; i < destination.Length * 1000; i++) | ||
{ | ||
destination[i / 1000] = default; | ||
} | ||
|
||
for (int i = 0; i < 5; i++) | ||
{ | ||
for (int j = 0; j < destination.Length; j++) | ||
{ | ||
destination[j] = GetValue(j); | ||
} | ||
|
||
Thread.Sleep(10); | ||
} | ||
} | ||
|
||
[MethodImpl(MethodImplOptions.NoInlining)] | ||
private static KeyValuePair<Container, double> GetValue(int i) | ||
=> KeyValuePair.Create(new Container(i.ToString()), (double)i); | ||
|
||
private struct Container | ||
{ | ||
public string Name; | ||
public Container(string name) { this.Name = name; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<PropertyGroup> | ||
<Optimize>True</Optimize> | ||
</PropertyGroup> | ||
<ItemGroup> | ||
<Compile Include="$(MSBuildProjectName).cs" /> | ||
</ItemGroup> | ||
</Project> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a comment to explain the rationale for swapping retSize and secondRetSize in situations where the second return register is REG_INTRET, to clarify the underlying assumption about register allocation.
Copilot uses AI. Check for mistakes.