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

[cmake] Add hexagon-linux cmake cache files #98712

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

androm3da
Copy link
Member

These can be used to create a fully-bootstrapped toolchain to target hexagon {baremetal,linux} with scripts like the ones in https://github.com/quic/toolchain_for_hexagon

@androm3da androm3da self-assigned this Jul 13, 2024
@androm3da androm3da requested a review from a team as a code owner July 13, 2024 04:38
@llvmbot llvmbot added clang Clang issues not falling into any other category compiler-rt libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. labels Jul 13, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 13, 2024

@llvm/pr-subscribers-libcxx

@llvm/pr-subscribers-clang

Author: Brian Cain (androm3da)

Changes

These can be used to create a fully-bootstrapped toolchain to target hexagon {baremetal,linux} with scripts like the ones in https://github.com/quic/toolchain_for_hexagon


Full diff: https://github.com/llvm/llvm-project/pull/98712.diff

4 Files Affected:

  • (added) clang/cmake/caches/hexagon-unknown-linux-musl-clang-cross.cmake (+11)
  • (added) clang/cmake/caches/hexagon-unknown-linux-musl-clang.cmake (+15)
  • (added) compiler-rt/cmake/caches/hexagon-linux-builtins.cmake (+15)
  • (added) libcxx/cmake/caches/hexagon-linux-runtimes.cmake (+25)
