[NFC] Add explicit #include llvm-config.h where its macros are used, last part. #107615
Conversation
…last part. (this is the part related to bolt, lld and mlir) Without these explicit includes, removing other headers, who implicitly include llvm-config.h, may have non-trivial side effects. For example, `clangd` may report even `llvm-config.h` as "no used" in case it defines a macro, that is explicitly used with #ifdef. It is actually amplified with different build configs which use different set of macros.
dfukalov
requested review from
aaupov,
maksfb,
rafaelauler,
ayermolo,
dcci,
antiagainst and
kuhar
as code owners
llvmbot
added
lld
mlir:core
|
@llvm/pr-subscribers-lld-elf @llvm/pr-subscribers-mlir-llvm Author: Daniil Fukalov (dfukalov) Changes(this is the part related to bolt, lld and mlir) Without these explicit includes, removing other headers, who implicitly include llvm-config.h, may have non-trivial side effects. For example, Full diff: https://github.com/llvm/llvm-project/pull/107615.diff 6 Files Affected:
diff --git a/bolt/include/bolt/Core/BinaryBasicBlock.h b/bolt/include/bolt/Core/BinaryBasicBlock.h
index 9a9d7b8735d714..b4f31cf2bae6f6 100644
--- a/bolt/include/bolt/Core/BinaryBasicBlock.h
+++ b/bolt/include/bolt/Core/BinaryBasicBlock.h
@@ -19,6 +19,7 @@
#include "bolt/Core/MCPlus.h"
#include "llvm/ADT/GraphTraits.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Config/llvm-config.h" // for LLVM_ON_UNIX
#include "llvm/MC/MCInst.h"
#include "llvm/MC/MCSymbol.h"
#include "llvm/Support/ErrorOr.h"
diff --git a/lld/ELF/OutputSections.cpp b/lld/ELF/OutputSections.cpp
index cb17e107d6dae2..8801a9991939e2 100644
--- a/lld/ELF/OutputSections.cpp
+++ b/lld/ELF/OutputSections.cpp
@@ -16,7 +16,7 @@
#include "lld/Common/Arrays.h"
#include "lld/Common/Memory.h"
#include "llvm/BinaryFormat/Dwarf.h"
-#include "llvm/Config/llvm-config.h" // LLVM_ENABLE_ZLIB
+#include "llvm/Config/llvm-config.h" // LLVM_ENABLE_ZLIB, LLVM_ENABLE_ZSTD
#include "llvm/Support/Compression.h"
#include "llvm/Support/LEB128.h"
#include "llvm/Support/Parallel.h"
diff --git a/mlir/include/mlir/Bytecode/BytecodeWriter.h b/mlir/include/mlir/Bytecode/BytecodeWriter.h
index ea4b36832e0bac..0287e004bb9936 100644
--- a/mlir/include/mlir/Bytecode/BytecodeWriter.h
+++ b/mlir/include/mlir/Bytecode/BytecodeWriter.h
@@ -14,6 +14,7 @@
#define MLIR_BYTECODE_BYTECODEWRITER_H
#include "mlir/IR/AsmState.h"
+#include "llvm/Config/llvm-config.h" // for LLVM_VERSION_STRING
namespace mlir {
class DialectBytecodeWriter;
diff --git a/mlir/lib/Target/SPIRV/SPIRVBinaryUtils.cpp b/mlir/lib/Target/SPIRV/SPIRVBinaryUtils.cpp
index 18a108be764b37..31205d8f408f18 100644
--- a/mlir/lib/Target/SPIRV/SPIRVBinaryUtils.cpp
+++ b/mlir/lib/Target/SPIRV/SPIRVBinaryUtils.cpp
@@ -12,6 +12,7 @@
#include "mlir/Target/SPIRV/SPIRVBinaryUtils.h"
#include "mlir/Dialect/SPIRV/IR/SPIRVTypes.h"
+#include "llvm/Config/llvm-config.h" // for LLVM_VERSION_MAJOR
using namespace mlir;
diff --git a/mlir/unittests/Target/LLVM/SerializeNVVMTarget.cpp b/mlir/unittests/Target/LLVM/SerializeNVVMTarget.cpp
index a8fe20d52fb2a8..642aa045178095 100644
--- a/mlir/unittests/Target/LLVM/SerializeNVVMTarget.cpp
+++ b/mlir/unittests/Target/LLVM/SerializeNVVMTarget.cpp
@@ -18,6 +18,7 @@
#include "mlir/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.h"
#include "mlir/Target/LLVMIR/Dialect/NVVM/NVVMToLLVMIRTranslation.h"
+#include "llvm/Config/llvm-config.h" // for LLVM_HAS_NVPTX_TARGET
#include "llvm/IRReader/IRReader.h"
#include "llvm/Support/MemoryBufferRef.h"
#include "llvm/Support/Process.h"
diff --git a/polly/lib/Support/RegisterPasses.cpp b/polly/lib/Support/RegisterPasses.cpp
index 34570f0c83c5d6..503c3ae1e07c73 100644
--- a/polly/lib/Support/RegisterPasses.cpp
+++ b/polly/lib/Support/RegisterPasses.cpp
@@ -40,6 +40,7 @@
#include "polly/Support/DumpFunctionPass.h"
#include "polly/Support/DumpModulePass.h"
#include "llvm/Analysis/CFGPrinter.h"
+#include "llvm/Config/llvm-config.h" // for LLVM_VERSION_STRING
#include "llvm/IR/LegacyPassManager.h"
#include "llvm/IR/PassManager.h"
#include "llvm/IR/Verifier.h"
|
I'm not opposed to this change, but how does it differ from any other headers? I didn't quite get what would be special about llvm-config.h here?
You start this sentence with "For example", but I can't follow how what you're describing relates to the first sentence? |
This and some similar headers are actually generated by cmake "configuration" headers containing only defines are used in different places to switch some parts of sources depending on target, host, libraries used, etc. One of approaches to use such defines is '#ifdef'. In this case, if a config header was not actually included (explicit or implicit) the build will be successful, but can be incorrect. Moreover, tests can pass (that was observed in PR #104825).
|
The right fix for this IMO is to stop using ifdef and use if instead, with
I don't quite the explanation, or rather I still don't see the connection: how is this patch adding these headers will impact what clangd says? Seems like you're describing a bug in clang? |
I guess this is a good idea, to use
No. Just because |
Right, it'll use whatever you have in your local build environment I believe (even simple things as NDEBUG depends if you have a release or assert build), I don't think it can do differently?
I don't quite understand how adding this include changes anything? (I assume they are already transitively included right?) |
The change is needed to go further with includes cleanup, like in PR #104825. Without these explicit includes it is too easy to remove implicitly included config headers even for a local config that is being built. And then it is hard to find where an include was transitively included. For a "non-local" config it is much more complicated. |
(this is the part related to bolt, lld and mlir)
Without these explicit includes, removing other headers, who implicitly include llvm-config.h, may have non-trivial side effects. For example,
clangdmay report evenllvm-config.has "no used" in case it defines a macro, that is explicitly used with #ifdef. It is actually amplified with different build configs which use different set of macros.