Convert Volatile to JIT intrinsics (#88073)

src/coreclr/jit/importercalls.cpp

@@ -2589,6 +2589,9 @@ GenTree* Compiler::impIntrinsic(GenTree*                newobjThis,
             case NI_System_Threading_Interlocked_MemoryBarrier:
             case NI_System_Threading_Interlocked_ReadMemoryBarrier:
 
+            case NI_System_Threading_Volatile_Read:
+            case NI_System_Threading_Volatile_Write:
+
                 betterToExpand = true;
                 break;
 
@@ -3842,6 +3845,51 @@ GenTree* Compiler::impIntrinsic(GenTree*                newobjThis,
                 break;
             }
 
+            case NI_System_Threading_Volatile_Read:
+            {
+                assert((sig->sigInst.methInstCount == 0) || (sig->sigInst.methInstCount == 1));
+                var_types retType = sig->sigInst.methInstCount == 0 ? JITtype2varType(sig->retType) : TYP_REF;
+#ifndef TARGET_64BIT
+                if ((retType == TYP_LONG) || (retType == TYP_DOUBLE))
+                {
+                    break;
+                }
+#endif // !TARGET_64BIT
+                assert(retType == TYP_REF || impIsPrimitive(sig->retType));
+                retNode = gtNewIndir(retType, impPopStack().val, GTF_IND_VOLATILE);
+                break;
+            }
+
+            case NI_System_Threading_Volatile_Write:
+            {
+                var_types type = TYP_REF;
+                if (sig->sigInst.methInstCount == 0)
+                {
+                    CORINFO_CLASS_HANDLE typeHnd = nullptr;
+                    CorInfoType          jitType =
+                        strip(info.compCompHnd->getArgType(sig, info.compCompHnd->getArgNext(sig->args), &typeHnd));
+                    assert(impIsPrimitive(jitType));
+                    type = JITtype2varType(jitType);
+#ifndef TARGET_64BIT
+                    if ((type == TYP_LONG) || (type == TYP_DOUBLE))
+                    {
+                        break;
+                    }
+#endif // !TARGET_64BIT
+                }
+                else
+                {
+                    assert(sig->sigInst.methInstCount == 1);
+                    assert(!eeIsValueClass(sig->sigInst.methInst[0]));
+                }
+
+                GenTree* value = impPopStack().val;
+                GenTree* addr  = impPopStack().val;
+
+                retNode = gtNewStoreIndNode(type, addr, value, GTF_IND_VOLATILE);
+                break;
+            }
+
             default:
                 break;
         }
@@ -9158,6 +9206,17 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
                         result = NI_System_Threading_Thread_get_ManagedThreadId;
                     }
                 }
+                else if (strcmp(className, "Volatile") == 0)
+                {
+                    if (strcmp(methodName, "Read") == 0)
+                    {
+                        result = NI_System_Threading_Volatile_Read;
+                    }
+                    else if (strcmp(methodName, "Write") == 0)
+                    {
+                        result = NI_System_Threading_Volatile_Write;
+                    }
+                }
             }
         }
     }

src/coreclr/jit/namedintrinsiclist.h

@@ -68,6 +68,8 @@ enum NamedIntrinsic : unsigned short
     NI_System_GC_KeepAlive,
     NI_System_Threading_Thread_get_CurrentThread,
     NI_System_Threading_Thread_get_ManagedThreadId,
+    NI_System_Threading_Volatile_Read,
+    NI_System_Threading_Volatile_Write,
     NI_System_Type_get_IsEnum,
     NI_System_Type_GetEnumUnderlyingType,
     NI_System_Type_get_IsValueType,

src/coreclr/tools/Common/TypeSystem/IL/NativeAotILProvider.cs

@@ -58,12 +58,6 @@ private static MethodIL TryGetIntrinsicMethodIL(MethodDesc method)
                             return UnsafeIntrinsics.EmitIL(method);
                     }
                     break;
