[Compiler-rt][MIPS] Fix cross build for XRAY

[nitesh.jain]

Summary

The triple and target information (like ABI, Architecture revision) is not pass to XRAY_CFLAGS . This missing information in XRAY_CFLAGS causes cross build to fail. This patch fix it by appending CMAKE_CXX_FLAGS to XRAY_CFLAGS.

[sdardis]

I'm seeing a failure when attempting to run 'ninja check-xray'. The CMAKE_C_FLAGS are being appended twice / CMAKE_CXX_FLAGS and CMAKE_C_FLAGS are being appended to the commandline to build the unit-tests for xray, but the second set have '\' after each argument causing clang to reject the commandline.

[nitesh.jain]

Fix unit test build issues.

[nitesh.jain]

Hi Simon,

Please could you check whether it fixes unit test build ?

Thanks,
Nitesh

[dberris]

I'm a little bit confused here.

Are the CMAKE_C_FLAGS not included in the SANITIZER_COMMON_CFLAGS definition? If not, that's probably the right place to fix this.

Otherwise, I would have thought that instead of using CMAKE_C_FLAGS, maybe use CMAKE_CXX_FLAGS instead when building for cross-compilation?

[nitesh.jain]

Update diff as per suggestion

[dberris]

In D38021#891819, @nitesh.jain wrote:

Update diff as per suggestion

I think you misunderstood my suggestion.

SANITIZER_COMMON_CFLAGS is defined somewhere else in compiler-rt. It already should be including the CMAKE_C_FLAGS into its definition, so I'm confused why we even need this change.

I still don't understand what problem you're trying to solve with this patch.

Can you clearly describe what the problem is, and why this patch is supposed to fix the problem? Why isn't CMAKE_CXX_FLAGS sufficient? Also, if this is in a hosted build, why not just set the target architecture for the compiler-rt build directly?

[nitesh.jain]

SANITIZER_COMMON_CFLAGS is defined somewhere else in compiler-rt. It already should be including the CMAKE_C_FLAGS into its definition, so I'm confused why we even need this change.

The CMAKE_C_FLAGS are not include in SANITIZER_COMMON_CFLAGS because of which cross-native build of x-ray is failing.

-Nitesh Jain

[dberris]

In D38021#891881, @nitesh.jain wrote:

SANITIZER_COMMON_CFLAGS is defined somewhere else in compiler-rt. It already should be including the CMAKE_C_FLAGS into its definition, so I'm confused why we even need this change.

The CMAKE_C_FLAGS are not include in SANITIZER_COMMON_CFLAGS because of which cross-native build of x-ray is failing.

Okay, but can't you use CMAKE_CXX_FLAGS instead of CMAKE_C_FLAGS?

Other question is why CMAKE_C_FLAGS is important and special here.

[nitesh.jain]

Updated diff as per suggestion

[dberris]

The patch seems ill-formed -- usually you want to create the patch from the root of the project. In this case it will be from the root of the compiler-rt repository/working copy.

CMakeLists.txt

273 ↗

