This is an archive of the discontinued LLVM Phabricator instance.

Differential D22987

[lldb][cmake] Remove libclang as an lldbBase dependency (NFCI)
ClosedPublic

Authored by vsk on Jul 29 2016, 5:16 PM.

Details

Reviewers
spyffe
zturner
Commits
rG181f924beb49: [lldb][cmake] Remove libclang as an lldbBase dependency (NFCI)
rLLDB279296: [lldb][cmake] Remove libclang as an lldbBase dependency (NFCI)
rL279296: [lldb][cmake] Remove libclang as an lldbBase dependency (NFCI)
Summary

It's pulling in all kinds of unused things (e.g, at this time of
writing, clang-tidy!). This needlessly slows down every build of lldb.

This passes check-lldb.

Diff Detail

Event Timeline

vsk updated this revision to Diff 66205.Jul 29 2016, 5:16 PM
vsk retitled this revision from to [lldb][cmake] Remove libclang as an lldbBase dependency (NFCI).
vsk updated this object.
vsk added reviewers: zturner, spyffe, clayborg.
vsk added a subscriber: llvm-commits.
vsk added a comment.Jul 29 2016, 6:12 PM

Along with D22986, this patch removes 453 dependencies from the lldb target.
This should be reproducible if you have {llvm,clang,clang-tools-extra} checked
out. On my iMac (4 cores), I see:

Pre-patch : ninja lldb  3814.81s user 340.08s system 361% cpu 19:08.84 total
Post-patch: ninja lldb  2485.63s user 227.54s system 371% cpu 12:09.50 total

So, a roughly 36% decrease in the time it takes to do a clean build.

I didn't average the trial runs (1 sample each) or run perf stabilization. Ultimately,
YMMV but I think this is promising.

zturner edited edge metadata.Aug 2 2016, 11:12 AM

For me:

with patch: command took 0:8:6.80 (486.80s total)
without patch: command took 0:10:49.74 (649.74s total)

So almost exactly a 25% savings. Pretty respectable. Do we at least need to add a dependency on the clang-tblgen target? Or is this ok without that?

vsk added a comment.Aug 2 2016, 11:42 AM

I can't find any #includes of headers installed by the 'install-clang-headers'
target in lldb, and 'check-lldb' seems to pass without adding in a clang-tblgen
dependency.

There could be a 'gotcha' here if there's some library we're not building along
with 'check-lldb' or 'lldb'. This doesn't seem likely, but if it happens, we
can go back and add an 'install-clang-headers' dependency for the affected
library.

For now, I don't think we need to add a 'clang-tblgen' dependency.

zturner requested changes to this revision.EditedAug 2 2016, 11:57 AM
zturner edited edge metadata.
In D22987#503639, @vsk wrote:

I can't find any #includes of headers installed by the 'install-clang-headers'
target in lldb, and 'check-lldb' seems to pass without adding in a clang-tblgen
dependency.

There could be a 'gotcha' here if there's some library we're not building along
with 'check-lldb' or 'lldb'. This doesn't seem likely, but if it happens, we
can go back and add an 'install-clang-headers' dependency for the affected
library.

For now, I don't think we need to add a 'clang-tblgen' dependency.

check-lldb isn't the problem so as much as simply guaranteeing that the clang headers are auto-generated *before* we compile any part of LLDB that attempts to #include them. I don't know what the exact right target is to build in this case, but we need a way to ensure that headers are auto-generated before compiling a library that #includes those headers.

In fact, I just did a clean build with the patch and got errors:

