Skip to content

Commit

Permalink
[CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, …
Browse files Browse the repository at this point in the history
…unify workarounds

It isn't very wise to pass an assembly file to the compiler and tell it to compile as a C file and hope that the compiler recognizes it as assembly instead.
Simply don't mark the file as C and CMake will recognize the rest.

This was attempted earlier in https://reviews.llvm.org/D85706, but reverted due to architecture issues on Apple.
Subsequent digging revealed a similar change was done earlier for libunwind in https://reviews.llvm.org/rGb780df052dd2b246a760d00e00f7de9ebdab9d09.
Afterwards workarounds were added for MinGW and Apple:
* https://reviews.llvm.org/rGb780df052dd2b246a760d00e00f7de9ebdab9d09
* https://reviews.llvm.org/rGd4ded05ba851304b26a437896bc3962ef56f62cb

The workarounds in libunwind and compiler-rt are unified and comments added pointing to each other.
The workaround is updated to only be used for MinGW for CMake versions before 3.17, which fixed the issue (https://gitlab.kitware.com/cmake/cmake/-/merge_requests/4287).

Additionally fixed Clang not being passed as the assembly compiler for compiler-rt runtime build.

Example error:
[525/634] Building C object lib/tsan/CMakeFiles/clang_rt.tsan-aarch64.dir/rtl/tsan_rtl_aarch64.S.o
FAILED: lib/tsan/CMakeFiles/clang_rt.tsan-aarch64.dir/rtl/tsan_rtl_aarch64.S.o
/opt/tooling/drive/host/bin/clang --target=aarch64-linux-gnu -I/opt/tooling/drive/llvm/compiler-rt/lib/tsan/.. -isystem /opt/tooling/drive/toolchain/opt/drive/toolchain/include -x c -Wall -Wno-unused-parameter -fno-lto -fPIC -fno-builtin -fno-exceptions -fomit-frame-pointer -funwind-tables -fno-stack-protector -fno-sanitize=safe-stack -fvisibility=hidden -fno-lto -O3 -gline-tables-only -Wno-gnu -Wno-variadic-macros -Wno-c99-extensions -Wno-non-virtual-dtor -fPIE -fno-rtti -Wframe-larger-than=530 -Wglobal-constructors --sysroot=. -MD -MT lib/tsan/CMakeFiles/clang_rt.tsan-aarch64.dir/rtl/tsan_rtl_aarch64.S.o -MF lib/tsan/CMakeFiles/clang_rt.tsan-aarch64.dir/rtl/tsan_rtl_aarch64.S.o.d -o lib/tsan/CMakeFiles/clang_rt.tsan-aarch64.dir/rtl/tsan_rtl_aarch64.S.o -c /opt/tooling/drive/llvm/compiler-rt/lib/tsan/rtl/tsan_rtl_aarch64.S
/opt/tooling/drive/llvm/compiler-rt/lib/tsan/rtl/tsan_rtl_aarch64.S:29:1: error: expected identifier or '('
.section .text
^
1 error generated.

Differential Revision: https://reviews.llvm.org/D86308
  • Loading branch information
tambry committed Aug 27, 2020
1 parent 45eeb8c commit 45344cf
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 13 deletions.
1 change: 1 addition & 0 deletions clang/runtime/CMakeLists.txt
Expand Up @@ -75,6 +75,7 @@ if(LLVM_BUILD_EXTERNAL_COMPILER_RT AND EXISTS ${COMPILER_RT_SRC_ROOT}/)
CMAKE_ARGS ${CLANG_COMPILER_RT_CMAKE_ARGS}
-DCMAKE_C_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_CXX_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++
-DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config
Expand Down
10 changes: 5 additions & 5 deletions compiler-rt/cmake/Modules/AddCompilerRT.cmake
Expand Up @@ -109,11 +109,11 @@ endfunction()

function(add_asm_sources output)
set(${output} ${ARGN} PARENT_SCOPE)
# Xcode will try to compile asm files as C ('clang -x c'), and that will fail.
if (${CMAKE_GENERATOR} STREQUAL "Xcode")
enable_language(ASM)
else()
# Pass ASM file directly to the C++ compiler.
# CMake doesn't pass the correct architecture for Apple prior to CMake 3.19. https://gitlab.kitware.com/cmake/cmake/-/issues/20771
# MinGW didn't work correctly with assembly prior to CMake 3.17. https://gitlab.kitware.com/cmake/cmake/-/merge_requests/4287 and https://reviews.llvm.org/rGb780df052dd2b246a760d00e00f7de9ebdab9d09
# Workaround these two issues by compiling as C.
# Same workaround used in libunwind. Also update there if changed here.
if((APPLE AND CMAKE_VERSION VERSION_LESS 3.19) OR (MINGW AND CMAKE_VERSION VERSION_LESS 3.17))
set_source_files_properties(${ARGN} PROPERTIES LANGUAGE C)
endif()
endfunction()
Expand Down
12 changes: 4 additions & 8 deletions libunwind/src/CMakeLists.txt
Expand Up @@ -24,14 +24,10 @@ set(LIBUNWIND_ASM_SOURCES
UnwindRegistersRestore.S
UnwindRegistersSave.S
)
if (MINGW OR APPLE)
# CMake doesn't build assembly sources for windows/gnu targets properly
# (up to current CMake, 3.16), so treat them as C files.
# Additionally, CMake ignores OSX_ARCHITECTURE for ASM files when targeting
# Apple platforms.
set_source_files_properties(${LIBUNWIND_ASM_SOURCES}
PROPERTIES
LANGUAGE C)

# See add_asm_sources() in compiler-rt for explanation of this workaround.
if((APPLE AND CMAKE_VERSION VERSION_LESS 3.19) OR (MINGW AND CMAKE_VERSION VERSION_LESS 3.17))
set_source_files_properties(${LIBUNWIND_ASM_SOURCES} PROPERTIES LANGUAGE C)
endif()

set(LIBUNWIND_HEADERS
Expand Down

0 comments on commit 45344cf

Please sign in to comment.