[API Proposal]: `Volatile.(Read|Write)Enum`

[colejohnson66]

Background and motivation

Normally, one would use Volatile.Read and Volatile.Write to handle interlocking of numeric values between tasks. Sometimes it's beneficial to use enums in these cases, but we are prevented from doing so due to limitations in the API. Currently, one must cast their enum values back and forth when reading and writing:

MyEnum nextState = default;
Task task = Task.Run(() =>
{
    ...
    Volatile.Write(ref nextState, success ? (int)MyEnum.SuccessState : (int)MyEnum.FailureState);
});

...

task.Wait();
MyEnum newState = (MyEnum)Volatile.Read(ref nextState);

Having overloads that take enums would be beneficial in these cases to avoid issues that occur when casting enum values (such as using incorrect numeric or enum types). For example, a ulong backed enum must be cast to either long or ulong lest you lose 32 bits of data. Casting to int above would throw away such data.

API Proposal

namespace System.Threading;

public static class Volatile
{
    public static TEnum ReadEnum<TEnum>(ref TEnum location)
        where TEnum : struct, Enum;
    public static void WriteEnum<TEnum>(ref TEnum location, TEnum value)
        where TEnum : struct, Enum;
}

In each function, sizeof(TEnum) would be used to determine the size of the enum. Then Unsafe.As and normal Volatile methods would be used to massage out the value. For example, an implementation of ReadEnum that only handled single-byte-backed enums would look like

public static TEnum ReadEnum<TEnum>(ref TEnum location)
    where TEnum : struct, Enum
{
    if (sizeof(TEnum) == sizeof(byte))
    {
        byte value = Volatile.Read(ref Unsafe.As<TEnum, byte>(ref location));
        return Unsafe.As<byte, TEnum>(ref value);
    }

    throw new NotSupportedException("Unsupported enum underlying type.");
}

API Usage

MyEnum nextState = default;
Task task = Task.Run(() =>
{
    ...
    Volatile.WriteEnum(ref nextState, success ? MyEnum.SuccessState : MyEnum.FailureState);
});

...

task.Wait();
MyEnum newState = Volatile.ReadEnum(ref nextState);

Alternative Designs

