[llvm-rc]: Find <target>-clang over just clang

[thieta]

Summary

This patch makes llvm-rc/windres prefer <target>-clang over
clang when doing it's preprocessing. This is so that we can
have a .cfg file for <target> and configure sysroot and other
important flags.

Config files not picked up with clang --target=<target>
automatically.

We only look for <target>-clang in the same dir as llvm-windres
and not for all PATHs to minimize the change.

[mstorsjo]

For these cases, I guess we'd end up executing <triple>-clang -target <triple>, which is a bit redundant, but it's probably not worth complicating the code to avoid that.

Ideally we'd have a test for this, but I think such a test for this can be pretty hard to fit into the test framework, so it can probably go in without.

llvm/tools/llvm-rc/llvm-rc.cpp

127

Please follow the coding convention of the surrounding code here (local variables starting with a uppercase letter)

130

(Triple + "-clang").str() would probably be simpler here

[thieta]

Addressed comments from Martin.

[thieta]

In D119219#3303792, @mstorsjo wrote:

For these cases, I guess we'd end up executing <triple>-clang -target <triple>, which is a bit redundant, but it's probably not worth complicating the code to avoid that.

Yeah I was looking into this - but it seemed a bit complicated to remove the -target in the invocation and made the code more complicated just for a cosmetic issue that is only visible if you pass verbose to llvm-windres and look at the output. It didn't think it was worth it.

Ideally we'd have a test for this, but I think such a test for this can be pretty hard to fit into the test framework, so it can probably go in without.

Same I also looked into this and couldn't figure out a decent way to do this.

[mstorsjo]

LGTM, thanks!

I guess it can be customary to wait until other timezones have been awake for some time too before landing though, in case someone else has something they want to say (even though this is fairly trivial).

[thieta]

In D119219#3303908, @mstorsjo wrote:

LGTM, thanks!

I guess it can be customary to wait until other timezones have been awake for some time too before landing though, in case someone else has something they want to say (even though this is fairly trivial).

Yep no hurry - I'll merge this tomorrow.

[MaskRay]

This is so that we can have a .cfg file for <target> and configure sysroot and other important flags.

I know nearly nothing about llvm-rc... but I wonder whether there is a better way.

For the GNU driver part of Clang, the general direction is to move away from $triple-$program.... 3452a0d8c17f7166f479706b293caf6ac76ffd90

https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-b-prefix "Search $prefix$file for executables, libraries, and data files. If $prefix is a directory, search $prefix/$file"

(I try to document everything I know in https://maskray.me/blog/2021-03-28-compiler-driver-and-cross-compilation#search-paths )

[thieta]

In D119219#3307031, @MaskRay wrote:

This is so that we can have a .cfg file for <target> and configure sysroot and other important flags.

I know nearly nothing about llvm-rc... but I wonder whether there is a better way.

Maybe the config file searching could be expanded to find config files based on target from other things than the target prefix? But that changes more than just around llvm-rc. I suggested that we would look for the sysroot based on the target in this discussion: https://discourse.llvm.org/t/teach-clang-to-find-a-sysroot-based-on-target-in-resource-dir/6100 and the answer to that was the config files, which makes sense since they are a bit more flexible and you might want to set other flags than just --sysroot.

llvm/tools/llvm-rc/llvm-rc.cpp

 std::string createTempFile(const Twine &Prefix, StringRef Suffix) {
   std::error_code EC;
   SmallString<128> FileName;
   if ((EC = sys::fs::createTemporaryFile(Prefix, Suffix, FileName)))
     fatalError("Unable to create temp file: " + EC.message());
   return static_cast<std::string>(FileName);
 }
 
-ErrorOr<std::string> findClang(const char *Argv0) {
+ErrorOr<std::string> findClang(const char *Argv0, StringRef Triple) {
   StringRef Parent = llvm::sys::path::parent_path(Argv0);
   ErrorOr<std::string> Path = std::error_code();
+  std::string TargetClang = (Triple + "-clang").str();
   if (!Parent.empty()) {
     // First look for the tool with all potential names in the specific
     // directory of Argv0, if known
-    for (const auto *Name : {"clang", "clang-cl"}) {
+    for (const auto *Name : {TargetClang.c_str(), "clang", "clang-cl"}) {
       Path = sys::findProgramByName(Name, Parent);
       if (Path)
         return Path;
     }
   }
   // If no parent directory known, or not found there, look everywhere in PATH
   for (const auto *Name : {"clang", "clang-cl"}) {
     Path = sys::findProgramByName(Name);

 };
 
 bool preprocess(StringRef Src, StringRef Dst, const RcOptions &Opts,
                 const char *Argv0) {
   std::string Clang;
   if (Opts.PrintCmdAndExit) {
     Clang = "clang";
   } else {
-    ErrorOr<std::string> ClangOrErr = findClang(Argv0);
+    ErrorOr<std::string> ClangOrErr = findClang(Argv0, Opts.Triple);
     if (ClangOrErr) {
       Clang = *ClangOrErr;
     } else {
       errs() << "llvm-rc: Unable to find clang, skipping preprocessing."
              << "\n";
       errs() << "Pass -no-cpp to disable preprocessing. This will be an error "
                 "in the future."
              << "\n";