-- Build files have been written to: D:/src/llvmbuild/ninja
[368/2856] Building CXX object tools\lldb\source\Commands\CMakeFiles\lldbCommands.dir\CommandObjectMemory.cpp.obj
FAILED: C:\PROGRA~2\MICROS~1.0\VC\bin\AMD64_~2\cl.exe   /nologo /TP -DGTEST_HAS_RTTI=0 -DLLDB_DISABLE_CURSES -DLLDB_DISABLE_LIBEDIT -DLLDB_PYTHON_HOME=\"C:/Python35\" -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_FILE_OFFSET_BITS=64 -D_HAS_EXCEPTIONS=0 -D_LARGEFILE_SOURCE -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\lldb\source\Commands -ID:\src\llvm\tools\lldb\source\Commands -ID:\src\llvm\tools\lldb\include -Itools\lldb\include -Iinclude -ID:\src\llvm\include -IC:\Python35\Include -ID:\src\llvm\tools\clang\include -Itools\lldb\..\clang\include -ID:\src\llvm\tools\lldb\source\. /DWIN32 /D_WINDOWS   /W4 -wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4324 -w14062 -we4238 /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /Zc:sizedDealloc- /D_DEBUG /MDd /Zi /Ob0 /Od /RTC1   -wd4018 -wd4068 -wd4150 -wd4251 -wd4521 -wd4530 /D _UNICODE /D UNICODE  /EHs-c- /GR- /showIncludes /Fotools\lldb\source\Commands\CMakeFiles\lldbCommands.dir\CommandObjectMemory.cpp.obj /Fdtools\lldb\source\Commands\CMakeFiles\lldbCommands.dir\ /FS -c D:\src\llvm\tools\lldb\source\Commands\CommandObjectMemory.cpp
D:\src\llvm\tools\clang\include\clang/Basic/DiagnosticIDs.h(53): fatal error C1083: Cannot open include file: 'clang/Basic/DiagnosticCommonKinds.inc': No such file or directory
[368/2856] Building CXX object tools\lldb\source\Breakpoint\CMakeFiles\lldbBreakpoint.dir\Watchpoint.cpp.obj
FAILED: C:\PROGRA~2\MICROS~1.0\VC\bin\AMD64_~2\cl.exe   /nologo /TP -DGTEST_HAS_RTTI=0 -DLLDB_DISABLE_CURSES -DLLDB_DISABLE_LIBEDIT -DLLDB_PYTHON_HOME=\"C:/Python35\" -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_FILE_OFFSET_BITS=64 -D_HAS_EXCEPTIONS=0 -D_LARGEFILE_SOURCE -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\lldb\source\Breakpoint -ID:\src\llvm\tools\lldb\source\Breakpoint -ID:\src\llvm\tools\lldb\include -Itools\lldb\include -Iinclude -ID:\src\llvm\include -IC:\Python35\Include -ID:\src\llvm\tools\clang\include -Itools\lldb\..\clang\include -ID:\src\llvm\tools\lldb\source\. /DWIN32 /D_WINDOWS   /W4 -wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4324 -w14062 -we4238 /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /Zc:sizedDealloc- /D_DEBUG /MDd /Zi /Ob0 /Od /RTC1   -wd4018 -wd4068 -wd4150 -wd4251 -wd4521 -wd4530 /D _UNICODE /D UNICODE  /EHs-c- /GR- /showIncludes /Fotools\lldb\source\Breakpoint\CMakeFiles\lldbBreakpoint.dir\Watchpoint.cpp.obj /Fdtools\lldb\source\Breakpoint\CMakeFiles\lldbBreakpoint.dir\ /FS -c D:\src\llvm\tools\lldb\source\Breakpoint\Watchpoint.cpp
D:\src\llvm\tools\clang\include\clang/AST/ASTFwd.h(22): fatal error C1083: Cannot open include file: 'clang/AST/DeclNodes.inc': No such file or directory
[368/2856] Building CXX object tools\lldb\source\Commands\CMakeFiles\lldbCommands.dir\CommandObjectArgs.cpp.obj
FAILED: C:\PROGRA~2\MICROS~1.0\VC\bin\AMD64_~2\cl.exe   /nologo /TP -DGTEST_HAS_RTTI=0 -DLLDB_DISABLE_CURSES -DLLDB_DISABLE_LIBEDIT -DLLDB_PYTHON_HOME=\"C:/Python35\" -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_FILE_OFFSET_BITS=64 -D_HAS_EXCEPTIONS=0 -D_LARGEFILE_SOURCE -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\lldb\source\Commands -ID:\src\llvm\tools\lldb\source\Commands -ID:\src\llvm\tools\lldb\include -Itools\lldb\include -Iinclude -ID:\src\llvm\include -IC:\Python35\Include -ID:\src\llvm\tools\clang\include -Itools\lldb\..\clang\include -ID:\src\llvm\tools\lldb\source\. /DWIN32 /D_WINDOWS   /W4 -wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4324 -w14062 -we4238 /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /Zc:sizedDealloc- /D_DEBUG /MDd /Zi /Ob0 /Od /RTC1   -wd4018 -wd4068 -wd4150 -wd4251 -wd4521 -wd4530 /D _UNICODE /D UNICODE  /EHs-c- /GR- /showIncludes /Fotools\lldb\source\Commands\CMakeFiles\lldbCommands.dir\CommandObjectArgs.cpp.obj /Fdtools\lldb\source\Commands\CMakeFiles\lldbCommands.dir\ /FS -c D:\src\llvm\tools\lldb\source\Commands\CommandObjectArgs.cpp
D:\src\llvm\tools\clang\include\clang/AST/ASTFwd.h(22): fatal error C1083: Cannot open include file: 'clang/AST/DeclNodes.inc': No such file or directory
[368/2856] Building CXX object tools\lldb\source\Commands\CMakeFiles\lldbCommands.dir\CommandObjectFrame.cpp.obj
FAILED: C:\PROGRA~2\MICROS~1.0\VC\bin\AMD64_~2\cl.exe   /nologo /TP -DGTEST_HAS_RTTI=0 -DLLDB_DISABLE_CURSES -DLLDB_DISABLE_LIBEDIT -DLLDB_PYTHON_HOME=\"C:/Python35\" -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_FILE_OFFSET_BITS=64 -D_HAS_EXCEPTIONS=0 -D_LARGEFILE_SOURCE -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\lldb\source\Commands -ID:\src\llvm\tools\lldb\source\Commands -ID:\src\llvm\tools\lldb\include -Itools\lldb\include -Iinclude -ID:\src\llvm\include -IC:\Python35\Include -ID:\src\llvm\tools\clang\include -Itools\lldb\..\clang\include -ID:\src\llvm\tools\lldb\source\. /DWIN32 /D_WINDOWS   /W4 -wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4324 -w14062 -we4238 /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /Zc:sizedDealloc- /D_DEBUG /MDd /Zi /Ob0 /Od /RTC1   -wd4018 -wd4068 -wd4150 -wd4251 -wd4521 -wd4530 /D _UNICODE /D UNICODE  /EHs-c- /GR- /showIncludes /Fotools\lldb\source\Commands\CMakeFiles\lldbCommands.dir\CommandObjectFrame.cpp.obj /Fdtools\lldb\source\Commands\CMakeFiles\lldbCommands.dir\ /FS -c D:\src\llvm\tools\lldb\source\Commands\CommandObjectFrame.cpp
D:\src\llvm\tools\clang\include\clang/AST/ASTFwd.h(22): fatal error C1083: Cannot open include file: 'clang/AST/DeclNodes.inc': No such file or directory
[368/2856] Python script building LLDB Python wrapper
ninja: build stopped: subcommand failed.

