Page MenuHomePhabricator

[libc] Add a simple linux aarch64 config.
ClosedPublic

Authored by sivachandra on Jun 9 2020, 10:30 PM.

Details

Summary

With this change, "ninja check-libc" on linux/aarch64 succeeds.

However, all entrypoints with machine dependent implementations
have been skipped. A good number of these skipped entrypoints can
be enabled once we have aarch64 syscall support available.

Diff Detail

Event Timeline

sivachandra created this revision.Jun 9 2020, 10:30 PM

I don't know the solution here, but there is so much which we end up copying between configs of architectures. It's most apparent with the math.h targets because our implementations are architecture and OS agnostic, but most of libc except for syscall like you mention is too. I think it's clear that having separate build config files for every OS * architecture is untenably difficult for us to maintain. I should have mentioned this earlier, it's not this patch which introduced this, but I didn't catch it then.

What if we had something like

config/entrypoints.txt
set(TARGET_LIBC_ENTRYPOINTS
 ...
)

add_subdirectory(${OS})
append(TARGET_LIBC_ENTRYPOINTS ${OS}_LIBC_ENTRYPOINTS) # or whatever the command for concatenating lists is
add_subdirectory(${ARCH})
append(...)

Or maybe the other way around where arch specific file has a set of targets it explicitly doesn't have support yet.

There might even be a clever way to do it with CMake and dependencies, like you said most which didn't make the list here for TARGET_LIBC_ENTRYPOINTS so we could have base targets like syscall and if those are missing from a target then we skip its tests.

Whatever the solution is, I don't think copying for every arch is the right way to go and even if it works right now, and I would be willing to move forward to make progress, we need an eventual solution.

I don't know the solution here, but there is so much which we end up copying between configs of architectures. It's most apparent with the math.h targets because our implementations are architecture and OS agnostic, but most of libc except for syscall like you mention is too. I think it's clear that having separate build config files for every OS * architecture is untenably difficult for us to maintain. I should have mentioned this earlier, it's not this patch which introduced this, but I didn't catch it then.

What if we had something like

config/entrypoints.txt
set(TARGET_LIBC_ENTRYPOINTS
 ...
)

add_subdirectory(${OS})
append(TARGET_LIBC_ENTRYPOINTS ${OS}_LIBC_ENTRYPOINTS) # or whatever the command for concatenating lists is
add_subdirectory(${ARCH})
append(...)

Or maybe the other way around where arch specific file has a set of targets it explicitly doesn't have support yet.

There might even be a clever way to do it with CMake and dependencies, like you said most which didn't make the list here for TARGET_LIBC_ENTRYPOINTS so we could have base targets like syscall and if those are missing from a target then we skip its tests.

Whatever the solution is, I don't think copying for every arch is the right way to go and even if it works right now, and I would be willing to move forward to make progress, we need an eventual solution.

I completely agree. I do not have a good solution either. If copying is the only problem, I wouldn't have worried that much here. But, there are problems beyond just copying. For example, the roles of api.td and headers.txt is mixed up. As you suggest, running tests only if all deps are available is an approach but there are short comings there as well. While we try to evolve our system, we need to have a way to move forward so I have stuck to the current approach for now.

abrachet accepted this revision.Jun 9 2020, 11:38 PM

This makes sense to me. We should look to do this later then. It makes sense to move on now.

I actually looked into how we could avoid having separate api.td files but TableGen only accepts one input file per invocation right now and the include wasn't very ergonomic. But yes this is also something we would need to look at.

If it's not too much trouble maybe the changes to LLVMLibCCheckCpuFeatures.cmake could be in a separate commit for the blame, as far as I can see it isn't related to adding AArch64 support, right?

This revision is now accepted and ready to land.Jun 9 2020, 11:38 PM
asteinhauser accepted this revision.Jun 9 2020, 11:46 PM

Split out the cpu feature check part into a different patch.

This revision was automatically updated to reflect the committed changes.