-                case "Volatile":
-                    {
-                        if (owningType.Namespace == "System.Threading")
-                            return VolatileIntrinsics.EmitIL(method);
-                    }
-                    break;
                 case "Debug":
                     {
                         if (owningType.Namespace == "System.Diagnostics" && method.Name == "DebugBreak")

src/coreclr/tools/Common/TypeSystem/IL/Stubs/VolatileIntrinsics.cs

-// 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 Internal.TypeSystem;
-
-using Debug = System.Diagnostics.Debug;
-
-namespace Internal.IL.Stubs
-{
-    /// <summary>
-    /// Provides method bodies for System.Threading.Volatile intrinsics.
-    /// </summary>
-    public static class VolatileIntrinsics
-    {
-        public static MethodIL EmitIL(MethodDesc method)
-        {
-            Debug.Assert(((MetadataType)method.OwningType).Name == "Volatile");
-
-            bool isRead = method.Name == "Read";
-            if (!isRead && method.Name != "Write")
-                return null;
-
-            // All interesting methods have a signature that starts with `ref location`
-            if (method.Signature.Length == 0 || !method.Signature[0].IsByRef)
-                return null;
-
-            ILOpcode opcode;
-            switch (((ByRefType)method.Signature[0]).ParameterType.Category)
-            {
-                case TypeFlags.SignatureMethodVariable:
-                    opcode = isRead ? ILOpcode.ldind_ref : ILOpcode.stind_ref;
-                    break;
-
-                case TypeFlags.Boolean:
-                case TypeFlags.SByte:
-                    opcode = isRead ? ILOpcode.ldind_i1 : ILOpcode.stind_i1;
-                    break;
-                case TypeFlags.Byte:
-                    opcode = isRead ? ILOpcode.ldind_u1 : ILOpcode.stind_i1;
-                    break;
-                case TypeFlags.Int16:
-                    opcode = isRead ? ILOpcode.ldind_i2 : ILOpcode.stind_i2;
-                    break;
-                case TypeFlags.UInt16:
-                    opcode = isRead ? ILOpcode.ldind_u2 : ILOpcode.stind_i2;
-                    break;
-                case TypeFlags.Int32:
-                    opcode = isRead ? ILOpcode.ldind_i4 : ILOpcode.stind_i4;
-                    break;
-                case TypeFlags.UInt32:
-                    opcode = isRead ? ILOpcode.ldind_u4 : ILOpcode.stind_i4;
-                    break;
-                case TypeFlags.IntPtr:
-                case TypeFlags.UIntPtr:
-                    opcode = isRead ? ILOpcode.ldind_i : ILOpcode.stind_i;
-                    break;
-                case TypeFlags.Single:
-                    opcode = isRead ? ILOpcode.ldind_r4 : ILOpcode.stind_r4;
-                    break;
-
-                //
-                // Ordinary volatile loads and stores only guarantee atomicity for pointer-sized (or smaller) data.
-                // So, on 32-bit platforms we must use Interlocked operations instead for the 64-bit types.
-                // The implementation in CoreLib already does this, so we will only substitute a new
-                // IL body if we're running on a 64-bit platform.
-                //
-                case TypeFlags.Int64 when method.Context.Target.PointerSize == 8:
-                case TypeFlags.UInt64 when method.Context.Target.PointerSize == 8:
-                    opcode = isRead ? ILOpcode.ldind_i8 : ILOpcode.stind_i8;
-                    break;
-                case TypeFlags.Double when method.Context.Target.PointerSize == 8:
-                    opcode = isRead ? ILOpcode.ldind_r8 : ILOpcode.stind_r8;
-                    break;
-                default:
-                    return null;
-            }
-
-            byte[] ilBytes;
-            if (isRead)
-            {
-                ilBytes = new byte[]
-                {
-                    (byte)ILOpcode.ldarg_0,
-                    (byte)ILOpcode.prefix1, unchecked((byte)ILOpcode.volatile_),
-                    (byte)opcode,
-                    (byte)ILOpcode.ret
-                };
-            }
-            else
-            {
-                ilBytes = new byte[]
-                {
-                    (byte)ILOpcode.ldarg_0,
-                    (byte)ILOpcode.ldarg_1,
-                    (byte)ILOpcode.prefix1, unchecked((byte)ILOpcode.volatile_),
-                    (byte)opcode,
-                    (byte)ILOpcode.ret
-                };
-            }
-
-            return new ILStubMethodIL(method, ilBytes, Array.Empty<LocalVariableDefinition>(), null);
-        }
-    }
-}

src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj

@@ -665,9 +665,6 @@
     <Compile Include="..\..\Common\TypeSystem\IL\Stubs\UnsafeIntrinsics.cs">
       <Link>IL\Stubs\UnsafeIntrinsics.cs</Link>
     </Compile>
-    <Compile Include="..\..\Common\TypeSystem\IL\Stubs\VolatileIntrinsics.cs">
-      <Link>IL\Stubs\VolatileIntrinsics.cs</Link>
-    </Compile>
     <Compile Include="..\..\Common\JitInterface\CorInfoInstructionSet.cs">
       <Link>JitInterface\CorInfoInstructionSet.cs</Link>
     </Compile>

src/coreclr/tools/aot/ILCompiler.ReadyToRun/IL/ReadyToRunILProvider.cs

@@ -85,11 +85,6 @@ private MethodIL TryGetIntrinsicMethodIL(MethodDesc method)
                 return UnsafeIntrinsics.EmitIL(method);
             }
 