I didn't get any errors the previous time because it wasn't a clean build. The headers had already been generated. So we do need to add some kind of dependency (even if it is not a link time dependency as it was with libclang)

This revision now requires changes to proceed.Aug 2 2016, 11:57 AM
vsk added a comment.Aug 2 2016, 12:37 PM

I can reproduce that if I build CommandObjectFrame.cpp.o in isolation. I'll take
another shot at this.

vsk updated this revision to Diff 66536.Aug 2 2016, 1:12 PM
vsk edited edge metadata.

Make it possible to build single object files in lldb libraries. This is done by making each library depend on all CLANG_USED_LIBS. Building a single object file now forces clang-tblgen to run, so we end up with the right headers in place. We still see the speed benefit of removing the libclang dependency:

TargetPre-patch # of dependenciesPost-patch # of dependencies
CommandObjectFrame.cpp.o1622126
lldb26422188
zturner added a comment.Aug 2 2016, 1:18 PM

Are the timing results the same with this patch and the original patch, or is this one a little bit slower?

cmake/modules/AddLLDB.cmake
87–90

I think you can just write:

add_dependencies(${name} ${CLANG_USED_LIBS})
vsk updated this revision to Diff 66582.Aug 2 2016, 3:59 PM
vsk edited edge metadata.

I'm glad you asked! While I was measuring this I went back to double-check what
I'd done, and found a few build edges in my ninja file that made no sense :(.
I had only partially cleared away my build directory, so the results I posted
in my last comment are bogus.

In this updated diff, I added an include of LLDBDependencies.cmake to make sure
that CLANG_USED_LIBS is visible everywhere we call add_lldb_library(). This is
what was missing from my previous patch.

Here are some less bogus numbers. I've highlighted the number which changed:

TargetPre-patch # of depsPre-patch build timePost-patch # of depsPost-patch build time
CommandObjectFrame.cpp.o1622?566?
lldb264219 min218812 min
vsk marked an inline comment as done.Aug 2 2016, 4:00 PM
vsk added inline comments.
cmake/modules/AddLLDB.cmake
87–90

