This is an archive of the discontinued LLVM Phabricator instance.

[clang] Enable clang build with LLVM_BUILD_INSTRUMENTED without setting LLVM_PROFTDATA
ClosedPublic

Authored by alexander-shaposhnikov on Oct 12 2017, 12:46 PM.

Details

Summary

At the moment if LLVM_BUILD_INSTRUMENTED is set to True
one has to set LLVM_PROFTDATA even if it's not necessary (because of message(FATAL_ERROR ...)).
Building instrumented Clang is useful even if one doesn't plan to use the target generate-profdata
(currently that target would use only llvm/tools/clang/utils/perf-training/cxx/hello_world.cpp as a source).
For example, one can run the instrumented version of Clang via a separate build system against a different codebase,
collect the profiles and merge them by lllvm-profdata later.

Diff Detail

Repository
rL LLVM

Event Timeline

vsk added a subscriber: vsk.Oct 12 2017, 5:20 PM

llvm-profdata is tightly coupled with the host compiler: while this setup may work if you get lucky, I don't think it's resilient to changes in libProfData. Also, using the instrumented llvm-profdata will be slow and create extra profiles.

As an alternative, have you considered building an llvm-profdata compatible with your host compiler in a separate step?

yeah, i agree, this is not a good idea. My thoughts were different - right now it's not particularly convenient that one has to specify LLVM_PROFDATA when it's not actually used by the build.
Maybe we can create the target "generate-profdata" only if LLVM_PROFDATA is set (but don't fail otherwise) ?

vsk added a comment.Oct 12 2017, 5:29 PM
In D38859#896517, @alexshap wrote:

yeah, i agree, this is not a good idea. My thoughts were different - right now it's not particularly convenient that one has to specify LLVM_PROFDATA when it's not actually used by the build.
Maybe we can create the target "generate-profdata" only if LLVM_PROFDATA is set (but don't fail otherwise) ?

+ 1, sgtm

vsk accepted this revision.Oct 12 2017, 5:57 PM

Thanks!

This revision is now accepted and ready to land.Oct 12 2017, 5:57 PM
This revision was automatically updated to reflect the committed changes.