search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.15k stars 4.71k forks source link

Duplicate assembly macros #64253

Open am11 opened 2 years ago

am11 commented 2 years ago

In CoreRT, we copied a few assembly macros from CoreCLR repo. Those macros were later modified in coreclr then in runtime repos. Now that NativeAOT and PAL code exist in the same repo, is it feasible to deduplicate that code? 

/runtime $ git ls-files ':/src/coreclr/pal/*unixasmmacros*.inc'
src/coreclr/pal/inc/unixasmmacros.inc
src/coreclr/pal/inc/unixasmmacrosamd64.inc
src/coreclr/pal/inc/unixasmmacrosarm.inc
src/coreclr/pal/inc/unixasmmacrosarm64.inc
src/coreclr/pal/inc/unixasmmacross390x.inc
src/coreclr/pal/inc/unixasmmacrosx86.inc

# vs.

/runtime $ git ls-files ':/src/coreclr/nativeaot/*unixasmmacros*.inc'
src/coreclr/nativeaot/Runtime/unix/unixasmmacros.inc
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosamd64.inc
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm.inc
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm64.inc
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosx86.inc

cc @MichalStrehovsky, @jkotas, @janvorli

am11 commented 2 years ago

cc @directhex (regarding https://github.com/dotnet/runtime/pull/62594#discussion_r791251077), for some reason the outerloop armv6 failure is caused by /pal/ variant https://github.com/dotnet/runtime/blob/a5cf724aa2c9a67eb45827b56d6315acd4edea84/src/coreclr/pal/inc/unixasmmacrosarm.inc#L265 but not the nativeaot one: https://github.com/dotnet/runtime/blob/a5cf724aa2c9a67eb45827b56d6315acd4edea84/src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm.inc#L185 (although both components are bulilding; maybe it's unreachable code :thinking:)

MichalStrehovsky commented 2 years ago

It would definitely be nice to deduplicate.

My only concern is that we currently have zero testing for NativeAOT ARM64 (outside of just building it), so I would rather not make big changes to ARM64 files right now. ARM64 testing is on my TODO list for the coming weeks.

but not the nativeaot one:

NativeAOT is only building on ARM64 and x64:

https://github.com/dotnet/runtime/blob/a5cf724aa2c9a67eb45827b56d6315acd4edea84/src/coreclr/CMakeLists.txt#L137-L142

It might be buildable on ARM32, but this was scoped down to what we really need in the initial move from runtimelab.