sepavloff added inline comments.
================ Comment at: clang/lib/Driver/Driver.cpp:1098 + // Try loading separate config for the target (variants 3. and 4.) + CfgFileName = RealTriple.str() + ".cfg"; + if (searchForFile(CfgFilePath, CfgFileSearchDirs, CfgFileName, getVFS()) && ---------------- mgorny wrote: > sepavloff wrote: > > Loading driver-mode config should take place before loading target config, > > it allows to override default settings set by driver mode config in target > > config. Also a test is need for such case. > Do you have any specific option in mind? The main idea was that you use full > triple+driver config when you need overrides because — as we pretty much > established before — the options accepted by different drivers are too > different for them to be meaningfully used with a single config file shared > between targets. I mean we can have several driver-mode configs like `tool.cfg`, where some default settings are set (maybe by using @file constructs`) and several `target.cfg` which may override options defined in `tool.cfg`. Due to different target and driver configs loaded separately, we can avoid creating separate config for each target+driver combination. Not a killer feature, but it is convenient. As I understand, it is enough to swap code that tries loading driver config below with this piece of code, no? ================ 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. // ---------------- 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134337/new/ https://reviews.llvm.org/D134337 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
