Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate assembly macros #64253

Open
am11 opened this issue Jan 25, 2022 · 2 comments
Open

Duplicate assembly macros #64253

am11 opened this issue Jan 25, 2022 · 2 comments

Comments

@am11
Copy link
Member

am11 commented Jan 25, 2022

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

@dotnet-issue-labeler dotnet-issue-labeler bot added area-PAL-coreclr untriaged New issue has not been triaged by the area owner labels Jan 25, 2022
@am11
Copy link
Member Author

am11 commented Jan 25, 2022

cc @directhex (regarding #62594 (comment)), for some reason the outerloop armv6 failure is caused by /pal/ variant


but not the nativeaot one:

(although both components are bulilding; maybe it's unreachable code 🤔)

@MichalStrehovsky
Copy link
Member

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:

if(NOT CLR_CROSS_COMPONENTS_BUILD)
# NativeAOT only buildable for a subset of CoreCLR-supported configurations
if((CLR_CMAKE_HOST_LINUX OR CLR_CMAKE_HOST_OSX OR CLR_CMAKE_HOST_WIN32) AND (CLR_CMAKE_HOST_ARCH_ARM64 OR CLR_CMAKE_HOST_ARCH_AMD64) AND NOT (CLR_CMAKE_HOST_OSX AND CLR_CMAKE_HOST_ARCH_ARM64))
add_subdirectory(nativeaot)
endif()
endif(NOT CLR_CROSS_COMPONENTS_BUILD)

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

@janvorli janvorli added this to the Future milestone Jun 13, 2022
@janvorli janvorli removed the untriaged New issue has not been triaged by the area owner label Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants