[Mlir-commits] [llvm] [mlir] Shard the public llvm-config.h in multiple files (NFC) (PR #71273)

[Mehdi Amini]

joker-eph wrote:

> If we do this, what's the advantage of putting all these headers in Config/? LLVM_ENABLE_DIA_SDK for example is used exclusively in llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp – should we put the header for that over there then?

In Config/ there is `llvm-config.h` and `config.h`: the former is for things that are used in our public headers or must be made available publicly (through our `install/llvm/include/`). The latter is for private things, and I actually had written initially in this PR description that some of these should move to be private instead: it may be the case for LLVM_ENABLE_DIA_SDK for example. 
However I removed it from the description because I am not totally sure: something that is used only in a tool can be defined in CMake and never in a header. Right now I can't know if a setting is here "accidentally" (someone thought they were "doing the right thing" by adding a setting in the header) or meant to be exposed to third-party in the distribution (that is: even without any in-tree user for a setting, it may be a useful setting for external user to guard code depending on how LLVM was configured, I'm actually building my downstream project that way against a prebuilt LLVM and we support different configs). 
So that requires a bit more archeology and consideration on a case by case. (I can also just make it "private" and wait for someone to complain...).

> I think this is a noble patch, but doing this piecemeal for the ones people actually toggle is imho good enough.

I started to do that and I realized that there were only a couple of options like that (HAVE_SYSEXIT and HAS_ATOMICS I think), I can group them into "llvm-config-platform.h.cmake" if you prefer. Right now `llvm/include/llvm/Support/ExitCodes.h` is the only user of HAVE_SYSEXITS_H, I was seeing it as some kind of "private config" that is here only because it's used in a public header.
There is also `llvm-config-version.h.cmake`  but seemed to me preferable to not have version included everywhere has_atomics is ("platforms" flag are also conceptually different than LLVM-version).

> (I would love to be able to turn assertions on and off without a full rebuild, but even if that's in its own .h, assertions are used anywhere, so it's still a full rebuild even then :P)

(right it is affecting virtually everything, even then we don't define NDEBUG ourselves anyway, and we don't have a macro for our own assertions I believe)


https://github.com/llvm/llvm-project/pull/71273