(On Diff #118327)

Do you still really need this if you just use cmake -DCMAKE_CXX_FLAGS=... without this change? What happens when you revert this change and provide the target flags instead when you invoke cmake?

[nitesh.jain]

CMakeLists.txt

273 ↗

(On Diff #118327)

Without this change I am getting following errors

[3/788] Building ASM object projects/compiler-rt/lib/xray/CMakeFiles/clang_rt.xray-mipsel.dir/xray_trampoline_mips.S.o
FAILED: projects/compiler-rt/lib/xray/CMakeFiles/clang_rt.xray-mipsel.dir/xray_trampoline_mips.S.o
/home/nin/LLVM/x86_build/bin/clang -DXRAY_HAS_EXCEPTIONS=1 -DXRAY_SUPPORTED=1 -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -D_LARGEFILE_SOURCE -DSTDC_CONSTANT_MACROS -DSTDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iprojects/compiler-rt/lib/xray -I/home/nin/LLVM/llvm/projects/compiler-rt/lib/xray -Iinclude -I/home/nin/LLVM/llvm/include -I/home/nin/LLVM/llvm/projects/compiler-rt/lib/xray/.. -I/home/nin/LLVM/llvm/projects/compiler-rt/lib/xray/../../include -O3 -DNDEBUG -fno-lto -fPIC -fno-builtin -fno-exceptions -fomit-frame-pointer -funwind-tables -fno-stack-protector -fno-sanitize=safe-stack -fvisibility=hidden -fvisibility-inlines-hidden -fno-lto -O3 -gline-tables-only -MD -MT projects/compiler-rt/lib/xray/CMakeFiles/clang_rt.xray-mipsel.dir/xray_trampoline_mips.S.o -MF projects/compiler-rt/lib/xray/CMakeFiles/clang_rt.xray-mipsel.dir/xray_trampoline_mips.S.o.d -o projects/compiler-rt/lib/xray/CMakeFiles/clang_rt.xray-mipsel.dir/xray_trampoline_mips.S.o -c /home/nin/LLVM/llvm/projects/compiler-rt/lib/xray/xray_trampoline_mips.S

clang -cc1as: error: unknown target triple 'x86_64-unknown-linux-gnu', please use -triple or -arch

My build commands is

cmake -G "Ninja" -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_FLAGS_RELEASE='' -DCMAKE_C_FLAGS="$cflags" -DCMAKE_CXX_FLAGS="$cflags -std=c++11" -DLLVM_TABLEGEN=$LLVM_TBLGEN -DCLANG_TABLEGEN=$CLANG_TBLGEN -DCMAKE_CROSSCOMPILING=1 -DLLVM_TARGETS_TO_BUILD=Mips -DLLVM_DEFAULT_TARGET_TRIPLE=$TRIPLE -DCMAKE_C_COMPILER_TARGET=$TRIPLE -DCMAKE_CXX_COMPILER_TARGET=$TRIPLE -DLLVM_HOST_TRIPLE=$TRIPLE -DLLVM_TARGET_ARCH=Mips -DCOMPILER_RT_DEFAULT_TARGET_ONLY=1 -DCOMPILER_RT_DEFAULT_TARGET_ARCH=mipsel -DCOMPILER_RT_TEST_COMPILER=$cxx_bin -DCMAKE_FIND_ROOT_PATH=$sysroot -DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM=BOTH -DCMAKE_FIND_ROOT_PATH_MODE_LIBRARY=ONLY -DCMAKE_FIND_ROOT_PATH_MODE_INCLUDE=ONLY -DCMAKE_C_COMPILER=$cc_bin -DCMAKE_CXX_COMPILER=$cxx_bin

cflags=--gcc-toolchain=/projects/mipssw/toolchains/mips-mti-linux-gnu/2015.01-7/ --target=mipsel-mti-linux-gnu -mabi=32 -EL -mips32r2 -latomic.

Let me know if I miss anything

Thanks

[dberris]

CMakeLists.txt

273 ↗

(On Diff #118327)

Aha -- this is much better, thanks.

I think I see what the problem is here, and it might be better to fix this a different way.

There's a change I prepared a while back that changes the way the runtime is built (https://reviews.llvm.org/D37372#65c567b0) which I've since abandoned. That's isolated to the XRay CMakeLists.txt file in compiler-rt, which might be a better definition conducive to cross-compilation.

I see that it might be useful to use COMPILER_RT_DEFAULT_TARGET_ARCH somehow if we're not already using it in the compiler-rt cmake helpers.

@beanz might know better how to support cross-compiler set-ups too. @eugenis might also know more about how the sanitizers achieve this better.

[eugenis]

CMakeLists.txt

273 ↗

(On Diff #118327)

I think you are right. add_compiler_rt* functions should take care of this on their own by using TARGET_{$ARCH}_FLAGS which should include CMAKE_C_FLAGS / CMAKE_CXX_FLAGS.

[nitesh.jain]

Update diff as per suggestion.

Thanks

[dberris]

For XRay, I think this is fine. Although I suspect it would be better to do this in the definition of add_compiler_rt_runtime and other add_compiler_rt_* functions instead.

I'll leave it up to you to decide whether you want to change the definitions of the helpers.

lib/xray/CMakeLists.txt

70–73

I think you should just be able to check whether TARGET_FLAGS is defined, regardless of whether CMAKE_CXX_FLAGS is defined or not.

lib/xray/CMakeLists.txt

set(powerpc64le_SOURCES
         xray_trampoline_powerpc64.cc
         xray_trampoline_powerpc64_asm.S
         ${XRAY_SOURCES})
 
 include_directories(..)
 include_directories(../../include)
 
 set(XRAY_CFLAGS ${SANITIZER_COMMON_CFLAGS})
+
+# To cross build XRAY we need to append CMAKE_C_FLAGS  
+if(DEFINED CMAKE_C_FLAGS)
+ LIST(APPEND XRAY_CFLAGS ${CMAKE_C_FLAGS})
+endif()
+
 set(XRAY_COMMON_DEFINITIONS XRAY_HAS_EXCEPTIONS=1)
 append_list_if(
   COMPILER_RT_HAS_XRAY_COMPILER_FLAG XRAY_SUPPORTED=1 XRAY_COMMON_DEFINITIONS)
 append_list_if(
   COMPILER_RT_BUILD_XRAY_NO_PREINIT XRAY_NO_PREINIT XRAY_COMMON_DEFINITIONS)
 
 add_compiler_rt_object_libraries(RTXray
   ARCHS ${XRAY_SUPPORTED_ARCH}
   SOURCES ${XRAY_SOURCES} CFLAGS ${XRAY_CFLAGS}
   DEFS ${XRAY_COMMON_DEFINITIONS})
 
 add_compiler_rt_component(xray)
 
 set(XRAY_COMMON_RUNTIME_OBJECT_LIBS