[libcxx-commits] [PATCH] D140925: [CMake] Use Clang to infer the target triple

[Tamás Szelei via Phabricator via libcxx-commits]

tamas added inline comments.


================
Comment at: runtimes/CMakeLists.txt:168
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+  set(CXX_TARGET_TRIPLE ${CMAKE_CXX_COMPILER} --target=${LLVM_RUNTIME_TRIPLE} -print-target-triple)
+  execute_process(COMMAND ${CXX_TARGET_TRIPLE}
----------------
phosek wrote:
> tamas wrote:
> > I think there is one downside to this: the `normalize` function will not, in fact, normalize alternative spellings for equivalent architectures (and likely other components, I haven't checked that):
> > ```
> > + clang -print-target-triple -target amd64-unknown-linux-musl
> > amd64-unknown-linux-musl
> > + clang -print-target-triple -target x86_64-unknown-linux-musl
> > x86_64-unknown-linux-musl
> > ```
> > So the issue where the distribution build can install runtimes in paths that won't be found by the driver still remains (https://github.com/llvm/llvm-project/issues/57781). One way to fix this could be adding a new switch that does a more opinionated normalization (for example, always picks `x86_64` in the above). Sort of a reverse for `Triple::parseArch` etc. where it always returns one certain spelling when there are multiple common alternatives. Alternatively, `normalize` could be changed to do this, but that might subtle breaking consequences for users which are hard to foresee.
> I think this is an orthogonal issue to the one that this change is trying to address. Clang currently uses normalized triples to for its include directories (libc++ headers) and runtime libraries. This change ensures that the triple spelling used by CMake and Clang matches, but it doesn't change the normalization logic.
> 
> From Clang's perspective, `amd64-unknown-linux-musl` and `x86_64-unknown-linux-musl` are two different triples and so it'd use different search directories. We could consider changing the normalization rules to normalize these to the same triple, but that should be done separately (and would likely require further discussion to understand the potential consequences of such a change).
I don't mean to raise this as a potential blocker and this is certainly somewhat orthogonal. However, I think this not true: 

> From Clang's perspective, amd64-unknown-linux-musl and x86_64-unknown-linux-musl are two different triples

`amd64` and `x86_64` are very explictly parsed as the same: https://github.com/llvm/llvm-project/blob/6dc85bd3fde7df2999fda07e9e9f2e83d52c6125/llvm/lib/TargetParser/Triple.cpp#L454

I'm not sure if it's a good idea to change the current normalization logic, hence my suggestion for a new flag. But this is probably a discussion for discourse/discord.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140925/new/

https://reviews.llvm.org/D140925