Alternatively, Volatile.Read<T>(ref T) and Volatile.Write<T>(ref T, T) can have their generic constraint removed and replaced with runtime checks (a la #99205 and #100842):

namespace System.Threading;

public static class Volatile
{
    public static T Read<T>(ref T location)
        /* where T : class */;
    public static void Write<T>(ref T location, T value)
        /* where T : class */;
}

If T : class, it operates the same. However, if T : struct, Enum, it would now operate as proposed. This is a non-breaking change, as it relaxes a generic constraint to allow previously non-compilable code.

It could be extended even further to work on any unmanaged struct that's eight bytes or fewer. In other words, Volatile.Read(ref myFourByteStruct) would now be legal.

This alternate design is probably undesirable. Despite being non-breaking, it can be risky. Say I have a four-byte struct and use Volatile.(Read|Write) on it. If, in the future, I change it to be, say, 12 bytes large, the code will still compile without warnings, but would always throw at runtime. If this alternate design is chosen, a source analyzer should be included. It would detect T types that are greater than eight bytes and raise a compile-time warning.

[colejohnson66]

Ideally, this would just be Read and Write, but because C# does not support overloading on generic constraints, this would interfere with the existing T Read<T>(ref T) where T:class and void Write<T>(ref T, T) where T:class methods

[DaZombieKiller]

These APIs can be implemented with sizeof(T) dispatch rather than typeof(T).GetEnumUnderlyingType():

static unsafe T ReadEnum<T>(ref T location)
    where T : struct, Enum
{
    T result;
#pragma warning disable 8500
    if (sizeof(T) == sizeof(byte))
        *(byte*)&result = Volatile.Read(ref Unsafe.As<T, byte>(ref location));
    else if (sizeof(T) == sizeof(ushort))
        *(ushort*)&result = Volatile.Read(ref Unsafe.As<T, ushort>(ref location));
    else if (sizeof(T) == sizeof(uint))
        *(uint*)&result = Volatile.Read(ref Unsafe.As<T, uint>(ref location));
    else if (sizeof(T) == sizeof(ulong))
        *(ulong*)&result = Volatile.Read(ref Unsafe.As<T, ulong>(ref location));
    else
        throw new NotSupportedException();
    return result;
#pragma warning restore 8500
}

Which provides much nicer codegen and doesn't rely on the very recent (.NET 8) GetEnumUnderlyingType optimization, so you get the nice codegen on .NET 7 (and probably Mono) too.

[colejohnson66]

Didn't even look at codegen, or consider optimizations. It was just a proof-of-concept, but you're right. However, your usage of pointer magic to write the result (instead of Unsafe.As to read it) makes no difference to codegen. The only important part is using sizeof. It's all optimized down to:

; bytes
movzx eax, byte ptr [rcx]
ret

; words
movzx eax, word ptr [rcx]
cwde
ret

; double-words
mov eax, [rcx]
ret

; quad-words
mov rax, [rcx]
ret

[DaZombieKiller]

Related https://github.com/dotnet/runtime/issues/65184

However, your usage of pointer magic to write the result (instead of Unsafe.As to read it) makes no difference to codegen.

Yeah, that was just an attempt to make the implementation smaller (should have a little less IL too).

[ghost]

Tagging subscribers to this area: @mangod9 See info in area-owners.md if you want to be subscribed.

Issue Details

---

### Background and motivation Normally, one would use `Volatile.Read` and `Volatile.Write` to handle interlocking of numeric values between tasks. Sometimes it's beneficial to use enums in these cases, but we are prevented from doing so due to limitations in the API. Currently, one must cast their enum values back and forth when reading and writing: ```csharp MyEnum nextState = default; Task task = Task.Run(() => { ... Volatile.Write(ref nextState, success ? (int)MyEnum.SuccessState : (int)MyEnum.FailureState); }); ... task.Wait(); MyEnum newState = (MyEnum)Volatile.Read(ref nextState); ``` Having overloads that take enums would be beneficial in these cases to avoid issues that occur when casting enum values (such as using incorrect numeric or enum types). For example, a `ulong` backed enum _must_ be cast to either `long` or `ulong` lest you lose 32 bits of data. Casting to `int` above would throw away such data. ### API Proposal ```csharp namespace System.Threading; public static class Volatile { public static TEnum ReadEnum(ref TEnum location) where TEnum : struct, Enum; public static void WriteEnum(ref TEnum location, TEnum value) where TEnum : struct, Enum; } ``` In each function, `typeof(TEnum).GetEnumUnderlyingType()` would be used to determine the size of the enum. Then `Unsafe.As` and normal `Volatile` methods would be used to massage out the value. For example, an implementation of `ReadEnum` that only handled single-byte-backed enums would look like ```csharp public static TEnum ReadEnum(ref TEnum location) where TEnum : struct, Enum { Type memberType = typeof(TEnum).GetEnumUnderlyingType(); if (memberType == typeof(byte) || memberType == typeof(sbyte)) { byte value = Volatile.Read(ref Unsafe.As(ref location)); return Unsafe.As(ref value); } throw new NotSupportedException("Unsupported enum underlying type."); } ``` ### API Usage ```csharp MyEnum nextState = default; Task task = Task.Run(() => { ... Volatile.WriteEnum(ref nextState, success ? MyEnum.SuccessState : MyEnum.FailureState); }); ... task.Wait(); MyEnum newState = Volatile.ReadEnum(ref nextState); ``` ### Alternative Designs Leave as is: use memory barriers or cast enums back and forth between numeric types. ### Risks _No response_

Author:	colejohnson66
Assignees:	-
Labels:	`api-suggestion`, `area-System.Threading`, `untriaged`, `needs-area-label`
Milestone:	-