diff --git a/clang/cmake/caches/hexagon-unknown-linux-musl-clang-cross.cmake b/clang/cmake/caches/hexagon-unknown-linux-musl-clang-cross.cmake
new file mode 100644
index 0000000000000..2df7d8183baa6
--- /dev/null
+++ b/clang/cmake/caches/hexagon-unknown-linux-musl-clang-cross.cmake
@@ -0,0 +1,11 @@
+# This file is for the llvm+clang options that are specific to building
+# a cross-toolchain targeting hexagon linux.
+set(DEFAULT_SYSROOT "../target/hexagon-unknown-linux-musl/" CACHE STRING "")
+set(CLANG_LINKS_TO_CREATE
+            hexagon-unknown-linux-musl-clang++
+            hexagon-unknown-linux-musl-clang
+            hexagon-unknown-none-elf-clang++
+            hexagon-unknown-none-elf-clang
+            CACHE STRING "")
+
+set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
diff --git a/clang/cmake/caches/hexagon-unknown-linux-musl-clang.cmake b/clang/cmake/caches/hexagon-unknown-linux-musl-clang.cmake
new file mode 100644
index 0000000000000..9f3eb4678fd17
--- /dev/null
+++ b/clang/cmake/caches/hexagon-unknown-linux-musl-clang.cmake
@@ -0,0 +1,15 @@
+
+set(LLVM_TARGETS_TO_BUILD "Hexagon" CACHE STRING "")
+set(LLVM_DEFAULT_TARGET_TRIPLE "hexagon-unknown-linux-musl" CACHE STRING "")
+set(CLANG_DEFAULT_CXX_STDLIB "libc++" CACHE STRING "")
+set(CLANG_DEFAULT_OBJCOPY "llvm-objcopy" CACHE STRING "")
+set(CLANG_DEFAULT_RTLIB "compiler-rt" CACHE STRING "")
+set(CLANG_DEFAULT_UNWINDLIB "libunwind" CACHE STRING "")
+set(CLANG_DEFAULT_LINKER "lld" CACHE STRING "")
+set(LLVM_ENABLE_PROJECTS "clang;lld" CACHE STRING "")
+
+set(LLVM_INCLUDE_TESTS OFF CACHE BOOL "")
+set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
+# Enabling toolchain-only causes problems when doing some of the
+# subsequent builds, will need to investigate:
+set(LLVM_INSTALL_TOOLCHAIN_ONLY OFF CACHE BOOL "")
diff --git a/compiler-rt/cmake/caches/hexagon-linux-builtins.cmake b/compiler-rt/cmake/caches/hexagon-linux-builtins.cmake
new file mode 100644
index 0000000000000..d9c9ff2a4655e
--- /dev/null
+++ b/compiler-rt/cmake/caches/hexagon-linux-builtins.cmake
@@ -0,0 +1,15 @@
+set(CMAKE_ASM_FLAGS "-G0 -mlong-calls -fno-pic" CACHE STRING "")
+
+set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR OFF CACHE BOOL "")
+set(LLVM_TARGET_TRIPLE hexagon-unknown-linux-musl CACHE STRING "")
+set(COMPILER_RT_DEFAULT_TARGET_TRIPLE hexagon-unknown-linux-musl CACHE STRING "")
+set(COMPILER_RT_BUILD_BUILTINS ON CACHE BOOL "")
+set(COMPILER_RT_BUILD_SANITIZERS OFF CACHE BOOL "")
+set(COMPILER_RT_BUILD_XRAY OFF CACHE BOOL "")
+set(COMPILER_RT_BUILD_LIBFUZZER OFF CACHE BOOL "")
+set(COMPILER_RT_BUILD_PROFILE OFF CACHE BOOL "")
+set(COMPILER_RT_BUILD_MEMPROF OFF CACHE BOOL "")
+set(COMPILER_RT_BUILD_ORC OFF CACHE BOOL "")
+set(COMPILER_RT_BUILD_GWP_ASAN OFF CACHE BOOL "")
+set(COMPILER_RT_BUILTINS_ENABLE_PIC OFF CACHE BOOL "")
+set(COMPILER_RT_SUPPORTED_ARCH hexagon CACHE STRING "")
diff --git a/libcxx/cmake/caches/hexagon-linux-runtimes.cmake b/libcxx/cmake/caches/hexagon-linux-runtimes.cmake
new file mode 100644
index 0000000000000..fc669ecab737e
--- /dev/null
+++ b/libcxx/cmake/caches/hexagon-linux-runtimes.cmake
@@ -0,0 +1,25 @@
+
+set(CMAKE_EXE_LINKER_FLAGS "-lclang_rt.builtins-hexagon -nostdlib" CACHE STRING "")
+set(CMAKE_SHARED_LINKER_FLAGS "-lclang_rt.builtins-hexagon -nostdlib" CACHE STRING "")
+set(CMAKE_CXX_COMPILER_TARGET hexagon-unknown-linux-musl CACHE STRING "")
+
+set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR OFF CACHE BOOL "")
+set(LLVM_ENABLE_RUNTIMES "libcxx;libcxxabi;libunwind;compiler-rt" CACHE STRING "")
+set(LIBCXX_INCLUDE_BENCHMARKS OFF CACHE BOOL "")
+set(LIBCXX_HAS_MUSL_LIBC ON CACHE BOOL "")
+set(LIBCXX_INCLUDE_TESTS ON CACHE BOOL "")
+set(LIBCXXABI_INCLUDE_TESTS ON CACHE BOOL "")
+set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "")
+set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
+set(LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL OFF CACHE BOOL "")
+set(LIBCXXABI_ENABLE_SHARED ON CACHE BOOL "")
+set(COMPILER_RT_DEFAULT_TARGET_TRIPLE hexagon-unknown-linux-musl CACHE STRING "")
+set(COMPILER_RT_BUILD_BUILTINS OFF CACHE BOOL "")
+set(COMPILER_RT_BUILD_SANITIZERS ON CACHE BOOL "")
+set(COMPILER_RT_SUPPORTED_ARCH hexagon CACHE STRING "")
+set(COMPILER_RT_USE_LLVM_UNWINDER ON CACHE BOOL "")
+set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
+
+# This was added as a fix/workaround for changes introduced by llvm-project
+# commit https://github.com/llvm/llvm-project/commit/ab41ea4be364dcac32d0c4ec990735c8adb279c8
+set(CXX_SUPPORTS_NOSTDLIBXX_FLAG OFF CACHE BOOL "")

