[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.


================
Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries x86_64-unknown-linux-gnu-clang++.cfg first.
 //
----------------
sepavloff wrote:
> mgorny wrote:
> > sepavloff wrote:
> > > mgorny wrote:
> > > > sepavloff wrote:
> > > > > mgorny wrote:
> > > > > > sepavloff wrote:
> > > > > > > mgorny wrote:
> > > > > > > > arichardson wrote:
> > > > > > > > > sepavloff wrote:
> > > > > > > > > > Tests must check the case when target prefix is not a real triple as in the original test (qqq-clang).
> > > > > > > > > It would be quite important for me that this continues to work. I made use of that in the CheriBSD toolchain when creating [[ https://github.com/CTSRD-CHERI/cheribuild/blob/master/pycheribuild/projects/cross/llvm.py#L499 | symlinked binaries to easily build for different ABIs]] such as `cheribsd-riscv64-hybrid-clang++` and `cheribsd-riscv64-purecap-clang-cpp`. It appears this previously only worked if the prefix did not start with a valid triple (which is why I put the OS before the architecture). I think it would also be nice if the whole prefix was checked even if it starts with a valid triple, but this does not need to be changed in this patch (haven't looked at it in detail so this might actually work).
> > > > > > > > If the prefix is not a valid triple, then clang ignores it and uses host triple instead. And now we're back to square one. If I check both variants, it's too complex. If I don't, it's bad too.
> > > > > > > Our customers use this feature. Target prefix may designate, for example, debug build or build with specific framework.
> > > > > > How are we supposed to avoid the "absurd" case where x86_64 configs are loaded for `-m32` invocation then?
> > > > > In this case overloading target does not makes sense. You need to analyze `RealTriple` in `Driver::loadDefaultConfigFiles` and if it is wrong, use target prefix as if it is real target.
> > > > What do you mean by "wrong"? The target triple is always a correct triple.
> > > Not triple, sorry. Target prefix taken from `ClangNameParts.TargetPrefix`.
> > Now I'm confused. Are you suggesting that we use prefix instead of target if it's… incorrect?
> Basically yes.
> 
> We need to support arbitrary target prefixes, because they are used now. If `ClangNameParts.TargetPrefix` is not a valid triple, use it to build config file names.
> 
> We also need to use config files like `x86_64.cfg` where middle components are dropped, they are also used. They also need to support target overloading.
I can replace my approach with simple shell scripts that invoke clang --config=..., but IMO it would be nice if the logic was something like the first check being
`if "explicit --target empty" and "ClangNameParts.TargetPrefix non-empty" and "ClangNameParts.TargetPrefix not a valid triple" -> load ClangNameParts.TargetPrefix + ".cfg"` followed by the current logic.
I don't mind either way whether the first check should also load a driver-specific config file or not. I'd also be happy if the logic was "try loading ClangNameParts.TargetPrefix if explicit --target empty" regardless of whether it is a valid triple or not.
Does this approach sound reasonable?


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

https://reviews.llvm.org/D134337