This is an archive of the discontinued LLVM Phabricator instance.

[docs] Corrected inaccuracies in Common Problems section
ClosedPublic

Authored by e-leclercq on Apr 2 2020, 4:27 PM.

Details

Summary

Changed the language in LLVM_USE_LINKER to more strongly recommend LLD and to specify that the GNU gold linker is only useful if LLD is unavailable in binary form and it is the first build of LLVM. Added that LLD will help when used on ELF-based platforms. Corrected information in CMAKE_BUILD_TYPE regarding the Release build type and enabling assertions. Added option LLVM_ENABLE_ASSERTIONS and mentioned enabling this option with a Release build as an alternative to using a Debug build. Specified that the LLVM_OPTIMIZED_TABLEGEN option is only for Debug builds, that the LLVM_USE_SPLIT_DWARF option is only available on ELF host platforms, and that setting CLANG_ENABLE_STATIC_ANALYZER to OFF only slightly improves build time. These changes address comments made in D75425.

Diff Detail

Event Timeline

e-leclercq created this revision.Apr 2 2020, 4:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 2 2020, 4:27 PM

In the summary, could you mention this addresses additional comments from D75425?

llvm/docs/GettingStarted.rst
1108

Could you clarify that it reduces linking time for the LLVM excecutables (and not e.g. performance of programs compiled with clang)

1156

I am not sure this is Linux-only. It certainly requires DWARF debug information (.i.e no Windows PDB), but It will probably work on BSDs and maybe also using the target x86_64-w64-mingw32.

e-leclercq edited the summary of this revision. (Show Details)Apr 3 2020, 11:08 AM
e-leclercq marked an inline comment as done.Apr 3 2020, 4:00 PM
e-leclercq added inline comments.
llvm/docs/GettingStarted.rst
1156

I read up on this a bit more and it appears that this works on both UNIX and Linux, so I will definitely change that. I am not quite sure about Windows though, as I came across this article. Would changing this to "This option can be used on both UNIX and Linux" be adequate?

Meinersbur added inline comments.Apr 6 2020, 9:33 AM
llvm/docs/GettingStarted.rst
1156

Since the implementation is:

