This is an archive of the discontinued LLVM Phabricator instance.

[arm][compiler-rt] add armv8m.main and arv8.1m.main targets
ClosedPublic

Authored by stuij on Mar 30 2021, 9:05 AM.

Details

Summary

These changes were enough to compile compiler-rt builtins for armv8m.main and
armv8.1m.main.

Diff Detail

Event Timeline

stuij created this revision.Mar 30 2021, 9:05 AM
stuij requested review of this revision.Mar 30 2021, 9:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 30 2021, 9:05 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript

From an ELF perspective this looks good to me, armv7e-m appears in the same places for these targets. Will be worth adding someone from Apple to see if it is worth adding to CompilerRTDarwinUtils.cmake.

compiler-rt/lib/builtins/CMakeLists.txt
564

in cmake/Modules/CompilerRTDarwinUtils.cmake there are a few references to macho_embedded_ARCHS for example:

set(DARWIN_macho_embedded_ARCHS armv6m armv7m armv7em armv7 i386 x86_64)

Would be useful to get someone from Apple on the review to check if we should add to this list as well. My guess is that these aren't needed for building bare-metal ELF.

stuij added a reviewer: beanz.Apr 6 2021, 8:55 AM
stuij added a comment.Apr 6 2021, 8:58 AM

Hi @beanz . @peter.smith wondered if it would be useful to add armv8 targets to DARWIN_macho_embedded_ARCHS or somewhere else. Do you have an opinion on this, or could you recommend someone else who might have?

Thanks!

stuij added a comment.Apr 14 2021, 5:53 AM

It's now been a bit more than a week since the last reply. I suggest we merge this. If we want to add the architectures to the Apple-specific targets, that can always be done later.

It's now been a bit more than a week since the last reply. I suggest we merge this. If we want to add the architectures to the Apple-specific targets, that can always be done later.

Sounds reasonable to me.

beanz added a comment.Apr 14 2021, 7:13 AM

Sorry for not seeing this. Ignoring macho_embedded is the right call. That is really just used for macho baremetal and not many people outside Apple would bother with it.

stuij added a comment.Apr 14 2021, 7:48 AM

Ok, thanks for the confirmation @beanz.

Is someone willing to accept the revision?

peter.smith accepted this revision.Apr 14 2021, 8:04 AM

LGTM, sorry for the delay.

This revision is now accepted and ready to land.Apr 14 2021, 8:04 AM
stuij added a comment.Apr 14 2021, 8:39 AM

np, thanks so much!

This revision was automatically updated to reflect the committed changes.