Page MenuHomePhabricator

LLVM Driver Multicall tool
Needs ReviewPublic

Authored by beanz on Sep 17 2021, 9:36 AM.

Details

Summary

This patch adds an llvm-driver multicall tool that can combine multiple
LLVM-based tools. The build infrastructure is enabled for a tool by
adding the GENERATE_DRIVER option to the add_llvm_executable CMake
call, and changing the tool's main function to a canonicalized
tool_name_main format (i.e. llvm_ar_main, clang_main, etc...).

As currently implemented llvm-driver contains dsymutil, llvm-ar,
llvm-cxxfilt, llvm-objcopy, and clang (if clang is included in the
build).

llvm-driver can be disabled from builds by setting
LLVM_TOOL_LLVM_DRIVER_BUILD=Off.

There are several limitations in the current implementation, which can
be addressed in subsequent patches:

(1) the multicall binary cannot currently properly handle
multi-dispatch tools. This means symlinking llvm-ranlib to llvm-driver
will not properly result in llvm-ar's main being called.
(2) the multicall binary cannot be comprised of tools containing
conflicting cl::opt options as the global cl::opt option list cannot
contain duplicates.

These limitations can be addressed in subsequent patches.

Diff Detail

Event Timeline

beanz created this revision.Sep 17 2021, 9:36 AM
beanz requested review of this revision.Sep 17 2021, 9:36 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptSep 17 2021, 9:36 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
aganea added a subscriber: aganea.Sep 17 2021, 9:41 AM

That's pretty nice! Have you thought about looking into a lit option (triggered by a cmake flag maybe) that would change the substitution for the tools that are enabled by llvm-driver to use the latter instead when running the tests?

phosek added inline comments.Sep 17 2021, 10:23 AM
llvm/tools/llvm-driver/CMakeLists.txt
17

As was already suggested on D104686, I'd prefer naming this just llvm so you can invoke tools with llvm name which doesn't require any additional typing compared to llvm-name.

That's pretty nice! Have you thought about looking into a lit option (triggered by a cmake flag maybe) that would change the substitution for the tools that are enabled by llvm-driver to use the latter instead when running the tests?

That's an awesome idea. I had been thinking about having a CMake option to generate symlinks instead of the tools, but you could totally bake this into the lit substitutions. I'll take a look at that.

llvm/tools/llvm-driver/CMakeLists.txt
17

Can do

beanz updated this revision to Diff 373300.Sep 17 2021, 12:04 PM

Renaming llvm-driver binary to llvm.

This is really a great change, thanks @beanz!

llvm/tools/llvm-objcopy/llvm-objcopy.cpp
404

Shouldn't we say:

int objcopy_main(int argc, char **argv) {

here and the other places + all supporting code, if we want llvm objcopy (without the dash) like @phosek suggests?

beanz added inline comments.Sep 17 2021, 2:51 PM
llvm/tools/llvm-objcopy/llvm-objcopy.cpp
404

I had a different thought for that. I think we want the tools to respond to llvm-objcopy since we will want them to exist in parallel to binutils tools just like the current tools do today.

Many of the current tools also support symlink-redirection, to support that we'll need to have a multiplex where multiple tool names point to the same main function.

Handling that was my point (1) in the main commit message, and I intended to work on it in a follow-on commit.

@beanz Is this ready to land or do you plan on making any more changes?