This is an archive of the discontinued LLVM Phabricator instance.

[ORC-RT] Refactor ORC runtime CMake for future test tool(s).
ClosedPublic

Authored by lhames on Aug 31 2022, 8:19 PM.

Details

Summary

We want to move functionality from the LLVM ORCTargetProcess library into the
ORC runtime, and this will mean implementing remote-executor testing tools
(like llvm-jitlink-executor and lli-child-target) in the ORC runtime.

This patch refactors the ORC runtime build system to introduce an
add_orc_tool function that can be used to add new test tools. The code is
modeled on existing functions for adding unit tests.

A placeholder orc-rt-executor tool and test are added to verify that the
config changes behave as expected.

Diff Detail

Event Timeline

lhames created this revision.Aug 31 2022, 8:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2022, 8:19 PM
lhames requested review of this revision.Aug 31 2022, 8:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2022, 8:19 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript

Modeling the tools code on unit tests was the easiest way for me to get up and running here, but if there's a better way to build test programs for use with lit I'm happy to use that too.

phosek added inline comments.Sep 2 2022, 12:45 AM
compiler-rt/lib/orc/tests/CMakeLists.txt
15

Do you see a build failure without this flag? This should now be enabled globally for all of compiler-rt: https://github.com/llvm/llvm-project/blob/9185d6e6bca8ec41b48661c371e813498f840455/compiler-rt/CMakeLists.txt#L71

lhames added inline comments.Sep 2 2022, 2:46 PM
compiler-rt/lib/orc/tests/CMakeLists.txt
15

@phosek I do see a failure without it. The problem is that generate_compiler_rt_tests results in a call to clang_compile, which uses add_custom_command to perform the compilation: https://github.com/llvm/llvm-project/blob/9185d6e6bca8ec41b48661c371e813498f840455/compiler-rt/cmake/Modules/CompilerRTCompile.cmake#L103.

Since add_custom_command doesn't know about CMAKE_CXX_STANDARD we don't set the standard in that command.

Looks like gwp_asan hit a similar issue recently -- they also specify c++17 manually in their config: https://github.com/llvm/llvm-project/commit/474145c0b2420cb316bb8a9dcc031d613679d496

phosek accepted this revision.Sep 2 2022, 2:51 PM

LGTM

compiler-rt/lib/orc/tests/CMakeLists.txt
15

OK that's something we should address but it doesn't need to block this change.

compiler-rt/lib/orc/tests/tools/orc-rt-executor.cpp
2

Should this file also have the copyright header?

This revision is now accepted and ready to land.Sep 2 2022, 2:51 PM
lhames added inline comments.Sep 2 2022, 4:20 PM
compiler-rt/lib/orc/tests/tools/orc-rt-executor.cpp
2

Yep. Thanks for catching that -- I'll add one.