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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
