Diff Detail
Event Timeline
While I fully agree that the current fallback-to-GCC behavior is far from helpful, I'm not sure how much sense providing linker support for bare-metal makes. I would expect this to changes wildly depending on the specific environment.
I would expect this to changes wildly depending on the specific environment.
My assertion is that our default "specific environment" ought to cater to using llvm's own tools... at least until someone comes along and says they actually want -fuse-ld=gcc, or whatever.
cmake/caches/BaremetalARM.cmake | ||
---|---|---|
2 | Please rename this file to BareMetalARMv6.cmake. (I'm interested in the suffix primarily). | |
lib/Driver/ToolChains/BareMetal.cpp | ||
69 | Why not just the standard arm directory? | |
75 | This seems wrong. The libc++ headers should *not* be in the resource dir. This should be based off of the sysroot IMO. | |
108–109 | I think that this is a bit extreme. We already have -stdlib=libc++ and -stdlib=libstdc++. Why not just honor that? | |
lib/Driver/ToolChains/BareMetal.h | ||
43 | I think that this really should be ld still, as that is the canonical name for the linker. | |
58 | Unnecessary? | |
68 | Update this accordingly. |
cmake/caches/BaremetalARM.cmake | ||
---|---|---|
2 | My plan is to eventually add multilibs to this configuration, so while that makes sense short term, I don't think it makes sense long term. With that in mind, do you still want me to rename it? | |
lib/Driver/ToolChains/BareMetal.cpp | ||
69 | There are a few differences between the stuff in the existing ones, and what is needed on baremetal. For example __enable_execute_stack, emutls, as well as anything else that assumes existence of pthreads support shouldn't be there. | |
75 | Oh, excellent point. I was just making that same argument a few weeks ago. | |
108–109 | I wasn't going for "support every possible thing out of the tin", instead preferring incremental development :) |
lib/Driver/ToolChains/BareMetal.h | ||
---|---|---|
43 | You mean lld? $ lld lld is a generic driver. Invoke ld.lld (Unix), ld (macOS) or lld-link (Windows) instead. Or are you saying: "make binutils ld the default, not llvm's lld"? |
lib/Driver/ToolChains/BareMetal.cpp | ||
---|---|---|
108–109 | Added support for -stdlib=. I made the assumption that -stdlib=libstdc++ implies the user wants libsupc++ as their ABI library, though someone may want libstdc++ + libc++abi, or other combinations. If that's the case, we can add -abilib=, and likewise -unwinder=... but let's save that for another patch, on another day. |
cmake/caches/BaremetalARM.cmake | ||
---|---|---|
2 | I think it makes sense longer term still. ARMv6 vs ARMv7. Even if you do multilib, that wouldnt take care of that right? | |
lib/Driver/ToolChains/BareMetal.cpp | ||
69 | Well, I think that "baremetal" here is a bad idea. How about using the android approach? Use clang_rt.builtins-arm-baremetal.a ? | |
108–109 | Yeah, switching between libc++/libc++abi and libstdc++/libsupc++ sounds reasonable. Anything beyond that is something which doesnt work well within the driver anyways. I should polish off my -unwinder patch. | |
lib/Driver/ToolChains/BareMetal.h | ||
43 | Im saying use the name "ld". ld is a symlink on most linux distributions these days. ld -> ld.gold, ld.bfd, ld.lld |
cmake/caches/BaremetalARM.cmake | ||
---|---|---|
2 | It's not limited to just ARMv6m multilibs... imagine it had: set(LLVM_BUILTIN_TARGETS "armv6m-none-eabi;armv7m-none-eabi" CACHE STRING "Builtin Targets") Then naming the file BaremetalARMv6m.cmake will be inappropriately specific. Granted, the way clangrt is built/used now isn't very conducive to multilibs that differ by more than just the arch, since the name currently has to be of the form: libclang_rt-builtins-${triple's subarch}.a, which for example doesn't allow for differentiating between: v7m;@mthumb@march=armv7-m v7m-PIC;@mthumb@march=armv7-m@fPIC or v7a-neon;@march=armv7-a@mfloat-abi=softfp@mfpu=neon v7a-vfpv3-d16;@march=armv7-a@mfloat-abi=softfp@mfpu=vfpv3-d16 | |
lib/Driver/ToolChains/BareMetal.cpp | ||
69 | Why? Given the way the cmake goop works in lib/runtimes + compiler-rt, the folder name there has to be the same as the CMAKE_SYSTEM_NAME. The alternative, I guess, is to call it 'generic', but I'm not convinced that's better than 'baremetal'. | |
lib/Driver/ToolChains/BareMetal.h | ||
43 | I don't think this makes sense. I don't care about "most linux distributions", but rather about putting together an LLVM baremetal distribution based as much as possible on LLVM components. If someone wants a different linker, they can use -fuse-ld= and/or -B. Also, doesn't using a symlink named ld cause lld to behave like a mach-o linker? We really want the elf one here. |
lib/Driver/ToolChains/BareMetal.h | ||
---|---|---|
43 | If you invoke lld as ld, it behaves as a Mach-O linker on macOS. On other OSes, it behaves as an ELF linker. |
lib/Driver/ToolChains/BareMetal.cpp | ||
---|---|---|
69 | Because I can have a baremetal environment that uses a different architecture. How do you differentiate between the MIPS and ARM bare metal runtimes? The way that the compiler actually looks up the builtins is that it uses clang_rt.[component]-[arch][-variant] | |
lib/Driver/ToolChains/BareMetal.h | ||
43 | ld really is just the unix name for the linker. I think that at some point we are going to have to consider cross-linking with lld. Adding a -flavor <flavor> with -fuse-ld=lld makes sense I think. |
lib/Driver/ToolChains/BareMetal.cpp | ||
---|---|---|
69 | Yes, and that's still how they're being looked up (and built/installed), even in this patch: lib/clang/[version]/lib/[cmake_system_name]/libclangrt.[component]-[arch][-variant].a Having arch+variant in the name means they won't intersect, just as they don't for any other system. The only difference here is that baremetal doesn't really have a "system" per se, and it's not appropriate to use the darwin/linux/whatever ones, hence the 'baremetal' folder. |
cmake/caches/BaremetalARM.cmake | ||
---|---|---|
11 | hmm. not sure why these two aren't getting built. |
Looks generally pretty good. This is going to be a pretty cool addition!
lib/Driver/ToolChains/BareMetal.cpp | ||
---|---|---|
69 | Ah. I see, I was visualizing the tree incorrectly. Using baremetal this way is fine by me. | |
111 | Is this layout consistent between libc++ and libstdc++? | |
131–134 | Use StringSwitch? | |
lib/Driver/ToolChains/BareMetal.h | ||
40 | Is the profiler support in compiler-rt sufficiently standalone to build it for baremetal? |
lib/Driver/ToolChains/BareMetal.cpp | ||
---|---|---|
111 | Damn, no it's not. | |
131–134 | StringSwitch isn't great when you want to error out of the default case. That being said, this whole function is unnecessary: I can just defer to the base class' implementation, which does almost the same thing. | |
lib/Driver/ToolChains/BareMetal.h | ||
40 | IIRC, there was a test case that wanted this on for one of the arm-none-eabi triples. This is when my patches were against 4.0. After rebasing to trunk, I think that particular test is gone (split up into separate ones, I guess?). I'll leave it off for now until someone confirms it is actually supported. | |
lib/Driver/ToolChains/Linux.cpp | ||
379 | Noticed that this was in the wrong place, and also, surprisingly, that it was incorrectly qualified. |
Fix a cmake warning:
Platform/baremetal to use this system, please send your config file to cmake@www.cmake.org so it can be added to cmake Your CMakeCache.txt file was copied to CopyOfCMakeCache.txt. Please send that file to cmake@www.cmake.org.
Generic does what we want already, it just has a name that's not useful.
This causes a test failure with non-standard CLANG_RESOURCE_DIR:
Command Output (stderr): -- /var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999/test/Driver/baremetal.cpp:8:22: error: expected string not found in input // CHECK-V6M-C-SAME: "-resource-dir" "[[PREFIX_DIR]]{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}[[VERSION:[^"]*]]" ^ <stdin>:5:128: note: scanning from here "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999-abi_x86_32.x86/./bin/clang" "-cc1" "-triple" "thumbv6m-none--eabi" "-emit-obj" "-mrelax-all" "-disable-free" "-disable-llvm-verifier" "-discard-value-names" "-main-file-name" "baremetal.cpp" "-mrelocation-model" "static" "-mthread-model" "single" "-mdisable-fp-elim" "-fmath-errno" "-no-integrated-as" "-mconstructor-aliases" "-nostdsysteminc" "-target-cpu" "cortex-m0" "-target-feature" "+soft-float-abi" "-target-feature" "+strict-align" "-target-abi" "aapcs" "-mfloat-abi" "soft" "-dwarf-column-info" "-debugger-tuning=gdb" "-resource-dir" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999-abi_x86_32.x86/./bin/../../../../lib/clang/5.0.0" "-isysroot" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999/test/Driver/Inputs/baremetal_arm" "-internal-isystem" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999/test/Driver/Inputs/baremetal_arm/include/c++/v1" "-internal-isystem" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999-abi_x86_32.x86/./bin/../../../../lib/clang/5.0.0/include" "-internal-isystem" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999/test/Driver/Inputs/baremetal_arm/include" "-fdeprecated-macro" "-fdebug-compilation-dir" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999-abi_x86_32.x86/test/Driver" "-ferror-limit" "19" "-fmessage-length" "0" "-fallow-half-arguments-and-returns" "-fno-signed-char" "-fobjc-runtime=gcc" "-fcxx-exceptions" "-fexceptions" "-fdiagnostics-show-option" "-o" "/var/tmp/portage/sys-devel/clang-9999/temp/baremetal-6b1d37.o" "-x" "c++" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999/test/Driver/baremetal.cpp" ^ <stdin>:5:128: note: with variable "PREFIX_DIR" equal to "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999-abi_x86_32.x86/." "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999-abi_x86_32.x86/./bin/clang" "-cc1" "-triple" "thumbv6m-none--eabi" "-emit-obj" "-mrelax-all" "-disable-free" "-disable-llvm-verifier" "-discard-value-names" "-main-file-name" "baremetal.cpp" "-mrelocation-model" "static" "-mthread-model" "single" "-mdisable-fp-elim" "-fmath-errno" "-no-integrated-as" "-mconstructor-aliases" "-nostdsysteminc" "-target-cpu" "cortex-m0" "-target-feature" "+soft-float-abi" "-target-feature" "+strict-align" "-target-abi" "aapcs" "-mfloat-abi" "soft" "-dwarf-column-info" "-debugger-tuning=gdb" "-resource-dir" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999-abi_x86_32.x86/./bin/../../../../lib/clang/5.0.0" "-isysroot" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999/test/Driver/Inputs/baremetal_arm" "-internal-isystem" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999/test/Driver/Inputs/baremetal_arm/include/c++/v1" "-internal-isystem" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999-abi_x86_32.x86/./bin/../../../../lib/clang/5.0.0/include" "-internal-isystem" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999/test/Driver/Inputs/baremetal_arm/include" "-fdeprecated-macro" "-fdebug-compilation-dir" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999-abi_x86_32.x86/test/Driver" "-ferror-limit" "19" "-fmessage-length" "0" "-fallow-half-arguments-and-returns" "-fno-signed-char" "-fobjc-runtime=gcc" "-fcxx-exceptions" "-fexceptions" "-fdiagnostics-show-option" "-o" "/var/tmp/portage/sys-devel/clang-9999/temp/baremetal-6b1d37.o" "-x" "c++" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999/test/Driver/baremetal.cpp" ^ <stdin>:5:588: note: possible intended match here "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999-abi_x86_32.x86/./bin/clang" "-cc1" "-triple" "thumbv6m-none--eabi" "-emit-obj" "-mrelax-all" "-disable-free" "-disable-llvm-verifier" "-discard-value-names" "-main-file-name" "baremetal.cpp" "-mrelocation-model" "static" "-mthread-model" "single" "-mdisable-fp-elim" "-fmath-errno" "-no-integrated-as" "-mconstructor-aliases" "-nostdsysteminc" "-target-cpu" "cortex-m0" "-target-feature" "+soft-float-abi" "-target-feature" "+strict-align" "-target-abi" "aapcs" "-mfloat-abi" "soft" "-dwarf-column-info" "-debugger-tuning=gdb" "-resource-dir" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999-abi_x86_32.x86/./bin/../../../../lib/clang/5.0.0" "-isysroot" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999/test/Driver/Inputs/baremetal_arm" "-internal-isystem" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999/test/Driver/Inputs/baremetal_arm/include/c++/v1" "-internal-isystem" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999-abi_x86_32.x86/./bin/../../../../lib/clang/5.0.0/include" "-internal-isystem" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999/test/Driver/Inputs/baremetal_arm/include" "-fdeprecated-macro" "-fdebug-compilation-dir" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999-abi_x86_32.x86/test/Driver" "-ferror-limit" "19" "-fmessage-length" "0" "-fallow-half-arguments-and-returns" "-fno-signed-char" "-fobjc-runtime=gcc" "-fcxx-exceptions" "-fexceptions" "-fdiagnostics-show-option" "-o" "/var/tmp/portage/sys-devel/clang-9999/temp/baremetal-6b1d37.o" "-x" "c++" "/var/tmp/portage/sys-devel/clang-9999/work/x/y/clang-9999/test/Driver/baremetal.cpp"
Please rename this file to BareMetalARMv6.cmake. (I'm interested in the suffix primarily).