From d3eeb0ea697fe9f3e6ff362cd944fe3883a451f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Sowi=C5=84ski?= Date: Wed, 24 Jul 2024 23:18:09 +0200 Subject: [PATCH] [RISC-V] Fix passing float and uint arguments in VM (#105021) * Add tests * Fix passing float and uint arguments in VM * Change test lib name so it doesn't clash with managed DLL on Windows --- src/coreclr/vm/argdestination.h | 20 ++ src/coreclr/vm/callhelpers.cpp | 5 + src/coreclr/vm/invokeutil.cpp | 13 +- .../JIT/Directed/PrimitiveABI/CMakeLists.txt | 12 ++ .../JIT/Directed/PrimitiveABI/PrimitiveABI.c | 41 ++++ .../JIT/Directed/PrimitiveABI/PrimitiveABI.cs | 182 ++++++++++++++++++ .../Directed/PrimitiveABI/PrimitiveABI.csproj | 16 ++ 7 files changed, 281 insertions(+), 8 deletions(-) create mode 100644 src/tests/JIT/Directed/PrimitiveABI/CMakeLists.txt create mode 100644 src/tests/JIT/Directed/PrimitiveABI/PrimitiveABI.c create mode 100644 src/tests/JIT/Directed/PrimitiveABI/PrimitiveABI.cs create mode 100644 src/tests/JIT/Directed/PrimitiveABI/PrimitiveABI.csproj diff --git a/src/coreclr/vm/argdestination.h b/src/coreclr/vm/argdestination.h index 57df032664816..15f3e120ca1d3 100644 --- a/src/coreclr/vm/argdestination.h +++ b/src/coreclr/vm/argdestination.h @@ -173,6 +173,26 @@ class ArgDestination _ASSERTE(!"---------UNReachable-------LoongArch64/RISC-V64!!!"); } } + +#ifdef TARGET_RISCV64 + void CopySingleFloatToRegister(void* src) + { + void* dest = GetDestinationAddress(); + UINT32 value = *(UINT32*)src; + if (TransitionBlock::IsFloatArgumentRegisterOffset(m_offset)) + { + // NaN-box the floating register value or single-float instructions will treat it as NaN + *(UINT64*)dest = 0xffffffff00000000L | value; + } + else + { + // When a single float is passed according to integer calling convention + // (in integer register or on stack), the upper bits are not specified. + *(UINT32*)dest = value; + } + } +#endif // TARGET_RISCV64 + #endif // !DACCESS_COMPILE PTR_VOID GetStructGenRegDestinationAddress() diff --git a/src/coreclr/vm/callhelpers.cpp b/src/coreclr/vm/callhelpers.cpp index 3125c21ea3d13..25388ed164046 100644 --- a/src/coreclr/vm/callhelpers.cpp +++ b/src/coreclr/vm/callhelpers.cpp @@ -474,10 +474,15 @@ void MethodDescCallSite::CallTargetWorker(const ARG_SLOT *pArguments, ARG_SLOT * *((INT64*)pDest) = (INT16)pArguments[arg]; break; case 4: +#ifdef TARGET_RISCV64 + // RISC-V integer calling convention requires to sign-extend `uint` arguments as well + *((INT64*)pDest) = (INT32)pArguments[arg]; +#else // TARGET_LOONGARCH64 if (m_argIt.GetArgType() == ELEMENT_TYPE_U4) *((INT64*)pDest) = (UINT32)pArguments[arg]; else *((INT64*)pDest) = (INT32)pArguments[arg]; +#endif // TARGET_RISCV64 break; #else case 1: diff --git a/src/coreclr/vm/invokeutil.cpp b/src/coreclr/vm/invokeutil.cpp index cfec2c5259a5f..44f51dae7c7cf 100644 --- a/src/coreclr/vm/invokeutil.cpp +++ b/src/coreclr/vm/invokeutil.cpp @@ -139,7 +139,9 @@ void InvokeUtil::CopyArg(TypeHandle th, PVOID argRef, ArgDestination *argDest) { switch (type) { #ifdef TARGET_RISCV64 - // RISC-V call convention requires signed ints sign-extended (unsigned -- zero-extended) to register width + // RISC-V call convention requires integer scalars narrower than XLEN bits to be widened according to the sign + // of their type up to 32 bits, then sign-extended to XLEN bits. In practice it means type-extending all ints + // except `uint` which is sign-extended regardless. case ELEMENT_TYPE_BOOLEAN: case ELEMENT_TYPE_U1: _ASSERTE(argRef != NULL); @@ -164,18 +166,13 @@ void InvokeUtil::CopyArg(TypeHandle th, PVOID argRef, ArgDestination *argDest) { case ELEMENT_TYPE_R4: _ASSERTE(argRef != NULL); - // NaN-box the register value or single-float instructions will treat it as NaN - *(UINT64 *)pArgDst = 0xffffffff00000000L | *(UINT32 *)argRef; + argDest->CopySingleFloatToRegister(argRef); break; case ELEMENT_TYPE_I4: - _ASSERTE(argRef != NULL); - *(INT64 *)pArgDst = *(INT32 *)argRef; - break; - case ELEMENT_TYPE_U4: _ASSERTE(argRef != NULL); - *(UINT64 *)pArgDst = *(UINT32 *)argRef; + *(INT64 *)pArgDst = *(INT32 *)argRef; break; #else // !TARGET_RISCV64 diff --git a/src/tests/JIT/Directed/PrimitiveABI/CMakeLists.txt b/src/tests/JIT/Directed/PrimitiveABI/CMakeLists.txt new file mode 100644 index 0000000000000..20cb7991ed09a --- /dev/null +++ b/src/tests/JIT/Directed/PrimitiveABI/CMakeLists.txt @@ -0,0 +1,12 @@ +project (PrimitiveABINative) +include_directories(${INC_PLATFORM_DIR}) + +if(CLR_CMAKE_HOST_WIN32) + set_source_files_properties(PrimitiveABI.c PROPERTIES COMPILE_OPTIONS /TC) # compile as C +else() + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fvisibility=hidden -Oz") +endif() + +add_library (PrimitiveABINative SHARED PrimitiveABI.c) + +install (TARGETS PrimitiveABINative DESTINATION bin) diff --git a/src/tests/JIT/Directed/PrimitiveABI/PrimitiveABI.c b/src/tests/JIT/Directed/PrimitiveABI/PrimitiveABI.c new file mode 100644 index 0000000000000..0bfc2a3aa5f55 --- /dev/null +++ b/src/tests/JIT/Directed/PrimitiveABI/PrimitiveABI.c @@ -0,0 +1,41 @@ +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#include +#include +#include + +#ifdef _MSC_VER +#define DLLEXPORT __declspec(dllexport) +#else +#define DLLEXPORT __attribute__((visibility("default"))) +#endif // _MSC_VER + +DLLEXPORT int64_t Echo_ExtendedUint_RiscV(int a0, uint32_t a1) +{ + return (int32_t)a1; +} + +DLLEXPORT int64_t Echo_ExtendedUint_OnStack_RiscV( + int a0, int a1, int a2, int a3, int a4, int a5, int a6, int a7, uint32_t stack0) +{ + return (int32_t)stack0; +} + +DLLEXPORT double Echo_Float_RiscV(float fa0, float fa1) +{ + return fa1 + fa0; +} + +DLLEXPORT double Echo_Float_InIntegerReg_RiscV( + float fa0, float fa1, float fa2, float fa3, float fa4, float fa5, float fa6, float fa7, + float a0) +{ + return a0 + fa7; +} + +DLLEXPORT double Echo_Float_OnStack_RiscV( + float fa0, float fa1, float fa2, float fa3, float fa4, float fa5, float fa6, float fa7, + int a0, int a1, int a2, int a3, int a4, int a5, int a6, int a7, float stack0) +{ + return stack0 + fa7; +} diff --git a/src/tests/JIT/Directed/PrimitiveABI/PrimitiveABI.cs b/src/tests/JIT/Directed/PrimitiveABI/PrimitiveABI.cs new file mode 100644 index 0000000000000..8bcc34996d744 --- /dev/null +++ b/src/tests/JIT/Directed/PrimitiveABI/PrimitiveABI.cs @@ -0,0 +1,182 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; +using System.Runtime.CompilerServices; +using Xunit; + +public static class Program +{ +#region ExtendedUint_RiscVTests + [DllImport("PrimitiveABINative")] + public static extern long Echo_ExtendedUint_RiscV(int a0, uint a1); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static long Echo_ExtendedUint_RiscV_Managed(int a0, uint a1) => unchecked((int)a1); + + [Fact] + public static void Test_ExtendedUint_RiscV() + { + const uint arg = 0xB1ED0C1Eu; + const long ret = unchecked((int)arg); + long managed = Echo_ExtendedUint_RiscV_Managed(0, arg); + long native = Echo_ExtendedUint_RiscV(0, arg); + + Assert.Equal(ret, managed); + Assert.Equal(ret, native); + } + + [Fact] + public static void Test_ExtendedUint_ByReflection_RiscV() + { + const uint arg = 0xB1ED0C1Eu; + const long ret = unchecked((int)arg); + long managed = (long)typeof(Program).GetMethod("Echo_ExtendedUint_RiscV_Managed").Invoke( + null, new object[] {0, arg}); + long native = (long)typeof(Program).GetMethod("Echo_ExtendedUint_RiscV").Invoke( + null, new object[] {0, arg}); + + Assert.Equal(ret, managed); + Assert.Equal(ret, native); + } + + [DllImport("PrimitiveABINative")] + public static extern long Echo_ExtendedUint_OnStack_RiscV( + int a0, int a1, int a2, int a3, int a4, int a5, int a6, int a7, uint stack0); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static long Echo_ExtendedUint_OnStack_RiscV_Managed( + int a0, int a1, int a2, int a3, int a4, int a5, int a6, int a7, uint stack0) => unchecked((int)stack0); + + [Fact] + public static void Test_ExtendedUint_OnStack_RiscV() + { + const uint arg = 0xB1ED0C1Eu; + const long ret = unchecked((int)arg); + long managed = Echo_ExtendedUint_OnStack_RiscV_Managed(0, 0, 0, 0, 0, 0, 0, 0, arg); + long native = Echo_ExtendedUint_OnStack_RiscV(0, 0, 0, 0, 0, 0, 0, 0, arg); + + Assert.Equal(ret, managed); + Assert.Equal(ret, native); + } + + [Fact] + public static void Test_ExtendedUint_OnStack_ByReflection_RiscV() + { + const uint arg = 0xB1ED0C1Eu; + const long ret = unchecked((int)arg); + long managed = (long)typeof(Program).GetMethod("Echo_ExtendedUint_OnStack_RiscV_Managed").Invoke( + null, new object[] {0, 0, 0, 0, 0, 0, 0, 0, arg}); + long native = (long)typeof(Program).GetMethod("Echo_ExtendedUint_OnStack_RiscV").Invoke( + null, new object[] {0, 0, 0, 0, 0, 0, 0, 0, arg}); + + Assert.Equal(ret, managed); + Assert.Equal(ret, native); + } +#endregion + +#region Float_RiscVTests + [DllImport("PrimitiveABINative")] + public static extern double Echo_Float_RiscV(float fa0, float fa1); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static double Echo_Float_RiscV_Managed(float fa0, float fa1) => fa1; + + [Fact] + public static void Test_Float_RiscV() + { + const float arg = 3.14159f; + const double ret = 3.14159f; + double managed = Echo_Float_RiscV_Managed(0f, arg); + double native = Echo_Float_RiscV(0f, arg); + + Assert.Equal(ret, managed); + Assert.Equal(ret, native); + } + + [Fact] + public static void Test_Float_ByReflection_RiscV() + { + const float arg = 3.14159f; + const double ret = 3.14159f; + double managed = (double)typeof(Program).GetMethod("Echo_Float_RiscV_Managed").Invoke( + null, new object[] {0f, arg}); + double native = (double)typeof(Program).GetMethod("Echo_Float_RiscV").Invoke( + null, new object[] {0f, arg}); + + Assert.Equal(ret, managed); + Assert.Equal(ret, native); + } + + [DllImport("PrimitiveABINative")] + public static extern double Echo_Float_InIntegerReg_RiscV( + float fa0, float fa1, float fa2, float fa3, float fa4, float fa5, float fa6, float fa7, float a0); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static double Echo_Float_InIntegerReg_RiscV_Managed( + float fa0, float fa1, float fa2, float fa3, float fa4, float fa5, float fa6, float fa7, float a0) => a0; + + [Fact] + public static void Test_Float_InIntegerReg_RiscV() + { + const float arg = 3.14159f; + const double ret = 3.14159f; + double managed = Echo_Float_InIntegerReg_RiscV_Managed(0f, 0f, 0f, 0f, 0f, 0f, 0f, 0f, arg); + double native = Echo_Float_InIntegerReg_RiscV(0f, 0f, 0f, 0f, 0f, 0f, 0f, 0f, arg); + + Assert.Equal(ret, managed); + Assert.Equal(ret, native); + } + + [Fact] + public static void Test_Float_InIntegerReg_ByReflection_RiscV() + { + const float arg = 3.14159f; + const double ret = 3.14159f; + double managed = (double)typeof(Program).GetMethod("Echo_Float_InIntegerReg_RiscV_Managed").Invoke( + null, new object[] {0f, 0f, 0f, 0f, 0f, 0f, 0f, 0f, arg}); + double native = (double)typeof(Program).GetMethod("Echo_Float_InIntegerReg_RiscV").Invoke( + null, new object[] {0f, 0f, 0f, 0f, 0f, 0f, 0f, 0f, arg}); + + Assert.Equal(ret, managed); + Assert.Equal(ret, native); + } + + [DllImport("PrimitiveABINative")] + public static extern double Echo_Float_OnStack_RiscV( + float fa0, float fa1, float fa2, float fa3, float fa4, float fa5, float fa6, float fa7, + int a0, int a1, int a2, int a3, int a4, int a5, int a6, int a7, float stack0); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static double Echo_Float_OnStack_RiscV_Managed( + float fa0, float fa1, float fa2, float fa3, float fa4, float fa5, float fa6, float fa7, + int a0, int a1, int a2, int a3, int a4, int a5, int a6, int a7, float stack0) => stack0; + + [Fact] + public static void Test_Float_OnStack_RiscV() + { + const float arg = 3.14159f; + const double ret = 3.14159f; + double managed = Echo_Float_OnStack_RiscV_Managed(0f, 0f, 0f, 0f, 0f, 0f, 0f, 0f, 0, 0, 0, 0, 0, 0, 0, 0, arg); + double native = Echo_Float_OnStack_RiscV(0f, 0f, 0f, 0f, 0f, 0f, 0f, 0f, 0, 0, 0, 0, 0, 0, 0, 0, arg); + + Assert.Equal(ret, managed); + Assert.Equal(ret, native); + } + + [Fact] + public static void Test_Float_OnStack_ByReflection_RiscV() + { + const float arg = 3.14159f; + const double ret = 3.14159f; + double managed = (double)typeof(Program).GetMethod("Echo_Float_OnStack_RiscV_Managed").Invoke( + null, new object[] {0f, 0f, 0f, 0f, 0f, 0f, 0f, 0f, 0, 0, 0, 0, 0, 0, 0, 0, arg}); + double native = (double)typeof(Program).GetMethod("Echo_Float_OnStack_RiscV").Invoke( + null, new object[] {0f, 0f, 0f, 0f, 0f, 0f, 0f, 0f, 0, 0, 0, 0, 0, 0, 0, 0, arg}); + + Assert.Equal(ret, managed); + Assert.Equal(ret, native); + } +#endregion +} \ No newline at end of file diff --git a/src/tests/JIT/Directed/PrimitiveABI/PrimitiveABI.csproj b/src/tests/JIT/Directed/PrimitiveABI/PrimitiveABI.csproj new file mode 100644 index 0000000000000..a38b585f8185e --- /dev/null +++ b/src/tests/JIT/Directed/PrimitiveABI/PrimitiveABI.csproj @@ -0,0 +1,16 @@ + + + + true + + + PdbOnly + True + + + + + + + +