This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt] Compile assembly files as ASM not C
ClosedPublic

Authored by tambre on Aug 10 2020, 10:58 PM.

Details

Summary

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.
Instead enable the ASM language and mark the files as being ASM.

[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.

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

Diff Detail

Event Timeline

tambre created this revision.Aug 10 2020, 10:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2020, 10:58 PM
Herald added subscribers: Restricted Project, dexonsmith, kristof.beyls and 2 others. · View Herald Transcript
tambre requested review of this revision.Aug 10 2020, 10:58 PM

One concern I have with this change is that we may not always consistently update CMAKE_ASM_FLAGS or set CMAKE_ASM_COMPILER. This wouldn't make a difference before, but it'll with this change. Can you check if these variables are being updated as needed?

tambre updated this revision to Diff 285692.Aug 14 2020, 10:57 AM

Pass CMAKE_ASM_COMPILER to compiler-rt in runtime build

Herald added a project: Restricted Project. · View Herald TranscriptAug 14 2020, 10:57 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

One concern I have with this change is that we may not always consistently update CMAKE_ASM_FLAGS or set CMAKE_ASM_COMPILER. This wouldn't make a difference before, but it'll with this change. Can you check if these variables are being updated as needed?

I searched and was able to find only one instance. I've fixed it in this change.
I lack commit privileges, so whoever commits this should be ready to revert it, as there's a chance for breakage.

tambre edited the summary of this revision. (Show Details)Aug 14 2020, 10:59 AM
This revision was not accepted when it landed; it landed in state Needs Review.Aug 20 2020, 12:35 AM
This revision was automatically updated to reflect the committed changes.

This broke Green Dragon's compiler-rt builds and also my local compiler-rt build. See for example here: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/23424/console

I reverted in adf0b8cc70325f027d202139e3ff984c41896b57 to get the bots green again . Feel free to ping if you need someone to test the patch on macOS.