set(LIBCXXABI_INCLUDE_TESTS ON CACHE BOOL "")
set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "")
set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
set(LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL OFF CACHE BOOL "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is supposed to be determined automatically, not set in cache files.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are overrides that were necessary at one time, perhaps necessary still. Or possibly they're masking a bug somewhere else.

Good point - I'll take some time to investigate these and see about omitting them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL has been removed.

Comment on lines 16 to 20
set(COMPILER_RT_DEFAULT_TARGET_TRIPLE hexagon-unknown-linux-musl CACHE STRING "")
set(COMPILER_RT_BUILD_BUILTINS OFF CACHE BOOL "")
set(COMPILER_RT_BUILD_SANITIZERS ON CACHE BOOL "")
set(COMPILER_RT_SUPPORTED_ARCH hexagon CACHE STRING "")
set(COMPILER_RT_USE_LLVM_UNWINDER ON CACHE BOOL "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think those settings belong in this cache?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think those settings belong in this cache?

This comment refers to all five of those? COMPILER_RT_DEFAULT_TARGET_TRIPLE, COMPILER_RT_BUILD_BUILTINS, COMPILER_RT_BUILD_SANITIZERS, COMPILER_RT_SUPPORTED_ARCH, COMPILER_RT_USE_LLVM_UNWINDER?

Of those, COMPILER_RT_BUILD_BUILTINS, COMPILER_RT_BUILD_SANITIZERS are option()s used to control the build scope - those can stay, right? And COMPILER_RT_USE_LLVM_UNWINDER - this is an option() too. Can't we set this here?

COMPILER_RT_SUPPORTED_ARCH: this has been removed.

COMPILER_RT_DEFAULT_TARGET_TRIPLE is still present, but if it's not allowed/expected I will explore removing it. It may be redundant with LLVM_TARGET_TRIPLE?

Comment on lines 2 to 3
set(CMAKE_EXE_LINKER_FLAGS "-lclang_rt.builtins-hexagon -nostdlib" CACHE STRING "")
set(CMAKE_SHARED_LINKER_FLAGS "-lclang_rt.builtins-hexagon -nostdlib" CACHE STRING "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are those flags necessary when building for Hexagon?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll double check these too - I recall they were necessary during bootstrapping, but could have been due to driver bugs fixed since we added that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first failure encountered when I remove these is related to C_SUPPORTS_NODEFAULTLIBS_FLAG.

And I think it's llvm_check_compiler_linker_flag(C "-nodefaultlibs" C_SUPPORTS_NODEFAULTLIBS_FLAG) from llvm-project/libunwind/cmake/config-ix.cmake failing.

Sorry if I'm being dense here but if the compiler feature test tries to build an executable using -nodefaultlibs but without providing the necessary libraries to satisfy a trivial C executable, wouldn't we expect it to fail? I mean, I guess other compilers must be succeeding here, so I'll have to try and understand how. Or maybe it could be that -nodefaultlibs is mishandled by the hexagon driver? I guess I'd figure it would remove at least -lc though, which seems like it should fail something like this.

Change Dir: /local/mnt/workspace/upstream/toolchain_for_hexagon/obj_libs/CMakeFiles/CMakeScratch/TryCompile-z4nhCq

Run Build Command(s):/pkg/qct/software/ninja/1.8.2/ninja cmTC_11433 && [1/2] Building C object CMakeFiles/cmTC_11433.dir/src.c.o
[2/2] Linking C executable cmTC_11433
FAILED: cmTC_11433 
: && /local/mnt/workspace/upstream/toolchain_for_hexagon/clang+llvm-19.0.0-cross-hexagon-unknown-linux-musl/x86_64-linux-gnu/bin/hexagon-unknown-linux-musl-clang --target=hexagon-unknown-linux-musl -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections  --start-no-unused-arguments --unwindlib=none --end-no-unused-arguments -nodefaultlibs  CMakeFiles/cmTC_11433.dir/src.c.o -o cmTC_11433   && :
ld.lld: error: undefined symbol: __libc_start_main
>>> referenced by crt1.c
>>>               /local/mnt/workspace/upstream/toolchain_for_hexagon/clang+llvm-19.0.0-cross-hexagon-unknown-linux-musl/x86_64-linux-gnu/bin/../target/hexagon-unknown-linux-musl//usr/lib/crt1.o:(_start_c)
>>> referenced by crt1.c
>>>               /local/mnt/workspace/upstream/toolchain_for_hexagon/clang+llvm-19.0.0-cross-hexagon-unknown-linux-musl/x86_64-linux-gnu/bin/../target/hexagon-unknown-linux-musl//usr/lib/crt1.o:(_start_c)
hexagon-unknown-linux-musl-clang: error: hexagon-ld command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.


Source file was:
int main(void) { return 0; }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually, -nodefaultlibs should work if you don't make any calls to standard library functions. In this case it looks like the compiler is relying on an external library to provide __libc_start_main (presumably the thing that calls main), which causes linker errors like this.

I think you want to try setting CMAKE_TRY_COMPILE_TARGET_TYPE to STATIC_LIBRARY instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion, it looks like this uncovers a driver bug. I'll take a look at that first.

Copy link
Member Author

@androm3da androm3da Jul 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think the driver bug (--unwindlib=none was ignored) might not have been the critical factor. I'll open a PR for that regardless. But I made a few fixes to address these explicit clangrt link flags and perhaps some of the other concerns you raised.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the driver bug (--unwindlib=none was ignored) might not have been the critical factor.

This was fixed in #99552. And these flag overrides have now been removed.

@@ -0,0 +1,25 @@

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meta comment: As part of our support policy, we would like to have a CI job that builds and tests libc++ for Hexagon. I'm not very familiar with that target, but do you think you could set that up?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO landing these cache files are baby steps towards having such a CI job. I'll look into what it would take, yes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack. Just be aware that so long as there's no CI job, the platform is not officially supported so it may happen that we'll make changes that will make your life harder. We won't do it on purpose, of course, but it could happen because "no CI" == "no visibility into the impact of changes for that configuration".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely. That's been the status quo and this PR on its own can't change that.

The intent of this pull req is to allow anyone to build a toolchain for targeting hexagon (of which libc++, libc++abi, libunwind are a part). But another benefit can be that we should be able to setup CI job(s) leveraging this, indeed. We can enable regression tests for the entire toolchain including compiler, linker and target libraries.

@androm3da androm3da force-pushed the bcain/hex_cmake_cache branch 2 times, most recently from c01c1ee to bfe841f Compare July 18, 2024 13:12
androm3da added a commit to quic/toolchain_for_hexagon that referenced this pull request Jul 18, 2024
During llvm/llvm-project#98712 we got feedback
on the hexagon-linux-runtimes cache file that we were using unconventional
cmake features/overrides (ones that are not declared as explicit build
options).

Also: after llvm/llvm-project#98650 landed, it removed
the need for some of these awkward overrides.

Signed-off-by: Brian Cain <bcain@quicinc.com>
@androm3da androm3da requested a review from ldionne July 19, 2024 04:47
@androm3da
Copy link
Member Author

@ldionne ping

@androm3da
Copy link
Member Author

@ldionne ping

@ldionne let me know if it would be helpful for me to decompose this change into a smaller one that would be easier to review.

@androm3da
Copy link
Member Author

@ldionne hopefully the changes I've made here are satisfactory and address your comments, let me know if there's other changes I could make.

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay. I have some questions and comments but this looks reasonable.

@@ -0,0 +1,25 @@

set(LLVM_DEFAULT_TARGET_TRIPLE hexagon-unknown-linux-musl CACHE STRING "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you really need this setting here? Can't you instead set CMAKE_CXX_COMPILER_TARGET?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm willing to try that, it makes sense.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so I already have CMAKE_CXX_COMPILER_TARGET set this way in hexagon-linux-cross.cmake (I left this cache out of this PR but I could include it if it's useful).

So omitting LLVM_DEFAULT_TARGET_TRIPLE results in the failure below.

cmake -G Ninja \
		-DCMAKE_BUILD_TYPE=Release \
		-DLLVM_CMAKE_DIR:PATH=${TOOLCHAIN_LIB} \
		-DCMAKE_INSTALL_PREFIX:PATH=${HEX_TOOLS_TARGET_BASE} \
		-DCMAKE_CROSSCOMPILING:BOOL=ON \
		-DCMAKE_CXX_COMPILER_FORCED:BOOL=ON \
		-C ./hexagon-linux-cross.cmake \
		-C ./hexagon-linux-runtimes.cmake \
		-B ./obj_libs \
		-S ./llvm-project/runtimes
...
-- Configuring done
CMake Error at /local/mnt/workspace/upstream/toolchain_for_hexagon/llvm-project/compiler-rt/cmake/Modules/AddCompilerRT.cmake:357 (add_library):
  Error evaluating generator expression:

    $<TARGET_OBJECTS:RTSanitizerCommon.x86_64>

  Objects of target "RTSanitizerCommon.x86_64" referenced but no such target
  exists.
Call Stack (most recent call first):
  /local/mnt/workspace/upstream/toolchain_for_hexagon/llvm-project/compiler-rt/lib/ctx_profile/CMakeLists.txt:25 (add_compiler_rt_runtime)


CMake Error at /local/mnt/workspace/upstream/toolchain_for_hexagon/llvm-project/compiler-rt/cmake/Modules/AddCompilerRT.cmake:357 (add_library):
  Error evaluating generator expression:

    $<TARGET_OBJECTS:RTSanitizerCommon.x86_64>

  Objects of target "RTSanitizerCommon.x86_64" referenced but no such target
  exists.
Call Stack (most recent call first):
  /local/mnt/workspace/upstream/toolchain_for_hexagon/llvm-project/compiler-rt/lib/ctx_profile/CMakeLists.txt:25 (add_compiler_rt_runtime)


CMake Error at /local/mnt/workspace/upstream/toolchain_for_hexagon/llvm-project/compiler-rt/cmake/Modules/AddCompilerRT.cmake:357 (add_library):
  Error evaluating generator expression:

    $<TARGET_OBJECTS:RTSanitizerCommon.x86_64>

  Objects of target "RTSanitizerCommon.x86_64" referenced but no such target
  exists.
Call Stack (most recent call first):
  /local/mnt/workspace/upstream/toolchain_for_hexagon/llvm-project/compiler-rt/lib/ctx_profile/CMakeLists.txt:25 (add_compiler_rt_runtime)


CMake Error at /local/mnt/workspace/upstream/toolchain_for_hexagon/llvm-project/compiler-rt/cmake/Modules/AddCompilerRT.cmake:357 (add_library):
  No SOURCES given to target: clang_rt.ctx_profile-x86_64
Call Stack (most recent call first):
  /local/mnt/workspace/upstream/toolchain_for_hexagon/llvm-project/compiler-rt/lib/ctx_profile/CMakeLists.txt:25 (add_compiler_rt_runtime)

But llvm-project/runtimes/CMakeLists.txt has this, which I think is how I determined that I should be using LLVM_DEFAULT_TARGET_TRIPLE (or LLVM_TARGET_TRIPLE eventually?).

# Host triple is used by tests to check if they are running natively.
include(GetHostTriple)
get_host_triple(LLVM_HOST_TRIPLE)
message(STATUS "LLVM host triple: ${LLVM_HOST_TRIPLE}")

# TODO: We shouldn't be using LLVM_DEFAULT_TARGET_TRIPLE for runtimes since we
# aren't generating code, LLVM_TARGET_TRIPLE is a better fit.
set(LLVM_DEFAULT_TARGET_TRIPLE "${LLVM_HOST_TRIPLE}" CACHE STRING
    "Default target for which the runtimes will be built.")
message(STATUS "LLVM default target triple: ${LLVM_DEFAULT_TARGET_TRIPLE}")

set(LLVM_TARGET_TRIPLE "${LLVM_DEFAULT_TARGET_TRIPLE}")

set(COMPILER_RT_BUILD_XRAY OFF CACHE BOOL "")
set(COMPILER_RT_BUILD_MEMPROF OFF CACHE BOOL "")

set(LIBCXX_HAS_ATOMIC_LIB OFF CACHE BOOL "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be detected automatically at

check_library_exists(atomic __atomic_fetch_add_8 "" LIBCXX_HAS_ATOMIC_LIB)
. Is that somehow broken?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose it may have been at some point and we ended up with this as a workaround? Hopefully now it works. I'll investigate.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, they are no longer necessary. These have been removed. Thank you for pointing it out.

Comment on lines 15 to 18
set(COMPILER_RT_BUILD_BUILTINS OFF CACHE BOOL "")
set(COMPILER_RT_BUILD_SANITIZERS OFF CACHE BOOL "")
set(COMPILER_RT_BUILD_XRAY OFF CACHE BOOL "")
set(COMPILER_RT_BUILD_MEMPROF OFF CACHE BOOL "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are also in the compiler-rt CMake caches -- I think they can be removed from here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are build options -- they indicate the scope of what to build - but the defaults are overridden specifically for the hexagon toolchain. This is in order to avoid defects either in the code we're building or in the compiler. There's no existing cache that could know about these architecture-specific bugs, I figure.

For example, in compiler-rt/lib/sanitizer_common/sanitizer_redefine_builtins.h there's these inline asm statements to alias some functions - the hexagon assembler rejects these.

asm("memcpy = __sanitizer_internal_memcpy");
asm("memmove = __sanitizer_internal_memmove");
asm("memset = __sanitizer_internal_memset");

I think it probably makes sense to use a directive instead of this syntax which isn't (as) portable. Or maybe it's a bug in the assembler. In any case, I'd like to punt and just descope the toolchain build for now.

But maybe I've misunderstood what you're trying to say about the compiler-rt CMake caches - maybe I should specify these outside of hexagon-linux-runtimes.cmake in hexagon-linux-compiler-rt.cmake? If so, that's fine w/me (though we'll be loading both cache files when building the runtimes).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on your comments it's as if you're saying "compiler-rt isn't part of 'runtimes'" and yet - it certainly can be, so I assumed that it should.

When we specify LLVM_ENABLE_RUNTIMES as libcxx;libcxxabi;libunwind;compiler-rt then it builds all of them, so it made sense to me that we could/should include all of the option inputs here in one place.

But for the sake of being able to build them independently, I'm happy to decompose the cache files.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, oops - since I put this in libcxx/cmake/caches/hexagon-linux-runtimes.cmake it does look like it's intended for libc++. Sorry, that should've been obvious to me but it wasn't.

Okay, working on an update that separates these.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're now separated, I think this is how you'll prefer it.

set(LIBUNWIND_INCLUDE_TESTS OFF CACHE BOOL "")
set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "")
set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
set(COMPILER_RT_USE_LLVM_UNWINDER ON CACHE BOOL "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one belongs to the compiler-rt cache

@androm3da androm3da force-pushed the bcain/hex_cmake_cache branch 2 times, most recently from bca45b0 to 29554e6 Compare September 6, 2024 04:15
@androm3da
Copy link
Member Author

@ldionne I think you'll be satisfied with the changes I've made. Previously I didn't think your feedback made sense, but I've come around 😉

@androm3da
Copy link
Member Author

@ldionne please take a look when you get a chance. Thanks!

These can be used to create a fully-bootstrapped cross-toolchain to target
hexagon {baremetal,linux} with scripts like the ones in
https://github.com/quic/toolchain_for_hexagon
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category compiler-rt libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants