Uh oh!
There was an error while loading. Please reload this page.
Share MemoryMarshal.GetArrayDataReference between CoreCLR and NativeAOT - #132057
Conversation
MemoryMarshal.CoreCLR.cs and MemoryMarshal.NativeAot.cs were identical apart from a single line, and only because NativeAOT declares GetMethodTable as an extension method: RuntimeHelpers.GetMethodTable(array)->BaseSize // CoreCLR array.GetMethodTable()->BaseSize // NativeAOT Both spellings resolve to the same method, so use the static form and move the file to the shared partition as MemoryMarshal.GetArrayDataReference.cs. Mono is excluded because it uses a different array layout and keeps MemoryMarshal.Mono.cs. No behavior change: CoreCLR and NativeAOT now emit byte-identical IL for both GetArrayDataReference overloads (7 and 39 IL bytes), matching what each flavor produced before. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 820f1340-6846-4a20-9510-ebb3254a3d19
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
There was a problem hiding this comment.
Pull request overview
This PR consolidates MemoryMarshal.GetArrayDataReference into the shared System.Private.CoreLib source set so it can be used by both CoreCLR and NativeAOT, while continuing to exclude Mono due to its differing array layout.
Changes:
- Added
MemoryMarshal.GetArrayDataReference.cstoSystem.Private.CoreLib.Shared.projitemsfor all runtimes except Mono. - Removed the now-redundant runtime-specific compile inclusions for CoreCLR and NativeAOT (and deleted the NativeAOT-specific implementation file).
- Updated an in-file comment to reference both CoreCLR and NativeAOT
RuntimeHelperslayout documentation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.GetArrayDataReference.cs | Updates the layout-reference comment to reflect the shared implementation used by both CoreCLR and NativeAOT. |
| src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems | Includes the shared MemoryMarshal.GetArrayDataReference.cs for non-Mono builds. |
| src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj | Removes the CoreCLR-specific MemoryMarshal.CoreCLR.cs compile include now that the code is shared. |
| src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.NativeAot.cs | Deletes the NativeAOT-specific implementation that is replaced by the shared file. |
| src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj | Removes the compile include for the deleted MemoryMarshal.NativeAot.cs. |
Uh oh!
There was an error while loading. Please reload this page.
Address PR feedback: rather than a separate file conditioned on FeatureMono, put the shared implementation directly in MemoryMarshal.cs guarded by #if !MONO, so the guard can simply be deleted once Mono no longer needs its own copy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 820f1340-6846-4a20-9510-ebb3254a3d19
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs:10
using System.Runtime.Versioning;is only needed for[NonVersionable]within#if !MONO. When compiling withMONOdefined, that using becomes unused and can trigger CS8019 (often treated as an error in CoreLib builds). Make the using conditional so Mono builds don't see an unused directive.
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using System.Runtime.Versioning;
Uh oh!
There was an error while loading. Please reload this page.
…OT (dotnet#132057) Mono is excluded because it uses a different array layout --------- Co-authored-by: EgorBo <egorbo@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 820f1340-6846-4a20-9510-ebb3254a3d19
Mono is excluded because it uses a different array layout