This is an archive of the discontinued LLVM Phabricator instance.

[BOLT] Fix runtime osx cross-compile build
ClosedPublic

Authored by yota9 on Feb 4 2022, 2:34 PM.

Details

Summary

Place include elf.h under !apple condition

Diff Detail

Event Timeline

yota9 created this revision.Feb 4 2022, 2:34 PM
yota9 requested review of this revision.Feb 4 2022, 2:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 4 2022, 2:34 PM
Amir accepted this revision.Feb 4 2022, 3:13 PM
Amir added a subscriber: alexander-shaposhnikov.

CMAKE_SYSTEM_NAME is the compilation target, so it makes sense to use it instead of compiler id. We may need to add a Darwin cross-compilation target for build testing to prevent bitrot.

cc @alexander-shaposhnikov

This revision is now accepted and ready to land.Feb 4 2022, 3:13 PM
yota9 added inline comments.Feb 5 2022, 8:00 AM
bolt/runtime/CMakeLists.txt
40 ↗(On Diff #406108)

Probably the target line could be removed now, how do you think @Amir @alexander-shaposhnikov ?

alexander-shaposhnikov requested changes to this revision.Feb 7 2022, 3:25 AM
alexander-shaposhnikov added inline comments.
bolt/runtime/CMakeLists.txt
33 ↗(On Diff #406108)

unless I'm missing something this appears to be incorrect.
Basically we used to build the osx runtime library regardless of the host platform
(clang works as a cross compiler).

This revision now requires changes to proceed.Feb 7 2022, 3:25 AM
bolt/runtime/CMakeLists.txt
33 ↗(On Diff #406108)

@Amir - the runtime is not linked into BOLT, instead, it's used for creating the final output, thus the target for which we are building the project is not directly related to the target platforms supported by the tool itself.

yota9 added inline comments.Feb 7 2022, 4:20 AM
bolt/runtime/CMakeLists.txt
33 ↗(On Diff #406108)

The thing is that the clang may not support this target. I'm building it in arm linux env and it fails expectedly

Amir added inline comments.Feb 7 2022, 10:02 AM
bolt/runtime/CMakeLists.txt
33 ↗(On Diff #406108)

@alexander-shaposhnikov
Thanks for clarifying. My mistake.

@yota9
Are you using custom-built clang? IIUC clang is supposed to be a cross-compiler out of the box. As a workaround, we can group rt_instr targets similar to LLVM_ENABLE_TARGETS and select them at configuration time.

yota9 marked 3 inline comments as done.Feb 7 2022, 10:20 AM
yota9 added inline comments.
bolt/runtime/CMakeLists.txt
33 ↗(On Diff #406108)

You are right, this is my fault. Let me update the patch

yota9 updated this revision to Diff 406522.Feb 7 2022, 10:24 AM
yota9 marked an inline comment as done.

Fix header include

yota9 added a comment.Feb 7 2022, 10:26 AM

@Amir The problem was not with the compiler, but with the elf dependency on darwin build. I'm not quite sure that we need config.h now, but it is definitely OK to put it under !apple condition.

yota9 retitled this revision from [BOLT] Fix bolt_rt_instr for osx build condition to [BOLT] Fix runtime osx cross-compile build.Feb 7 2022, 4:14 PM
yota9 edited the summary of this revision. (Show Details)
Amir added a comment.Feb 7 2022, 4:19 PM

What again was the problem? I didn't have compilation issues when building in aarch64 docker (emulation mode).

yota9 added a comment.Feb 7 2022, 4:29 PM

@Amir Do you use clang as CC/CXX for building BOLT? Also don't forget that currently the build for aarch64 is disabled in cmake.

/usr/bin/clang++   -I. -O3 -DNDEBUG   -target x86_64-apple-darwin19.6.0 -ffreestanding -fno-exceptions -fno-rtti -fno-stack-protector -std=c++11 -MD -MT CMakeFiles/bolt_rt_instr_osx.dir/instr.cpp.o -MF CMakeFiles/bolt_rt_instr_osx.dir/instr.cpp.o.d -o CMakeFiles/bolt_rt_instr_osx.dir/instr.cpp.o -c /home/yota9/src/llvm/bolt/runtime/instr.cpp
In file included from /home/yota9/src/llvm/bolt/runtime/instr.cpp:44:
In file included from /home/yota9/src/llvm/bolt/runtime/common.h:13:
In file included from /usr/include/elf.h:22:
/usr/include/features.h:461:12: fatal error: 'sys/cdefs.h' file not found
#  include <sys/cdefs.h>

It does not look as an ARM problem to me. The thing is that HAVE_ELF_H is defined in config.h file since it is defining for the host compiler, but used for cross compile tool. I assume the osx instrumentation is working because in instr.cpp we have:

#if defined(HAVE_ELF_H) and !defined(__APPLE__)

And the second one is not true for the cross tool. But anyway we should not try include elf.h for non-linux target.

Amir accepted this revision.Feb 7 2022, 4:38 PM

@Amir Do you use clang as CC/CXX for building BOLT? Also don't forget that currently the build for aarch64 is disabled in cmake.

/usr/bin/clang++   -I. -O3 -DNDEBUG   -target x86_64-apple-darwin19.6.0 -ffreestanding -fno-exceptions -fno-rtti -fno-stack-protector -std=c++11 -MD -MT CMakeFiles/bolt_rt_instr_osx.dir/instr.cpp.o -MF CMakeFiles/bolt_rt_instr_osx.dir/instr.cpp.o.d -o CMakeFiles/bolt_rt_instr_osx.dir/instr.cpp.o -c /home/yota9/src/llvm/bolt/runtime/instr.cpp
In file included from /home/yota9/src/llvm/bolt/runtime/instr.cpp:44:
In file included from /home/yota9/src/llvm/bolt/runtime/common.h:13:
In file included from /usr/include/elf.h:22:
/usr/include/features.h:461:12: fatal error: 'sys/cdefs.h' file not found
#  include <sys/cdefs.h>

It does not look as an ARM problem to me. The thing is that HAVE_ELF_H is defined in config.h file since it is defining for the host compiler, but used for cross compile tool. I assume the osx instrumentation is working because in instr.cpp we have:

#if defined(HAVE_ELF_H) and !defined(__APPLE__)

And the second one is not true for the cross tool. But anyway we should not try include elf.h for non-linux target.

Ah, yes, I've seen this issue but worked around it. Thanks for properly fixing it! This reason is sufficient:

we should not try include elf.h for non-linux target.

This revision was not accepted when it landed; it landed in state Needs Review.Feb 7 2022, 4:43 PM
This revision was automatically updated to reflect the committed changes.