Skip to content

Convert Volatile to JIT intrinsics #88073

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Jun 29, 2023
Merged

Convert Volatile to JIT intrinsics #88073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
062056a
Convert Volatile to JIT intrinsics
MichalPetryka Jun 26, 2023
9bf057a
Fix 32bit, update comment
MichalPetryka Jun 26, 2023
c3a46c2
Add ulong too
MichalPetryka Jun 26, 2023
7ce4efe
Add ulong too (again)
MichalPetryka Jun 26, 2023
7d65cc8
Fix build
MichalPetryka Jun 27, 2023
a556bcc
Fix ILCompiler
MichalPetryka Jun 27, 2023
6887ec4
Update corelib.h
MichalPetryka Jun 27, 2023
250b836
Update importercalls.cpp
MichalPetryka Jun 27, 2023
b0c3e5d
Update importercalls.cpp
MichalPetryka Jun 27, 2023
5fe237e
Update importercalls.cpp
MichalPetryka Jun 27, 2023
1f04d55
Update importercalls.cpp
MichalPetryka Jun 27, 2023
071cfc9
Update importercalls.cpp
MichalPetryka Jun 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions src/coreclr/jit/importercalls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -9155,6 +9203,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;
}
}
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/namedintrinsiclist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 0 additions & 6 deletions src/coreclr/tools/Common/TypeSystem/IL/NativeAotILProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
106 changes: 0 additions & 106 deletions src/coreclr/tools/Common/TypeSystem/IL/Stubs/VolatileIntrinsics.cs
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
5 changes: 0 additions & 5 deletions src/coreclr/tools/aot/ILCompiler.ReadyToRun/IL/ReadyToRunILProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
23 changes: 0 additions & 23 deletions src/coreclr/vm/corelib.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
98 changes: 0 additions & 98 deletions src/coreclr/vm/jitinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6944,100 +6944,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)
{
Expand Down Expand Up @@ -7458,10 +7364,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);
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Private.CoreLib/src/System/Threading/Volatile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down