if (LLVM_USE_SPLIT_DWARF AND
    ((uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG") OR
     (uppercase_CMAKE_BUILD_TYPE STREQUAL "RELWITHDEBINFO")))
  # Limit to clang and gcc so far. Add compilers supporting this option.
  if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR
      CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
    add_compile_options(-gsplit-dwarf)
  endif()
endif()

AFAIK gcc only supports DWARF. Clang supports split-dwarf for ELF targets.

I'd make it a note saying that the option may not be supported on all platforms. On Windows the platform-standard is codeview (.pdb) which is always a separate file. I don't know about MacOS.

aprantl added inline comments.Apr 6 2020, 1:58 PM
llvm/docs/GettingStarted.rst
1156

Split DWARF is a DWARF+ELF thing, but not tied to an OS. It's specifically not needed/useful/available for Mach-O (Darwin) because debug info isn't processed by Mach-O linkers at all (instead there is dsymutil).

Notable other platforms using ELF are the BSDs, Fuchsia, PlayStation, ...

e-leclercq marked 4 inline comments as done.Apr 10 2020, 10:59 AM
e-leclercq edited the summary of this revision. (Show Details)

Corrected the LLVM_USE_SPLIT_DWARF option. Clarified that setting the LLVM_USE_LINKER option and using lld reduces linking time for LLVM executables.

aprantl added inline comments.Apr 10 2020, 2:55 PM
llvm/docs/GettingStarted.rst
1115

This is only true for ELF targets (I don't know about COFF). In the Mach-O case (i.e., on Darwin/macOS) the presence of de bug info has zero effect on the linker performance, since the linker doesn't touch the debug info at all. Can we prefix that with On Linux, or something similar?

aprantl added inline comments.Apr 10 2020, 2:57 PM
llvm/docs/GettingStarted.rst
1156

This option may not be supported on all platforms.

Why be so vague, if we know the answer? Can we just say that this only applies to ELF (e.g, Linux) host platforms?

Meinersbur added inline comments.Apr 10 2020, 3:10 PM
llvm/docs/GettingStarted.rst
1156

My thinking was that beginners may not necessarily know what ELF is. But clarifying in parens is a good idea.

e-leclercq marked an inline comment as done.Apr 13 2020, 7:14 PM
e-leclercq marked 2 inline comments as done.Apr 14 2020, 1:09 PM
e-leclercq edited the summary of this revision. (Show Details)

Added more information in CMAKE_BUILD_TYPE option. Specified that info under LLVM_USE_SPLIT_DWARF only applies to host platforms using ELF.

aprantl added inline comments.Apr 15 2020, 4:29 PM
llvm/docs/GettingStarted.rst
1114–1130

On platforms using ELF (such as Linux), this ...

rnk added inline comments.Apr 15 2020, 4:54 PM
llvm/docs/GettingStarted.rst
1108–1111

It occurs to me that these recommendations are not really applicable on platforms that don't use GNU binutils ld.bfd by default. Despite my comment below, it would make sense to add some kind of "... on platforms that use ELF, such as Linux" into this. =/

1114–1130

Actually, can I give the opposite feedback? I think adding platform specific admonitions can be confusing. Some day, someone will add an option to run dsymutil as a post-link step on Darwin, and then this statement will be true about Darwin as well as Linux. It is already true for Windows, whether using PDBs or DWARF.

Instead, would it be OK to soften it with This *can* cause high memory usage during the linking phase., so that we can simplify away the condition?

aprantl added inline comments.Apr 16 2020, 10:01 AM
llvm/docs/GettingStarted.rst
1109

This text also does not apply to Darwin at all, where you would always want to use the system linker (until lld supports Mach-O).

1114–1130

You can already run dsymutil as a post-processing step using the LLVM_EXTERNALIZE_DEBUGINFO cmake option. That's how the clang that ships with Xcode is being built. But it still doesn't come with the same kind of memory explosion that you would get on ELF without split dwarf, since dsymutil (generally speaking) only processes one file at a time and performs type uniquing. I just want to avoid developers cargo-culting configuration options that make sense on one platform to another. On Darwin, the cost of building with debug info is negligible and I want to encourage new developers to use all the great tools we have available in the LLVM project. I don't want people to be afraid of using a debugger on LLVM.

So much for my rant ;-)

I think this text might also benefit from distinguishing between optimizations and debug info (Debug/Release/RelWithDebInfo) which are two axes unfortunately rolled into one CMake setting.

e-leclercq added inline comments.Apr 16 2020, 1:26 PM
llvm/docs/GettingStarted.rst
1114–1130

Would clarifying what platforms use ELF in the first paragraph (lines 1098-1100), then marking which fixes are ELF specific in parentheses similar to this subsection in the LLD documentation, be agreeable?

aprantl added inline comments.Apr 17 2020, 9:44 AM
llvm/docs/GettingStarted.rst
1114–1130

I understand that this not supposed to be a full documentation, but a quick start guide, so let me know if this is going too far. What do you think about discussing all the options separately?

* -DCMAKE_BUILD_TYPE
 - Debug
   This is the default. It turns off optimizations (while compiling LLVM itself), enables assertions, and turns on debug info.
 - RelWithDebInfo
   Turns on optimizations, and enables debug info. This results in fast compiler that can still be inspected by a debugger.
 - Release
   Turns on optimizations, and disables debug info. On some platforms (e.g, ELF-based platforms, such as Linux) linking with debug info may consume a lot of memory ...

   Combining Release or RelWithDebInfo with -DLLVM_ENABLE_ASSERTIONS can be a good trade-off between speed and debugability during development, particularly for running the test suite.
 - MinSizeRel
   ...
e-leclercq marked 7 inline comments as done.May 2 2020, 5:38 PM
e-leclercq updated this revision to Diff 261676.May 2 2020, 6:07 PM
e-leclercq edited the summary of this revision. (Show Details)

Reformatted CMAKE_BUILD_TYPE section, adding more specific information on build types. Specified that setting lld with -DLLVM_USE_LINKER helps with memory usage and build time on ELF-based platforms.

Meinersbur added inline comments.May 4 2020, 9:23 AM
llvm/docs/GettingStarted.rst
1114–1130
1116

assertions are controlled by LLVM_ENABLE_ASSERTIONS, not CMAKE_BUILD_TYPE

1122–1125

I'd put the warning about lot's of memory at Debug (and RelWithDebInfo), which causes them.

1127–1128

MinSizeRel is for compiling with -Os yielding slighly smaller executables, but is not relevant for build times or memory consumption.

Think MinSizeRel or RelWithDebInfo should not be mentioned here. This is not a CMake guide, but intended to help in case of build problems.

e-leclercq marked 5 inline comments as done.May 8 2020, 11:15 AM
e-leclercq added inline comments.
llvm/docs/GettingStarted.rst
1116

I read in the CMake.html doc here that LLVM_ENABLE_ASSERTIONS defaults to ON iff the build type is Debug

e-leclercq marked an inline comment as done.May 8 2020, 11:18 AM
e-leclercq edited the summary of this revision. (Show Details)

Moved warning about memory consumption during linking on ELF-based platforms to Debug. Removed RelWithDebInfo and MinSizeRel, as they don't appear to be relevant to reducing build time or memory usage.

Meinersbur added inline comments.May 8 2020, 12:30 PM
llvm/docs/GettingStarted.rst
1116

Yes, it defaults to ON/OFF depending on CMAKE_BUILD_TYPE, but once the CMakeCache.txt has been created, it will not change when CMAKE_BUILD_TYPE is changed.

I recommend not to rely on the default value if you want to have it a specific value.

1121

Don't forget the =ON part for -DLLVM_ENABLE_ASSERTIONS. The cmake invocation will fail without an argument.

(btw, cmake interprets the empty string as false)

I'm happy with the text changes now.

e-leclercq marked 2 inline comments as done.May 11 2020, 7:49 AM

Removed misleading information regarding both build type and enabling assertions under CMAKE_BUILD_TYPE option. Added =ON to -DLLVM_ENABLE_ASSERTIONS option in paragraph on Release build type to leave no room for confusion.

Meinersbur accepted this revision.May 11 2020, 1:01 PM

LGTM.

Do you want me to commit this patch?

This revision is now accepted and ready to land.May 11 2020, 1:01 PM

@Meinersbur, sure. That would be great!

It would be nice if you could remove trailing whitespace in your patches. I removed them before pushing.

This revision was automatically updated to reflect the committed changes.