Replace `Unsafe.As` with `Unsafe.Read{Write}Unaligned` by xtqqczze · Pull Request #113293 · dotnet/runtime

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

                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

                    // 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


                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

            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

            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

            {
                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));


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

            {
                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

                    {
                        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.

dotnet-policy-service · 2025-05-09T14:34:30Z

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

Doesn't depend upon implementation specific behaviour, i.e. that data is properly aligned. Split from dotnet#113293.

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 MichalStrehovsky, steveisok and vitek-karas as code owners March 8, 2025 14:45

ghost 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

Closed

EgorBo reviewed Mar 8, 2025

View reviewed changes

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

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

dotnet-policy-service Bot closed this May 9, 2025

xtqqczze added a commit to xtqqczze/dotnet-runtime that referenced this pull request Jun 7, 2025

Replace Unsafe.As in System.Array

92b351f

Doesn't depend upon implementation specific behaviour, i.e. that data is properly aligned. Split from dotnet#113293.

xtqqczze mentioned this pull request Jun 7, 2025

Use Unsafe.BitCast in System.Array #116404

Merged

github-actions Bot locked and limited conversation to collaborators Jun 9, 2025

Conversation

xtqqczze commented Mar 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teo-tsirpanis Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

EgorBo Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

EgorBo Mar 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EgorBo Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

EgorBo Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

EgorBo Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

EgorBo Mar 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EgorBo Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jkotas Mar 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkotas Mar 9, 2025

Choose a reason for hiding this comment

Uh oh!

jkotas left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EgorBo commented Mar 9, 2025

Uh oh!

dotnet-policy-service Bot commented Mar 31, 2025

Uh oh!

dotnet-policy-service Bot commented May 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

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