Replace `Unsafe.As` with `Unsafe.Read{Write}Unaligned` #113293

xtqqczze · 2025-03-08T14:45:17Z

Doesn't depend upon implementation specific behaviour, i.e. that data is properly aligned.

For System.Private.CoreLib

Doesn't depend upon implementation specific behaviour, i.e. that the array data is properly aligned.

teo-tsirpanis · 2025-03-08T16:35:23Z

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs

@@ -600,7 +600,7 @@ internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, objec
                }
                else
                {
-                    Unsafe.As<byte, bool>(ref destPtr) = true;
+                    Unsafe.WriteUnaligned(ref destPtr, true);


Bools are one byte long so this is not needed; accessing them is always aligned.

Here and below, can you replace these byte <-> bool with ternary ops and run MihuBot?

EgorBo · 2025-03-08T17:36:51Z

src/coreclr/System.Private.CoreLib/src/System/RuntimeType.BoxCache.cs

@@ -69,7 +69,7 @@ private BoxCache(RuntimeType rt)
                {
                    // If the allocator is null, then we shouldn't allocate and make a copy,
                    // we should return the data as the object it currently is.
-                    return Unsafe.As<byte, object>(ref data);
+                    return Unsafe.ReadUnaligned<object>(ref data);


GC references are not expected to be not-aligned. We can probably do Unsafe.Read here with a debug assert checking the alignment.

Same in other places

I think we can introduce an internal helper in Unsafe: ReadAligned that checks alignment in Debug.Assert

EgorBo · 2025-03-08T17:40:44Z

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Enum.NativeAot.cs

                    return true;

                case EETypeElementType.Byte:
-                    result = (ulong)(long)Unsafe.As<byte, byte>(ref pValue);
+                    result = (ulong)(long)Unsafe.ReadUnaligned<byte>(ref pValue);


pValue is already byte, it doesn't need Unsafe. Same for SByte

EgorBo · 2025-03-08T17:42:01Z

src/libraries/System.Private.CoreLib/src/System/Enum.cs

@@ -1166,21 +1166,21 @@ internal object GetValue()
            ref byte data = ref this.GetRawData();
            return InternalGetCorElementType() switch
            {
-                CorElementType.ELEMENT_TYPE_I1 => Unsafe.As<byte, sbyte>(ref data),
+                CorElementType.ELEMENT_TYPE_I1 => Unsafe.ReadUnaligned<sbyte>(ref data),


same here: sbyte is read from byte with unsafe

EgorBo · 2025-03-08T17:42:22Z

src/libraries/System.Private.CoreLib/src/System/Enum.cs

@@ -1256,21 +1256,21 @@ public override int GetHashCode()
            ref byte data = ref this.GetRawData();
            return InternalGetCorElementType() switch
            {
-                CorElementType.ELEMENT_TYPE_I1 => Unsafe.As<byte, sbyte>(ref data).GetHashCode(),
+                CorElementType.ELEMENT_TYPE_I1 => Unsafe.ReadUnaligned<sbyte>(ref data).GetHashCode(),


EgorBo · 2025-03-08T17:42:38Z

src/libraries/System.Private.CoreLib/src/System/Enum.cs

@@ -1294,35 +1294,35 @@ public int CompareTo(object? target)
            switch (InternalGetCorElementType())
            {
                case CorElementType.ELEMENT_TYPE_I1:
-                    return Unsafe.As<byte, sbyte>(ref pThisValue).CompareTo(Unsafe.As<byte, sbyte>(ref pTargetValue));
+                    return Unsafe.ReadUnaligned<sbyte>(ref pThisValue).CompareTo(Unsafe.ReadUnaligned<sbyte>(ref pTargetValue));


src/libraries/System.Private.CoreLib/src/System/Guid.cs

jkotas · 2025-03-09T00:42:47Z

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Enum.NativeAot.cs

@@ -101,39 +101,39 @@ internal static unsafe bool TryGetUnboxedValueOfEnumOrInteger(object value, out
            switch (elementType)
            {
                case EETypeElementType.Char:
-                    result = (ulong)(long)Unsafe.As<byte, char>(ref pValue);
+                    result = (ulong)(long)Unsafe.ReadUnaligned<char>(ref pValue);


The pValue is expected to be aligned properly. Changes in this method are unnecessary deoptimization.

jkotas · 2025-03-09T00:45:19Z

src/coreclr/nativeaot/System.Private.CoreLib/src/System/InvokeUtils.cs

@@ -161,23 +161,23 @@ internal static void PrimitiveWiden(EETypeElementType dstType, EETypeElementType
                    switch (dstType)
                    {
                        case EETypeElementType.Single:
-                            Unsafe.As<byte, float>(ref dstValue) = srcValue;
+                            Unsafe.WriteUnaligned(ref dstValue, (float)srcValue);


Same here. All changes in this method are unnecessary deoptimizations.

jkotas

Many of the changes are unnecessary deoptimizations (I have not commented on all of them). You need to check what the surrounding code does and only fix this in places where unaligned access is actually possible.

EgorBo · 2025-03-09T22:20:05Z

@MihuBot

dotnet-policy-service · 2025-03-31T06:59:44Z

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Replace Unsafe.As with Unsafe.ReadUnaligned for loads

d03bb08

Doesn't depend upon implementation specific behaviour, i.e. that the array data is properly aligned.

xtqqczze requested review from steveisok, vitek-karas and MichalStrehovsky as code owners March 8, 2025 14:45

dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Mar 8, 2025

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 8, 2025

teo-tsirpanis reviewed Mar 8, 2025

View reviewed changes

xtqqczze changed the title ~~Replace Unsafe.As with Unsafe.ReadUnaligned for loads~~ Replace Unsafe.As with Unsafe.Read{Write}Unaligned Mar 8, 2025

This was referenced Mar 8, 2025

NuGet failing with Response status code does not indicate success: 503 (Service Unavailable) dotnet/arcade#11723

Open

[wasm] at System.Net.Http.BrowserHttpInterop.CancellationHelper #112533

Open

EgorBo reviewed Mar 8, 2025

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Guid.cs Show resolved Hide resolved

jkotas reviewed Mar 9, 2025

View reviewed changes

jkotas requested changes Mar 9, 2025

View reviewed changes

MihuBot mentioned this pull request Mar 9, 2025

[JitDiff X64] [xtqqczze] Replace Unsafe.As with Unsafe.Read{Write}Unaligned MihuBot/runtime-utils#1044

Open

marek-safar added the area-System.Runtime label Mar 31, 2025

jkotas marked this pull request as draft March 31, 2025 19:30

am11 removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Apr 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace `Unsafe.As` with `Unsafe.Read{Write}Unaligned` #113293

Replace `Unsafe.As` with `Unsafe.Read{Write}Unaligned` #113293

xtqqczze commented Mar 8, 2025 •

edited

Loading

teo-tsirpanis Mar 8, 2025

EgorBo Mar 8, 2025

EgorBo Mar 8, 2025 •

edited

Loading

EgorBo Mar 8, 2025

EgorBo Mar 8, 2025

EgorBo Mar 8, 2025

EgorBo Mar 8, 2025 •

edited

Loading

EgorBo Mar 8, 2025

jkotas Mar 9, 2025 •

edited

Loading

jkotas Mar 9, 2025

jkotas left a comment •

edited

Loading

EgorBo commented Mar 9, 2025

dotnet-policy-service bot commented Mar 31, 2025

Replace Unsafe.As with Unsafe.Read{Write}Unaligned #113293

Are you sure you want to change the base?

Replace Unsafe.As with Unsafe.Read{Write}Unaligned #113293

Conversation

xtqqczze commented Mar 8, 2025 • edited Loading

teo-tsirpanis Mar 8, 2025

Choose a reason for hiding this comment

EgorBo Mar 8, 2025

Choose a reason for hiding this comment

EgorBo Mar 8, 2025 • edited Loading

Choose a reason for hiding this comment

EgorBo Mar 8, 2025

Choose a reason for hiding this comment

EgorBo Mar 8, 2025

Choose a reason for hiding this comment

EgorBo Mar 8, 2025

Choose a reason for hiding this comment

EgorBo Mar 8, 2025 • edited Loading

Choose a reason for hiding this comment

EgorBo Mar 8, 2025

Choose a reason for hiding this comment

jkotas Mar 9, 2025 • edited Loading

Choose a reason for hiding this comment

jkotas Mar 9, 2025

Choose a reason for hiding this comment

jkotas left a comment • edited Loading

Choose a reason for hiding this comment

EgorBo commented Mar 9, 2025

dotnet-policy-service bot commented Mar 31, 2025

Replace `Unsafe.As` with `Unsafe.Read{Write}Unaligned` #113293

Replace `Unsafe.As` with `Unsafe.Read{Write}Unaligned` #113293

xtqqczze commented Mar 8, 2025 •

edited

Loading

EgorBo Mar 8, 2025 •

edited

Loading

EgorBo Mar 8, 2025 •

edited

Loading

jkotas Mar 9, 2025 •

edited

Loading

jkotas left a comment •

edited

Loading