[clang] [clang] [MinGW] Don't look for a GCC in path if the install base has a proper mingw sysroot (PR #76949)

Martin Storsjö via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 4 13:12:32 PST 2024


mstorsjo wrote:

> > Looks mostly good to me, but I wonder if we should change testTriple as well.
> 
> I thought so too based on the comment, but reviewing the code it seems `testTriple` is trying to find evidence that a given triple (and more specifically arch for things like `i386` vs `i686`) is valid. The evidence found by `looksLikeMinGWSysroot` does not provide any hint about what the triple or arch name should be, so I don't think it helps `testTriple`.

Thanks, this explanation is absolutely spot on. (In all honesty, I had forgot about `testTriple` when I wrote this patch - and despite the comment right next to the code I touched, I didn't remember to check it...)

Although, on a second thought, it might actually still be good to adjust it in sync. If we're invoking Clang with `-m32` and deciding on whether to use i386/i586/i686, and we end up using the install base as sysroot, without inferring any triple from there, we shouldn't go on and check another unrelated GCC in path in order to influence this. Therefore, I think we perhaps should amend this with the following:
```diff

diff --git a/clang/lib/Driver/ToolChains/MinGW.cpp b/clang/lib/Driver/ToolChains/MinGW.cpp
index e2965a08a0b9..18fc9d4b6807 100644
--- a/clang/lib/Driver/ToolChains/MinGW.cpp
+++ b/clang/lib/Driver/ToolChains/MinGW.cpp
@@ -793,9 +793,15 @@ static bool testTriple(const Driver &D, const llvm::Triple &Triple,
   if (D.SysRoot.size())
     return true;
   llvm::Triple LiteralTriple = getLiteralTriple(D, Triple);
+  std::string InstallBase =
+      std::string(llvm::sys::path::parent_path(D.getInstalledDir()));
   if (llvm::ErrorOr<std::string> TargetSubdir =
           findClangRelativeSysroot(D, LiteralTriple, Triple, SubdirName))
     return true;
+  // If the install base itself looks like a mingw sysroot, we'll use that
+  // - don't use any potentially unrelated gcc to influence what triple to use.
+  if (looksLikeMinGWSysroot(InstallBase))
+    return false;
   if (llvm::ErrorOr<std::string> GPPName = findGcc(LiteralTriple, Triple))
     return true;
   // If we neither found a colocated sysroot or a matching gcc executable,
```

Actually, for such cases, we could potentially check for the per-target runtime installs, e.g. looking for `<base>/include/<triple>` or `<base>/lib/<triple>`. Switching between multiple arches with e.g. `-m32` in such a setup could work, if all the arch specific files are in `<base>/lib/<triple>`, but I'm not sure if anyone does full mingw setups with such a hierarchy (yet). So this would be a separate potential future step.

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


More information about the cfe-commits mailing list