When I try doing this, I get a syntax error:

CMake Error at tools/lldb/cmake/modules/AddLLDB.cmake:88 (add_dependencies):
  add_dependencies called with incorrect number of arguments
Call Stack (most recent call first):
  tools/lldb/source/Symbol/CMakeLists.txt:1 (add_lldb_library)
vsk updated this revision to Diff 66584.Aug 2 2016, 4:03 PM
vsk marked an inline comment as done.
  • Remove the foreach loop. It only caused a syntax error earlier because CLANG_USED_LIBS wasn't always defined.
clayborg resigned from this revision.Aug 18 2016, 3:52 PM
clayborg removed a reviewer: clayborg.

As long as everything builds and the bots are happy I am good with this as long as the other cmake experts are

Closed by commit rL279296: [lldb][cmake] Remove libclang as an lldbBase dependency (NFCI) (authored by vedantk). · Explain WhyAug 19 2016, 11:49 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
1 line
cmake/
modules/
7 lines

Diff 66582

CMakeLists.txt

cmake_minimum_required(VERSION 3.4.3) cmake_minimum_required(VERSION 3.4.3)
include(cmake/LLDBDependencies.cmake)
include(cmake/modules/LLDBStandalone.cmake) include(cmake/modules/LLDBStandalone.cmake)
include(cmake/modules/LLDBConfig.cmake) include(cmake/modules/LLDBConfig.cmake)
include(cmake/modules/AddLLDB.cmake) include(cmake/modules/AddLLDB.cmake)
if (__ANDROID_NDK__ OR (CMAKE_SYSTEM_NAME MATCHES "Windows")) if (__ANDROID_NDK__ OR (CMAKE_SYSTEM_NAME MATCHES "Windows"))
set(LLDB_DEFAULT_DISABLE_LIBEDIT 1) set(LLDB_DEFAULT_DISABLE_LIBEDIT 1)
else() else()
set(LLDB_DEFAULT_DISABLE_LIBEDIT 0) set(LLDB_DEFAULT_DISABLE_LIBEDIT 0)
▲ Show 20 Lines • Show All 52 Lines • Show Last 20 Lines

cmake/modules/AddLLDB.cmake

Show First 20 Lines • Show All 78 Lines • ▼ Show 20 Lines if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ${name} STREQUAL "liblldb")
else() else()
install(TARGETS ${name} install(TARGETS ${name}
LIBRARY DESTINATION lib${LLVM_LIBDIR_SUFFIX} LIBRARY DESTINATION lib${LLVM_LIBDIR_SUFFIX}
ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX}) ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX})
endif() endif()
endif() endif()
endif() endif()
# Hack: only some LLDB libraries depend on the clang autogenerated headers, foreach(clang_dep ${CLANG_USED_LIBS})
# but it is simple enough to make all of LLDB depend on some of those add_dependencies(${name} ${clang_dep})
# headers without negatively impacting much of anything. endforeach(clang_dep)
add_dependencies(${name} libclang)
zturnerUnsubmitted
Done
ReplyInline Actions

I think you can just write:

add_dependencies(${name} ${CLANG_USED_LIBS})
zturner: I think you can just write: ``` add_dependencies(${name} ${CLANG_USED_LIBS}) ```
vskAuthorUnsubmitted
Not Done
ReplyInline Actions

When I try doing this, I get a syntax error:

CMake Error at tools/lldb/cmake/modules/AddLLDB.cmake:88 (add_dependencies):
  add_dependencies called with incorrect number of arguments
Call Stack (most recent call first):
  tools/lldb/source/Symbol/CMakeLists.txt:1 (add_lldb_library)
vsk: When I try doing this, I get a syntax error: ``` CMake Error at…
set_target_properties(${name} PROPERTIES FOLDER "lldb libraries") set_target_properties(${name} PROPERTIES FOLDER "lldb libraries")
endmacro(add_lldb_library) endmacro(add_lldb_library)
macro(add_lldb_executable name) macro(add_lldb_executable name)
add_llvm_executable(${name} DISABLE_LLVM_LINK_LLVM_DYLIB ${ARGN}) add_llvm_executable(${name} DISABLE_LLVM_LINK_LLVM_DYLIB ${ARGN})
set_target_properties(${name} PROPERTIES FOLDER "lldb executables") set_target_properties(${name} PROPERTIES FOLDER "lldb executables")
endmacro(add_lldb_executable) endmacro(add_lldb_executable)
Show All 15 Lines