[PATCH] D43578: -ftime-report switch support in Clang

[Andrew V. Tischenko via Phabricator via llvm-commits]

avt77 added a comment.

In https://reviews.llvm.org/D43578#1062950, @thakis wrote:

> @davezarzycki remarks in https://reviews.llvm.org/D45485 that this breaks the shared build. The proposed fix there is to make several of clang's modules depend on LLVM's IR library ("Core"). This seems weird to me for two reasons, one architectural, one form a build point of view:
>
> 1. The modules growing the dep don't really depend on IR, they just need that one bool that happens to be defined there. That bool is called `TimePassesIsEnabled` which is a reasonable bool to live in IR, but this patch starts using that bool for meanings other than "should we time passes?". It instead uses the same bool to decide if clang should print a bunch of timing info. We probably should have a separate bool in clang and key this off that and make -ftime-report set both.
> 2. From a build PoV, depending on Core means explicitly depending on TableGen processing the Attributes.td and Intrinsics.td files in include/llvm/IR, which needlessly (explicitly) serializes the build.


In fact the current trunk already depends on TimePassesIsEnabled (w/o this patch applied):

//$ find clang -name \*.cpp | xargs grep TimePassesIsEnabled
clang/lib/CodeGen/CodeGenAction.cpp:      llvm::TimePassesIsEnabled = TimePasses;
clang/lib/CodeGen/CodeGenAction.cpp:      if (llvm::TimePassesIsEnabled)
clang/lib/CodeGen/CodeGenAction.cpp:      if (llvm::TimePassesIsEnabled)
clang/lib/CodeGen/CodeGenAction.cpp:      if (llvm::TimePassesIsEnabled) {
clang/lib/CodeGen/CodeGenAction.cpp:      if (llvm::TimePassesIsEnabled) {
clang/lib/CodeGen/CodeGenAction.cpp:      if (llvm::TimePassesIsEnabled)
clang/lib/CodeGen/CodeGenAction.cpp:      if (llvm::TimePassesIsEnabled)
clang/lib/CodeGen/CodeGenAction.cpp:        if (llvm::TimePassesIsEnabled) {
clang/lib/CodeGen/CodeGenAction.cpp:        if (llvm::TimePassesIsEnabled) {
clang/lib/CodeGen/BackendUtil.cpp:  TimeRegion Region(llvm::TimePassesIsEnabled ? &CodeGenerationTime : nullptr);
clang/lib/CodeGen/BackendUtil.cpp:  TimeRegion Region(llvm::TimePassesIsEnabled ? &CodeGenerationTime : nullptr);
//
But I agree that such dependence is not OK. I'll create a separate bool instead of TimePassesIsEnabled but the question is should I remove the above usage of TimePassesIsEnabled as well? Or maybe it should be a separate pre-patch? Or it could be left as it is?


Repository:
  rL LLVM

https://reviews.llvm.org/D43578