[CMAKE][llvm-libraries] Add Precompiled Headers option to improve build times #91755
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
llvmbot
added
cmake
|
@llvm/pr-subscribers-llvm-support @llvm/pr-subscribers-llvm-binary-utilities Author: Ameer J (ameerj) ChangesOpening this PR to start a discussion on supporting PCH for LLVM Libraries. The changes are currently a "proof of concept" that are meant to be a starting point for discussion. The candidates for headers to be included in the PCH are chosen based on the analysis of Visual Studio's Some notes:
Patch is 52.87 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/91755.diff 15 Files Affected:
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index c06e661573ed4..e36abfaf475f0 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -1410,3 +1410,6 @@ endif()
if (LLVM_INCLUDE_UTILS AND LLVM_INCLUDE_TOOLS)
add_subdirectory(utils/llvm-locstats)
endif()
+
+include(LLVMPrecompiledHeaders)
+llvm_lib_precompiled_headers()
diff --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 185266c0861e8..7d7a451adc93c 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -538,6 +538,8 @@ endif()
option(LLVM_ENABLE_WARNINGS "Enable compiler warnings." ON)
+option(LLVM_ENABLE_PRECOMPILED_HEADERS "Enable precompiled headers to improve build times." ON)
+
if( MSVC )
# Add definitions that make MSVC much less annoying.
@@ -593,7 +595,11 @@ if( MSVC )
# PDBs without changing codegen.
option(LLVM_ENABLE_PDB OFF)
if (LLVM_ENABLE_PDB AND uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE")
- append("/Zi" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+ if (LLVM_ENABLE_PRECOMPILED_HEADERS)
+ append("/Z7" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+ else()
+ append("/Zi" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+ endif()
# /DEBUG disables linker GC and ICF, but we want those in Release mode.
append("/DEBUG /OPT:REF /OPT:ICF"
CMAKE_EXE_LINKER_FLAGS CMAKE_MODULE_LINKER_FLAGS
diff --git a/llvm/cmake/modules/LLVMPrecompiledHeaders.cmake b/llvm/cmake/modules/LLVMPrecompiledHeaders.cmake
new file mode 100644
index 0000000000000..ac6efe9e4599a
--- /dev/null
+++ b/llvm/cmake/modules/LLVMPrecompiledHeaders.cmake
@@ -0,0 +1,35 @@
+macro(get_all_targets_recursive targets dir)
+ get_property(subdirectories DIRECTORY ${dir} PROPERTY SUBDIRECTORIES)
+ foreach(subdir ${subdirectories})
+ get_all_targets_recursive(${targets} ${subdir})
+ endforeach()
+
+ get_property(current_targets DIRECTORY ${dir} PROPERTY BUILDSYSTEM_TARGETS)
+ list(APPEND ${targets} ${current_targets})
+endmacro()
+
+function(get_all_targets dir outvar)
+ set(targets)
+ get_all_targets_recursive(targets ${dir})
+ set(${outvar} ${targets} PARENT_SCOPE)
+endfunction()
+
+function(add_llvm_lib_precompiled_headers target)
+ if (LLVM_ENABLE_PRECOMPILED_HEADERS)
+ get_target_property(target_type ${target} TYPE)
+ if (target_type STREQUAL "STATIC_LIBRARY")
+ target_precompile_headers(
+ ${target}
+ PRIVATE
+ "$<$<COMPILE_LANGUAGE:CXX>:${LLVM_MAIN_INCLUDE_DIR}/llvm/PrecompiledHeaders.h>"
+ )
+ endif()
+ endif()
+endfunction()
+
+function(llvm_lib_precompiled_headers)
+ get_all_targets("${LLVM_MAIN_SRC_DIR}/lib" lib_targets)
+ foreach(target ${lib_targets})
+ add_llvm_lib_precompiled_headers(${target})
+ endforeach()
+endfunction()
diff --git a/llvm/include/llvm/Demangle/ItaniumDemangle.h b/llvm/include/llvm/Demangle/ItaniumDemangle.h
index d33af157543fe..0f61fc1d61133 100644
--- a/llvm/include/llvm/Demangle/ItaniumDemangle.h
+++ b/llvm/include/llvm/Demangle/ItaniumDemangle.h
@@ -586,11 +586,11 @@ class EnableIfAttr : public Node {
class ObjCProtoName : public Node {
const Node *Ty;
- std::string_view Protocol;
-
friend class PointerType;
public:
+ std::string_view Protocol;
+
ObjCProtoName(const Node *Ty_, std::string_view Protocol_)
: Node(KObjCProtoName), Ty(Ty_), Protocol(Protocol_) {}
diff --git a/llvm/include/llvm/PrecompiledHeaders.h b/llvm/include/llvm/PrecompiledHeaders.h
new file mode 100644
index 0000000000000..58edf5576a57e
--- /dev/null
+++ b/llvm/include/llvm/PrecompiledHeaders.h
@@ -0,0 +1,11 @@
+#pragma once
+
+#include "ADT/ArrayRef.h"
+
+#include "CodeGen/SelectionDAG.h"
+#include "CodeGen/TargetInstrInfo.h"
+
+#include "IR/IntrinsicInst.h"
+#include "IR/PassManager.h"
+
+#include "MC/MCContext.h"
diff --git a/llvm/lib/BinaryFormat/Dwarf.cpp b/llvm/lib/BinaryFormat/Dwarf.cpp
index 7324266172684..9a170db4f3755 100644
--- a/llvm/lib/BinaryFormat/Dwarf.cpp
+++ b/llvm/lib/BinaryFormat/Dwarf.cpp
@@ -579,15 +579,17 @@ StringRef llvm::dwarf::LocListEncodingString(unsigned Encoding) {
}
StringRef llvm::dwarf::CallFrameString(unsigned Encoding,
- Triple::ArchType Arch) {
+ Triple::ArchType Arch) {
assert(Arch != llvm::Triple::ArchType::UnknownArch);
-#define SELECT_AARCH64 (Arch == llvm::Triple::aarch64_be || Arch == llvm::Triple::aarch64)
+#define SELECT_AARCH64 \
+ (Arch == llvm::Triple::aarch64_be || Arch == llvm::Triple::aarch64)
#define SELECT_MIPS64 Arch == llvm::Triple::mips64
-#define SELECT_SPARC (Arch == llvm::Triple::sparc || Arch == llvm::Triple::sparcv9)
+#define SELECT_SPARC \
+ (Arch == llvm::Triple::sparc || Arch == llvm::Triple::sparcv9)
#define SELECT_X86 (Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64)
#define HANDLE_DW_CFA(ID, NAME)
-#define HANDLE_DW_CFA_PRED(ID, NAME, PRED) \
- if (ID == Encoding && PRED) \
+#define HANDLE_DW_CFA_PRED(ID, NAME, PRED) \
+ if (ID == Encoding && PRED) \
return "DW_CFA_" #NAME;
#include "llvm/BinaryFormat/Dwarf.def"
@@ -858,9 +860,9 @@ StringRef llvm::dwarf::RLEString(unsigned RLE) {
}
}
-constexpr char llvm::dwarf::EnumTraits<Attribute>::Type[];
-constexpr char llvm::dwarf::EnumTraits<Form>::Type[];
-constexpr char llvm::dwarf::EnumTraits<Index>::Type[];
-constexpr char llvm::dwarf::EnumTraits<Tag>::Type[];
-constexpr char llvm::dwarf::EnumTraits<LineNumberOps>::Type[];
-constexpr char llvm::dwarf::EnumTraits<LocationAtom>::Type[];
+constexpr char llvm::dwarf::EnumTraits<llvm::dwarf::Attribute>::Type[];
+constexpr char llvm::dwarf::EnumTraits<llvm::dwarf::Form>::Type[];
+constexpr char llvm::dwarf::EnumTraits<llvm::dwarf::Index>::Type[];
+constexpr char llvm::dwarf::EnumTraits<llvm::dwarf::Tag>::Type[];
+constexpr char llvm::dwarf::EnumTraits<llvm::dwarf::LineNumberOps>::Type[];
+constexpr char llvm::dwarf::EnumTraits<llvm::dwarf::LocationAtom>::Type[];
diff --git a/llvm/lib/BinaryFormat/MsgPackDocumentYAML.cpp b/llvm/lib/BinaryFormat/MsgPackDocumentYAML.cpp
index 3de3dccce0c6c..3917cd066360c 100644
--- a/llvm/lib/BinaryFormat/MsgPackDocumentYAML.cpp
+++ b/llvm/lib/BinaryFormat/MsgPackDocumentYAML.cpp
@@ -184,18 +184,18 @@ template <> struct TaggedScalarTraits<ScalarDocNode> {
static QuotingType mustQuote(const ScalarDocNode &S, StringRef ScalarStr) {
switch (S.getKind()) {
- case Type::Int:
+ case msgpack::Type::Int:
return ScalarTraits<int64_t>::mustQuote(ScalarStr);
- case Type::UInt:
+ case msgpack::Type::UInt:
return ScalarTraits<uint64_t>::mustQuote(ScalarStr);
- case Type::Nil:
+ case msgpack::Type::Nil:
return ScalarTraits<StringRef>::mustQuote(ScalarStr);
- case Type::Boolean:
+ case msgpack::Type::Boolean:
return ScalarTraits<bool>::mustQuote(ScalarStr);
- case Type::Float:
+ case msgpack::Type::Float:
return ScalarTraits<double>::mustQuote(ScalarStr);
- case Type::Binary:
- case Type::String:
+ case msgpack::Type::Binary:
+ case msgpack::Type::String:
return ScalarTraits<std::string>::mustQuote(ScalarStr);
default:
llvm_unreachable("unrecognized ScalarKind");
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp
index ecdbd004efadb..a2b65519e1770 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp
@@ -30,9 +30,7 @@ void DWARFAbbreviationDeclaration::clear() {
FixedAttributeSize.reset();
}
-DWARFAbbreviationDeclaration::DWARFAbbreviationDeclaration() {
- clear();
-}
+DWARFAbbreviationDeclaration::DWARFAbbreviationDeclaration() { clear(); }
llvm::Expected<DWARFAbbreviationDeclaration::ExtractState>
DWARFAbbreviationDeclaration::extract(DataExtractor Data, uint64_t *OffsetPtr) {
@@ -68,7 +66,7 @@ DWARFAbbreviationDeclaration::extract(DataExtractor Data, uint64_t *OffsetPtr) {
// Read all of the abbreviation attributes and forms.
while (Data.isValidOffset(*OffsetPtr)) {
- auto A = static_cast<Attribute>(Data.getULEB128(OffsetPtr, &Err));
+ auto A = static_cast<dwarf::Attribute>(Data.getULEB128(OffsetPtr, &Err));
if (Err)
return std::move(Err);
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 6febff89ac519..11c52182d20fd 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -92,7 +92,7 @@ Expected<ArrayRef<T>> MinidumpFile::getListStream(StreamType Type) const {
return getDataSliceAs<T>(*Stream, ListOffset, ListSize);
}
-template Expected<ArrayRef<Module>>
+template Expected<ArrayRef<minidump::Module>>
MinidumpFile::getListStream(StreamType) const;
template Expected<ArrayRef<Thread>>
MinidumpFile::getListStream(StreamType) const;
diff --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index fdbd2d8e6dcbc..a6ff0f57e0d93 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -44,9 +44,15 @@ static inline void mapOptionalAs(yaml::IO &IO, const char *Key, EndianType &Val,
namespace {
/// Return the appropriate yaml Hex type for a given endian-aware type.
template <typename EndianType> struct HexType;
-template <> struct HexType<support::ulittle16_t> { using type = yaml::Hex16; };
-template <> struct HexType<support::ulittle32_t> { using type = yaml::Hex32; };
-template <> struct HexType<support::ulittle64_t> { using type = yaml::Hex64; };
+template <> struct HexType<support::ulittle16_t> {
+ using type = yaml::Hex16;
+};
+template <> struct HexType<support::ulittle32_t> {
+ using type = yaml::Hex32;
+};
+template <> struct HexType<support::ulittle64_t> {
+ using type = yaml::Hex64;
+};
} // namespace
/// Yaml-map an endian-aware type as an appropriately-sized hex value.
@@ -499,7 +505,7 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
if (!ExpectedList)
return ExpectedList.takeError();
std::vector<ModuleListStream::entry_type> Modules;
- for (const Module &M : *ExpectedList) {
+ for (const minidump::Module &M : *ExpectedList) {
auto ExpectedName = File.getString(M.ModuleNameRVA);
if (!ExpectedName)
return ExpectedName.takeError();
@@ -516,7 +522,7 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
}
case StreamKind::RawContent:
return std::make_unique<RawContentStream>(StreamDesc.Type,
- File.getRawStream(StreamDesc));
+ File.getRawStream(StreamDesc));
case StreamKind::SystemInfo: {
auto ExpectedInfo = File.getSystemInfo();
if (!ExpectedInfo)
@@ -525,7 +531,7 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
if (!ExpectedCSDVersion)
return ExpectedInfo.takeError();
return std::make_unique<SystemInfoStream>(*ExpectedInfo,
- std::move(*ExpectedCSDVersion));
+ std::move(*ExpectedCSDVersion));
}
case StreamKind::TextContent:
return std::make_unique<TextContentStream>(
diff --git a/llvm/lib/Remarks/Remark.cpp b/llvm/lib/Remarks/Remark.cpp
index ef42271a3c8da..3418160f91f40 100644
--- a/llvm/lib/Remarks/Remark.cpp
+++ b/llvm/lib/Remarks/Remark.cpp
@@ -27,14 +27,16 @@ std::string Remark::getArgsAsMsg() const {
}
/// Returns the value of a specified key parsed from StringRef.
-std::optional<int> Argument::getValAsInt() const {
+std::optional<int> llvm::remarks::Argument::getValAsInt() const {
APInt KeyVal;
if (Val.getAsInteger(10, KeyVal))
return std::nullopt;
return KeyVal.getSExtValue();
}
-bool Argument::isValInt() const { return getValAsInt().has_value(); }
+bool llvm::remarks::Argument::isValInt() const {
+ return getValAsInt().has_value();
+}
void RemarkLocation::print(raw_ostream &OS) const {
OS << "{ "
@@ -42,7 +44,7 @@ void RemarkLocation::print(raw_ostream &OS) const {
<< " Column:" << SourceColumn << " }\n";
}
-void Argument::print(raw_ostream &OS) const {
+void llvm::remarks::Argument::print(raw_ostream &OS) const {
OS << Key << ": " << Val << "\n";
}
@@ -146,12 +148,12 @@ extern "C" uint32_t LLVMRemarkEntryGetNumArgs(LLVMRemarkEntryRef Remark) {
extern "C" LLVMRemarkArgRef
LLVMRemarkEntryGetFirstArg(LLVMRemarkEntryRef Remark) {
- ArrayRef<Argument> Args = unwrap(Remark)->Args;
+ ArrayRef<remarks::Argument> Args = unwrap(Remark)->Args;
// No arguments to iterate on.
if (Args.empty())
return nullptr;
return reinterpret_cast<LLVMRemarkArgRef>(
- const_cast<Argument *>(Args.begin()));
+ const_cast<remarks::Argument *>(Args.begin()));
}
extern "C" LLVMRemarkArgRef
@@ -160,10 +162,11 @@ LLVMRemarkEntryGetNextArg(LLVMRemarkArgRef ArgIt, LLVMRemarkEntryRef Remark) {
if (ArgIt == nullptr)
return nullptr;
- auto It = (ArrayRef<Argument>::const_iterator)ArgIt;
+ auto It = (ArrayRef<remarks::Argument>::const_iterator)ArgIt;
auto Next = std::next(It);
if (Next == unwrap(Remark)->Args.end())
return nullptr;
- return reinterpret_cast<LLVMRemarkArgRef>(const_cast<Argument *>(Next));
+ return reinterpret_cast<LLVMRemarkArgRef>(
+ const_cast<remarks::Argument *>(Next));
}
diff --git a/llvm/lib/Remarks/YAMLRemarkParser.cpp b/llvm/lib/Remarks/YAMLRemarkParser.cpp
index a287ef5742556..ff556791573b7 100644
--- a/llvm/lib/Remarks/YAMLRemarkParser.cpp
+++ b/llvm/lib/Remarks/YAMLRemarkParser.cpp
@@ -157,9 +157,8 @@ Expected<std::unique_ptr<YAMLRemarkParser>> remarks::createYAMLParserFromMeta(
}
std::unique_ptr<YAMLRemarkParser> Result =
- StrTab
- ? std::make_unique<YAMLStrTabRemarkParser>(Buf, std::move(*StrTab))
- : std::make_unique<YAMLRemarkParser>(Buf);
+ StrTab ? std::make_unique<YAMLStrTabRemarkParser>(Buf, std::move(*StrTab))
+ : std::make_unique<YAMLRemarkParser>(Buf);
if (SeparateBuf)
Result->SeparateBuf = std::move(SeparateBuf);
return std::move(Result);
@@ -260,15 +259,16 @@ YAMLRemarkParser::parseRemark(yaml::Document &RemarkEntry) {
}
// Check if any of the mandatory fields are missing.
- if (TheRemark.RemarkType == Type::Unknown || TheRemark.PassName.empty() ||
- TheRemark.RemarkName.empty() || TheRemark.FunctionName.empty())
+ if (TheRemark.RemarkType == remarks::Type::Unknown ||
+ TheRemark.PassName.empty() || TheRemark.RemarkName.empty() ||
+ TheRemark.FunctionName.empty())
return error("Type, Pass, Name or Function missing.",
*RemarkEntry.getRoot());
return std::move(Result);
}
-Expected<Type> YAMLRemarkParser::parseType(yaml::MappingNode &Node) {
+Expected<remarks::Type> YAMLRemarkParser::parseType(yaml::MappingNode &Node) {
auto Type = StringSwitch<remarks::Type>(Node.getRawTag())
.Case("!Passed", remarks::Type::Passed)
.Case("!Missed", remarks::Type::Missed)
@@ -362,7 +362,7 @@ YAMLRemarkParser::parseDebugLoc(yaml::KeyValueNode &Node) {
return RemarkLocation{*File, *Line, *Column};
}
-Expected<Argument> YAMLRemarkParser::parseArg(yaml::Node &Node) {
+Expected<remarks::Argument> YAMLRemarkParser::parseArg(yaml::Node &Node) {
auto *ArgMap = dyn_cast<yaml::MappingNode>(&Node);
if (!ArgMap)
return error("expected a value of mapping type.", Node);
diff --git a/llvm/lib/Remarks/YAMLRemarkSerializer.cpp b/llvm/lib/Remarks/YAMLRemarkSerializer.cpp
index 68285c3dde1bf..b87151040d50d 100644
--- a/llvm/lib/Remarks/YAMLRemarkSerializer.cpp
+++ b/llvm/lib/Remarks/YAMLRemarkSerializer.cpp
@@ -25,7 +25,7 @@ template <typename T>
static void mapRemarkHeader(yaml::IO &io, T PassName, T RemarkName,
std::optional<RemarkLocation> RL, T FunctionName,
std::optional<uint64_t> Hotness,
- ArrayRef<Argument> Args) {
+ ArrayRef<remarks::Argument> Args) {
io.mapRequired("Pass", PassName);
io.mapRequired("Name", RemarkName);
io.mapOptional("DebugLoc", RL);
@@ -41,19 +41,23 @@ template <> struct MappingTraits<remarks::Remark *> {
static void mapping(IO &io, remarks::Remark *&Remark) {
assert(io.outputting() && "input not yet implemented");
- if (io.mapTag("!Passed", (Remark->RemarkType == Type::Passed)))
+ if (io.mapTag("!Passed", (Remark->RemarkType == remarks::Type::Passed)))
;
- else if (io.mapTag("!Missed", (Remark->RemarkType == Type::Missed)))
+ else if (io.mapTag("!Missed",
+ (Remark->RemarkType == remarks::Type::Missed)))
;
- else if (io.mapTag("!Analysis", (Remark->RemarkType == Type::Analysis)))
+ else if (io.mapTag("!Analysis",
+ (Remark->RemarkType == remarks::Type::Analysis)))
;
- else if (io.mapTag("!AnalysisFPCommute",
- (Remark->RemarkType == Type::AnalysisFPCommute)))
+ else if (io.mapTag(
+ "!AnalysisFPCommute",
+ (Remark->RemarkType == remarks::Type::AnalysisFPCommute)))
;
else if (io.mapTag("!AnalysisAliasing",
- (Remark->RemarkType == Type::AnalysisAliasing)))
+ (Remark->RemarkType == remarks::Type::AnalysisAliasing)))
;
- else if (io.mapTag("!Failure", (Remark->RemarkType == Type::Failure)))
+ else if (io.mapTag("!Failure",
+ (Remark->RemarkType == remarks::Type::Failure)))
;
else
llvm_unreachable("Unknown remark type");
@@ -124,7 +128,7 @@ template <> struct BlockScalarTraits<StringBlockVal> {
/// Keep this in this file so that it doesn't get misused from YAMLTraits.h.
template <typename T> struct SequenceTraits<ArrayRef<T>> {
static size_t size(IO &io, ArrayRef<T> &seq) { return seq.size(); }
- static Argument &element(IO &io, ArrayRef<T> &seq, size_t index) {
+ static remarks::Argument &element(IO &io, ArrayRef<T> &seq, size_t index) {
assert(io.outputting() && "input not yet implemented");
// The assert above should make this "safer" to satisfy the YAMLTraits.
return const_cast<T &>(seq[index]);
@@ -132,8 +136,8 @@ template <typename T> struct SequenceTraits<ArrayRef<T>> {
};
/// Implement this as a mapping for now to get proper quotation for the value.
-template <> struct MappingTraits<Argument> {
- static void mapping(IO &io, Argument &A) {
+template <> struct MappingTraits<remarks::Argument> {
+ static void mapping(IO &io, remarks::Argument &A) {
assert(io.outputting() && "input not yet implemented");
if (auto *Serializer = dyn_cast<YAMLStrTabRemarkSerializer>(
@@ -155,7 +159,7 @@ template <> struct MappingTraits<Argument> {
} // end namespace yaml
} // end namespace llvm
-LLVM_YAML_IS_SEQUENCE_VECTOR(Argument)
+LLVM_YAML_IS_SEQUENCE_VECTOR(remarks::Argument)
YAMLRemarkSerializer::YAMLRemarkSerializer(raw_ostream &OS, SerializerMode Mode,
std::optional<StringTable> StrTabIn)
diff --git a/llvm/lib/TargetParser/X86TargetParser.cpp b/llvm/lib/TargetParser/X86TargetParser.cpp
index 21f46f576490a..9bac00f8ba45e 100644
--- a/llvm/lib/TargetParser/X86TargetParser.cpp
+++ b/llvm/lib/TargetParser/X86TargetParser.cpp
@@ -20,20 +20,20 @@ using namespace llvm::X86;
namespace {
-using FeatureBitset = Bitset<X86::CPU_FEATURE_MAX>;
+using X86FeatureBitset = Bitset<X86::CPU_FEATURE_MAX>;
struct ProcInfo {
StringLiteral Name;
X86::CPUKind Kind;
unsigned KeyFeature;
- FeatureBitset Features;
+ X86FeatureBitset Features;
char Mangling;
bool OnlyForCPUDispatchSpecific;
};
struct FeatureInfo {
StringLiteral NameWithPlus;
- FeatureBitset ImpliedFeatures;
+ X86FeatureBitset ImpliedFeatures;
StringRef getName(bool WithPlus = false) const {
assert(NameWithPlus[0] == '+' && "Expected string to start with '+'");
@@ -46,176 +46,178 @@ struct FeatureInfo {
} // end anonymous namespace
#define X86_FEATURE(ENUM, STRING) ...
[truncated]
|
|
You probably should post an RFC on discourse for wider visibility, as this affects essentially everyone. It looks like PCH is strictly an MSVC thing? but the change in llvm/CMakeLists.txt is executed for all builds, which slows down CMake processing for everyone. I see a number of formatting changes in this PR. Probably you ran clang-format on the modified files? We usually don't want to mix formatting changes with functional changes. You should use clang-format-diff instead. The new header does not follow the coding standards, see how we like to do file headers and header guards. |
Thanks for this suggestion! Opened an RFC here
It's most beneficial for MSVC, but PCH is well supported by all major compilers. |
|
My knee-jerk reaction would be: Why don't you just use modules, since we are maintaining a complete module map for the project? Is this an MSVC-specific option? |
|
Do you have any data on how faster the build gets with the precompiled headers? I've done some not particularly thorough experiments with clang and curious what you were able to gain. And it is useful to measure the difference in CMake processing. @pogo59 will timing a command |
I don't think modules are supported when compiling with clang-cl on windows, if I remember correctly. In that case this would also speed up that config and not just MSVC. |
This specific change has very minimal improvements in build time, since it's serving as a RFC. Build time improvements can be easily tested by editing the PrecompiledHeaders file, which will recompile all files that include the PCH. Empty PCH build time was around 14m on my system, with the current PCH being closer to 13m. With a very hacked up PCH implementation I was able to get close to 1.8x build time improvements.
Looks like CMake 3.28 introduces support for cxxmodules. |
That sounds good even if we would end up with more modest improvements. At least to me it is an indication that it is worth pursuing. Is it an option to make |
Reworked the changes to be far less intrusive. Currently, I see about ~1.5x build time improvements.
This will definitely be set to OFF by default before merging, I just want to see if build errors are uncovered by the CI build systems |
I think the suggestion was to try clang modules instead of c++ modules. Although #91755 (comment). |
I see now. It makes sense and is actually pretty smart to test in available CI. To assess the impact on others do you have any requirements for the community regarding the support of this build mode? For example, if after a commit the PCH build fails, is it OK if there is no prompt revert but you need to figure out what is broken and propose a fix? In the very beginning you've mentioned
so I assume that you don't expect any maintenance contributions from the rest of the community. |
|
As for the modules, clang modules work but they require clang to be the compiler. So it won't work with MSVC compiler. Not sure C++20 modules would work (don't know about any bots testing this configuration) but we don't support a sufficiently new CMake. In addition, clang's support of C++20 modules isn't complete, so I don't think that C++20 modules is a viable alternative to the precompiled headers at this time. I also have to admit that I have a selfish interest in comparing modules' performance with PCH and glad that I don't need to provide PCH part myself. |
I am not sure how feasible it is, but I think it would make sense to have the Windows CI bot run with PCH enabled.
Unfortunately I don't see PCH working in the CI runs (I would expect to see a |
|
CI seems to handle building PCH with no issue. How do the maintainers feel about enabling PCH by default for MSVC and/or Windows? |
|
Sorry for not supporting your aspirations. I suggest to try getting an agreement on a smaller proposal and leave the rest for the future. I understand it's not what you'd like to have but bigger changes are riskier and people try to avoid risks, so it is harder to get an agreement on bigger changes. I suggest to add precompiled headers as an opt-in option. So nothing changes for existing LLVM developers. But if anybody wants to try it, they can enable it. At this stage the community isn't responsible for maintaining the PCH configuration. If anything breaks, that's not grounds for a back-to-green revert. Someone (likely you) would work out with a breaking patch author what adjustments are required for PCH configuration to work again. Or maybe make the required tweaks yourself. As the future steps I consider
If this proposal makes sense, we'll try to get more visibility on discourse and get buy-in from the rest of the community. |
|
@vsapsai The proposal is fair. I've disabled PCH by default, making it opt-in. The CI failure seems unrelated to my changes. |
|
Ok, looks like there are no objections to the usage of precompiled headers in general. So now I'll look at the technical side of the change. Please note that I'm not good with CMake and might ask some stupid questions. I've tried your patch locally and hitting the error I haven't investigated the issue yet but maybe it is something you've seen, so I've decided to check first if it is a known problem. |
I believe the issue may have been case sensitivity with some paths, should be resolved with the latest commit. |
Thanks, that fixes the issue. Do you want to make the precompiled headers for CI build or for incremental local development? Asking because it affects how much to add to the precompiled header. For example, having in a pch multiple often-modified headers can slow down the incremental builds but it doesn't have the same downsides for clean builds. Have you thought about dependencies between PCH and generated headers? For example, should we use Attributes.inc in the precompiled header? If yes, does it mean that PCH shouldn't be used for llvmIR? Personally I'm not excited about managing layering between PCH and various libraries but I'm curious to know your opinion. |
Thanks for testing and catching the bug!! :)
The focus of this PR is on widely included headers, and I'm mainly interested in speeding up clean builds.
This came to bite me today after rebasing. This is a problem I need to give more thought to, but for now, the fix I came up with is to simply make sure tablegenned headers in the PCH can generate before the PCH gets included by other targets. |
|
Sorry for the looong delay in continuing the conversation, other stuff came up. How do you test the benefit from having certain headers in the precompiled header? I was interested in comparing the build time when you don't have generated headers in PCH and use PCH for more targets I have also collected how often different headers are included by various .cpp files to decide which files are worth adding to PCH. |
|
I'm considering closing this PR as I no longer have as much time to dedicate to maintaining this feature, and the generated header include dependency is likely to be a persistent problem.
The file that dictates which headers are in the PCH is Comparing generated/non-generated in the PCH is tricky because of the include dependencies. |
Opening this PR to start a discussion on supporting PCH for LLVM Libraries.
This can significantly improve build time performance, especially on MSVC.
The changes are currently a "proof of concept" that are meant to be a starting point for discussion.
The aim is to be as non-intrusive as possible with the addition.
The candidates for headers to be included in the PCH are chosen based on the analysis of Visual Studio's
Build Insights.This produces the time taken to parse files during compilation, and the inclusion dependencies for header files.
Ideally, each target would specify which headers to include for its PCH, since some headers are widely used within one target, but not all libraries.
Some notes: