From 62899277e887f5343d221621e5a85c9650bb5ed2 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 2 Jun 2025 11:05:21 +0200 Subject: [PATCH 1/3] JIT: Fix SysV first/second return register GC info mismatch when generating calls --- src/coreclr/jit/codegenxarch.cpp | 13 ++++++ .../JitBlue/Runtime_115815/Runtime_115815.cs | 42 +++++++++++++++++++ .../Runtime_115815/Runtime_115815.csproj | 8 ++++ 3 files changed, 63 insertions(+) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_115815/Runtime_115815.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_115815/Runtime_115815.csproj diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 12764affdd92e4..0f899fbdf62d1c 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -6173,6 +6173,19 @@ void CodeGen::genCallInstruction(GenTreeCall* call X86_ARG(target_ssize_t stackA { retSize = emitTypeSize(retTypeDesc->GetReturnRegType(0)); secondRetSize = emitTypeSize(retTypeDesc->GetReturnRegType(1)); + + if (retTypeDesc->GetABIReturnReg(1) == REG_INTRET) + { + // If the second return register is REG_INTRET, then the first + // return is expected to be in a SIMD register. + // The emitter has hardcoded belief that retSize corresponds to + // REG_INTRET and secondRetSize to REG_INTRET_1, so fix up the + // situation here. + assert(!EA_IS_GCREF_OR_BYREF(retSize)); + retSize = secondRetSize; + secondRetSize = EA_UNKNOWN; + } + } else { diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_115815/Runtime_115815.cs b/src/tests/JIT/Regression/JitBlue/Runtime_115815/Runtime_115815.cs new file mode 100644 index 00000000000000..77f42fe7d49453 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_115815/Runtime_115815.cs @@ -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[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 GetValue(int i) + => KeyValuePair.Create(new Container(i.ToString()), (double)i); + + private struct Container + { + public string Name; + public Container(string name) { this.Name = name; } + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_115815/Runtime_115815.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_115815/Runtime_115815.csproj new file mode 100644 index 00000000000000..de6d5e08882e86 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_115815/Runtime_115815.csproj @@ -0,0 +1,8 @@ + + + True + + + + + From 4780d34e30413620f4791ea4528d5bcac58cf178 Mon Sep 17 00:00:00 2001 From: zengandrew <7494393+zengandrew@users.noreply.github.com> Date: Sun, 1 Jun 2025 15:28:41 +0000 Subject: [PATCH 2/3] Fix struct ReturnKind on SysV AMD64. On SysV AMD64, structs returned in a float and int reg pair were being classified as RT_Scalar_XX. This causes downstream consumers (e.g., HijackFrame::GcScanRoots) to look for obj/byref's in the second int reg. Per the ABI, however, the first float is passed through a float reg and the obj/byref is passed through the _first_ int reg. We now detect and fix this case by skipping the first float type in the ReturnKind encoding and moving the second type into the first. Fix #115815 --- src/coreclr/jit/gcencode.cpp | 17 +++++++++++++++-- src/coreclr/vm/method.cpp | 9 +++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/gcencode.cpp b/src/coreclr/jit/gcencode.cpp index 96409d4ce904f9..33a81931fb16dd 100644 --- a/src/coreclr/jit/gcencode.cpp +++ b/src/coreclr/jit/gcencode.cpp @@ -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; + second = TYP_UNDEF; + } +#endif // UNIX_AMD64_ABI + return GetStructReturnKind(VarTypeToReturnKind(first), + VarTypeToReturnKind(second)); + } default: #ifdef DEBUG for (unsigned i = 0; i < regCount; i++) diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 29910d6cb4c1c3..d0a5b289e2b2e2 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -1168,6 +1168,15 @@ ReturnKind MethodDesc::ParseReturnKindFromSig(INDEBUG(bool supportStringConstruc regKinds[i] = RT_Scalar; } } + + if (eeClass->GetEightByteClassification(0) == SystemVClassificationTypeSSE) + { + // 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; } From 39f65557e089fb4ecfcddf229486d7840a58feaa Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 2 Jun 2025 11:29:35 +0200 Subject: [PATCH 3/3] Nit --- src/coreclr/jit/codegenxarch.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 0f899fbdf62d1c..8f397d04ea6a74 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -6185,7 +6185,6 @@ void CodeGen::genCallInstruction(GenTreeCall* call X86_ARG(target_ssize_t stackA retSize = secondRetSize; secondRetSize = EA_UNKNOWN; } - } else {