-            if (mdType.Name == "Volatile" && mdType.Namespace == "System.Threading")
-            {
-                return VolatileIntrinsics.EmitIL(method);
-            }
-
             if (mdType.Name == "Interlocked" && mdType.Namespace == "System.Threading")
             {
                 return InterlockedIntrinsics.EmitIL(_compilationModuleGroup, method);

src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj

@@ -49,7 +49,6 @@
     <Compile Include="..\..\Common\TypeSystem\IL\Stubs\InterlockedIntrinsics.cs" Link="IL\Stubs\InterlockedIntrinsics.cs" />
     <Compile Include="..\..\Common\TypeSystem\IL\Stubs\RuntimeHelpersIntrinsics.cs" Link="IL\Stubs\RuntimeHelpersIntrinsics.cs" />
     <Compile Include="..\..\Common\TypeSystem\IL\Stubs\UnsafeIntrinsics.cs" Link="IL\Stubs\UnsafeIntrinsics.cs" />
-    <Compile Include="..\..\Common\TypeSystem\IL\Stubs\VolatileIntrinsics.cs" Link="IL\Stubs\VolatileIntrinsics.cs" />
     <Compile Include="..\..\Common\TypeSystem\IL\Stubs\PInvokeILCodeStreams.cs" Link="IL\Stubs\PInvokeILCodeStreams.cs" />
     <Compile Include="..\..\Common\TypeSystem\Interop\IL\Marshaller.cs" Link="Interop\IL\Marshaller.cs" />
     <Compile Include="..\..\Common\TypeSystem\Interop\IL\MarshallerKind.cs" Link="Interop\IL\MarshallerKind.cs" />

src/coreclr/vm/corelib.h

@@ -615,29 +615,6 @@ END_ILLINK_FEATURE_SWITCH()
 DEFINE_CLASS(MONITOR,               Threading,              Monitor)
 DEFINE_METHOD(MONITOR,              ENTER,                  Enter,                      SM_Obj_RetVoid)
 
-DEFINE_CLASS(VOLATILE, Threading, Volatile)
-
-#define DEFINE_VOLATILE_METHODS(methodType, paramType) \
-    DEFINE_METHOD(VOLATILE, READ_##paramType, Read, methodType##_Ref##paramType##_Ret##paramType) \
-    DEFINE_METHOD(VOLATILE, WRITE_##paramType, Write, methodType##_Ref##paramType##_##paramType)
-
-DEFINE_VOLATILE_METHODS(SM,Bool)
-DEFINE_VOLATILE_METHODS(SM,SByt)
-DEFINE_VOLATILE_METHODS(SM,Byte)
-DEFINE_VOLATILE_METHODS(SM,Shrt)
-DEFINE_VOLATILE_METHODS(SM,UShrt)
-DEFINE_VOLATILE_METHODS(SM,Int)
-DEFINE_VOLATILE_METHODS(SM,UInt)
-DEFINE_VOLATILE_METHODS(SM,Long)
-DEFINE_VOLATILE_METHODS(SM,ULong)
-DEFINE_VOLATILE_METHODS(SM,IntPtr)
-DEFINE_VOLATILE_METHODS(SM,UIntPtr)
-DEFINE_VOLATILE_METHODS(SM,Flt)
-DEFINE_VOLATILE_METHODS(SM,Dbl)
-DEFINE_VOLATILE_METHODS(GM,T)
-
-#undef DEFINE_VOLATILE_METHODS
-
 DEFINE_CLASS(PARAMETER,             Reflection,             ParameterInfo)
 
 DEFINE_CLASS(PARAMETER_MODIFIER,    Reflection,             ParameterModifier)

src/coreclr/vm/jitinterface.cpp

@@ -7104,100 +7104,6 @@ bool getILIntrinsicImplementationForUnsafe(MethodDesc * ftn,
     return false;
 }
 
-bool getILIntrinsicImplementationForVolatile(MethodDesc * ftn,
-                                             CORINFO_METHOD_INFO * methInfo)
-{
-    STANDARD_VM_CONTRACT;
-
-    //
-    // This replaces the implementations of Volatile.* in CoreLib with more efficient ones.
-    // We do this because we cannot otherwise express these in C#.  What we *want* to do is
-    // to treat the byref args to these methods as "volatile."  In pseudo-C#, this would look
-    // like:
-    //
-    //   int Read(ref volatile int location)
-    //   {
-    //       return location;
-    //   }
-    //
-    // However, C# does not yet provide a way to declare a byref as "volatile."  So instead,
-    // we substitute raw IL bodies for these methods that use the correct volatile instructions.
-    //
-
-    _ASSERTE(CoreLibBinder::IsClass(ftn->GetMethodTable(), CLASS__VOLATILE));
-
-    const size_t VolatileMethodBodySize = 6;
-
-    struct VolatileMethodImpl
-    {
-        BinderMethodID methodId;
-        BYTE body[VolatileMethodBodySize];
-    };
-
-#define VOLATILE_IMPL(type, loadinst, storeinst) \
-    { \
-        METHOD__VOLATILE__READ_##type, \
-        { \
-            CEE_LDARG_0, \
-            CEE_PREFIX1, (CEE_VOLATILE & 0xFF), \
-            loadinst, \
-            CEE_NOP, /*pad to VolatileMethodBodySize bytes*/ \
-            CEE_RET \
-        } \
-    }, \
-    { \
-        METHOD__VOLATILE__WRITE_##type, \
-        { \
-            CEE_LDARG_0, \
-            CEE_LDARG_1, \
-            CEE_PREFIX1, (CEE_VOLATILE & 0xFF), \
-            storeinst, \
-            CEE_RET \
-        } \
-    },
-
-    static const VolatileMethodImpl volatileImpls[] =
-    {
-        VOLATILE_IMPL(T,       CEE_LDIND_REF, CEE_STIND_REF)
-        VOLATILE_IMPL(Bool,    CEE_LDIND_I1,  CEE_STIND_I1)
-        VOLATILE_IMPL(Int,     CEE_LDIND_I4,  CEE_STIND_I4)
-        VOLATILE_IMPL(IntPtr,  CEE_LDIND_I,   CEE_STIND_I)
-        VOLATILE_IMPL(UInt,    CEE_LDIND_U4,  CEE_STIND_I4)
-        VOLATILE_IMPL(UIntPtr, CEE_LDIND_I,   CEE_STIND_I)
-        VOLATILE_IMPL(SByt,    CEE_LDIND_I1,  CEE_STIND_I1)
-        VOLATILE_IMPL(Byte,    CEE_LDIND_U1,  CEE_STIND_I1)
-        VOLATILE_IMPL(Shrt,    CEE_LDIND_I2,  CEE_STIND_I2)
-        VOLATILE_IMPL(UShrt,   CEE_LDIND_U2,  CEE_STIND_I2)
-        VOLATILE_IMPL(Flt,     CEE_LDIND_R4,  CEE_STIND_R4)
-
-        //
-        // Ordinary volatile loads and stores only guarantee atomicity for pointer-sized (or smaller) data.
-        // So, on 32-bit platforms we must use Interlocked operations instead for the 64-bit types.
-        // The implementation in CoreLib already does this, so we will only substitute a new
-        // IL body if we're running on a 64-bit platform.
-        //
-        IN_TARGET_64BIT(VOLATILE_IMPL(Long,  CEE_LDIND_I8, CEE_STIND_I8))
-        IN_TARGET_64BIT(VOLATILE_IMPL(ULong, CEE_LDIND_I8, CEE_STIND_I8))
-        IN_TARGET_64BIT(VOLATILE_IMPL(Dbl,   CEE_LDIND_R8, CEE_STIND_R8))
-    };
-
-    mdMethodDef md = ftn->GetMemberDef();
-    for (unsigned i = 0; i < ARRAY_SIZE(volatileImpls); i++)
-    {
-        if (md == CoreLibBinder::GetMethod(volatileImpls[i].methodId)->GetMemberDef())
-        {
-            methInfo->ILCode = const_cast<BYTE*>(volatileImpls[i].body);
-            methInfo->ILCodeSize = VolatileMethodBodySize;
-            methInfo->maxStack = 2;
-            methInfo->EHcount = 0;
-            methInfo->options = (CorInfoOptions)0;
-            return true;
-        }
-    }
-
-    return false;
-}
-
 bool getILIntrinsicImplementationForInterlocked(MethodDesc * ftn,
                                                 CORINFO_METHOD_INFO * methInfo)
 {
@@ -7618,10 +7524,6 @@ static void getMethodInfoHelper(
             {
                 fILIntrinsic = getILIntrinsicImplementationForInterlocked(ftn, methInfo);
             }
-            else if (CoreLibBinder::IsClass(pMT, CLASS__VOLATILE))
-            {
-                fILIntrinsic = getILIntrinsicImplementationForVolatile(ftn, methInfo);
-            }
             else if (CoreLibBinder::IsClass(pMT, CLASS__RUNTIME_HELPERS))
             {
                 fILIntrinsic = getILIntrinsicImplementationForRuntimeHelpers(ftn, methInfo);

src/libraries/System.Private.CoreLib/src/System/Threading/Volatile.cs

@@ -10,8 +10,8 @@ namespace System.Threading
     /// <summary>Methods for accessing memory with volatile semantics.</summary>
     public static unsafe class Volatile
     {
-        // The VM may replace these implementations with more efficient ones in some cases.
-        // In coreclr, for example, see getILIntrinsicImplementationForVolatile() in jitinterface.cpp.
+        // The runtime may replace these implementations with more efficient ones in some cases.
+        // In coreclr, for example, see importercalls.cpp.
 
         #region Boolean
         private struct VolatileBoolean { public volatile bool Value; }