[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd758069f5e0d: [clang] [MinGW] Guess the right ix86 arch name 
spelling as sysroot (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111952

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-sysroot.cpp

Index: clang/test/Driver/mingw-sysroot.cpp
===
--- clang/test/Driver/mingw-sysroot.cpp
+++ clang/test/Driver/mingw-sysroot.cpp
@@ -12,6 +12,7 @@
 // RUN: mkdir -p %T/testroot-clang/bin
 // RUN: ln -s %clang %T/testroot-clang/bin/x86_64-w64-mingw32-clang
 // RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32 %T/testroot-clang/x86_64-w64-mingw32
+// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32 %T/testroot-clang/i686-w64-mingw32
 
 
 // If we find a gcc in the path with the right triplet prefix, pick that as
@@ -36,3 +37,14 @@
 // the libgcc directory:
 
 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-gcc/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s
+
+
+// If the user requests a different arch via the -m32 option, which changes
+// x86_64 into i386, check that the driver notices that it can't find a
+// sysroot for i386 but there is one for i686, and uses that one.
+// (In practice, the real usecase is when using an unprefixed native clang
+// that defaults to x86_64 mingw, but it's easier to test this in cross setups
+// with symlinks, like the other tests here.)
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang --target=x86_64-w64-mingw32 -m32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: "{{[^"]+}}/testroot-clang{{/|}}i686-w64-mingw32{{/|}}include"
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -60,6 +60,9 @@
   MinGW(const Driver , const llvm::Triple ,
 const llvm::opt::ArgList );
 
+  static void fixTripleArch(const Driver , llvm::Triple ,
+const llvm::opt::ArgList );
+
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
@@ -103,8 +106,6 @@
   mutable std::unique_ptr Preprocessor;
   mutable std::unique_ptr Compiler;
   void findGccLibDir();
-  llvm::ErrorOr findGcc();
-  llvm::ErrorOr findClangRelativeSysroot();
 
   bool NativeLLVMSupport;
 };
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -369,9 +369,9 @@
   }
 }
 
-llvm::ErrorOr toolchains::MinGW::findGcc() {
+static llvm::ErrorOr findGcc(const llvm::Triple ) {
   llvm::SmallVector, 2> Gccs;
-  Gccs.emplace_back(getTriple().getArchName());
+  Gccs.emplace_back(T.getArchName());
   Gccs[0] += "-w64-mingw32-gcc";
   Gccs.emplace_back("mingw32-gcc");
   // Please do not add "gcc" here
@@ -381,13 +381,14 @@
   return make_error_code(std::errc::no_such_file_or_directory);
 }
 
-llvm::ErrorOr toolchains::MinGW::findClangRelativeSysroot() {
+static llvm::ErrorOr
+findClangRelativeSysroot(const Driver , const llvm::Triple ,
+ std::string ) {
   llvm::SmallVector, 2> Subdirs;
-  Subdirs.emplace_back(getTriple().str());
-  Subdirs.emplace_back(getTriple().getArchName());
+  Subdirs.emplace_back(T.str());
+  Subdirs.emplace_back(T.getArchName());
   Subdirs[1] += "-w64-mingw32";
-  StringRef ClangRoot =
-  llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  StringRef ClangRoot = llvm::sys::path::parent_path(D.getInstalledDir());
   StringRef Sep = llvm::sys::path::get_separator();
   for (StringRef CandidateSubdir : Subdirs) {
 if (llvm::sys::fs::is_directory(ClangRoot + Sep + CandidateSubdir)) {
@@ -404,13 +405,16 @@
   RocmInstallation(D, Triple, Args) {
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
+  // The sequence for detecting a sysroot here should be kept in sync with
+  // the testTriple function below.
   if (getDriver().SysRoot.size())
 Base = getDriver().SysRoot;
   // Look for /../; if found, use /.. as the
   // base as it could still be a base for a gcc setup with libgcc.
-  else if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot())
+  else if (llvm::ErrorOr TargetSubdir =
+   findClangRelativeSysroot(getDriver(), getTriple(), SubdirName))
 Base = 

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: clang/lib/Driver/ToolChains/MinGW.h:63
 
+  static void FixTripleArch(const Driver , llvm::Triple ,
+const llvm::opt::ArgList );

MaskRay wrote:
> Use `functionName` for newer functions. (It's unfortunate that Clang has much 
> inconsistency but we should do the right thing for new methods.)
Sure, will change it.



Comment at: clang/test/Driver/mingw-sysroot.cpp:49
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" 
%T/testroot-clang/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -m32 
-rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck 
-check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: 
"{{[^"]+}}/testroot-clang{{/|}}i686-w64-mingw32{{/|}}include"

MaskRay wrote:
> Changing `PATH` seems unreliable for some test environments.
> 
> Any alternative way to test this?
> 
> `--target=` is preferred over `-target ` for driver options. `-target ` is 
> legacy but we probably need to keep indefinitely for its wide usage.
> 
> `linux-cross.cpp` has some nice tests which may inspire this file ? :)
Using `PATH` isn't very nice, no, but it's at least reliable enough to run on 
the current testing setups?

I don't see any good input for testing this in linux-cross.cpp - that file uses 
explicit `--sysroot=` parameters all over the place, while this test file is 
mostly about how to best implicitly deduce the sysroot when one isn't specified.

This file (both the old and the newly added tests) relies on checking what 
directories it finds in `/../`, so it requires 
fake-installing the clang binary by symlinking into a test toolchain tree.

I guess it could be possible to execute that fake-installed clang binary by 
specifying the direct path to it, instead of adding it to `PATH` - but it's an 
important usecase to make sure that it works when the literal path isn't 
spelled out in `argv[0]` too.

I can change to use `--target=` in the added lines, but the rest of the file 
uses the other spelling anyway (and I don't think it's on topic to do such 
rewrites as part of this patch).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111952

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 382946.
mstorsjo marked an inline comment as done.
mstorsjo added a comment.

Fixed the naming of the new function, using `--target=` in the newly added test 
lines, fixed a case of missed clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111952

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-sysroot.cpp

Index: clang/test/Driver/mingw-sysroot.cpp
===
--- clang/test/Driver/mingw-sysroot.cpp
+++ clang/test/Driver/mingw-sysroot.cpp
@@ -12,6 +12,7 @@
 // RUN: mkdir -p %T/testroot-clang/bin
 // RUN: ln -s %clang %T/testroot-clang/bin/x86_64-w64-mingw32-clang
 // RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32 %T/testroot-clang/x86_64-w64-mingw32
+// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32 %T/testroot-clang/i686-w64-mingw32
 
 
 // If we find a gcc in the path with the right triplet prefix, pick that as
@@ -36,3 +37,14 @@
 // the libgcc directory:
 
 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-gcc/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s
+
+
+// If the user requests a different arch via the -m32 option, which changes
+// x86_64 into i386, check that the driver notices that it can't find a
+// sysroot for i386 but there is one for i686, and uses that one.
+// (In practice, the real usecase is when using an unprefixed native clang
+// that defaults to x86_64 mingw, but it's easier to test this in cross setups
+// with symlinks, like the other tests here.)
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang --target=x86_64-w64-mingw32 -m32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: "{{[^"]+}}/testroot-clang{{/|}}i686-w64-mingw32{{/|}}include"
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -60,6 +60,9 @@
   MinGW(const Driver , const llvm::Triple ,
 const llvm::opt::ArgList );
 
+  static void fixTripleArch(const Driver , llvm::Triple ,
+const llvm::opt::ArgList );
+
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
@@ -103,8 +106,6 @@
   mutable std::unique_ptr Preprocessor;
   mutable std::unique_ptr Compiler;
   void findGccLibDir();
-  llvm::ErrorOr findGcc();
-  llvm::ErrorOr findClangRelativeSysroot();
 
   bool NativeLLVMSupport;
 };
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -369,9 +369,9 @@
   }
 }
 
-llvm::ErrorOr toolchains::MinGW::findGcc() {
+static llvm::ErrorOr findGcc(const llvm::Triple ) {
   llvm::SmallVector, 2> Gccs;
-  Gccs.emplace_back(getTriple().getArchName());
+  Gccs.emplace_back(T.getArchName());
   Gccs[0] += "-w64-mingw32-gcc";
   Gccs.emplace_back("mingw32-gcc");
   // Please do not add "gcc" here
@@ -381,13 +381,14 @@
   return make_error_code(std::errc::no_such_file_or_directory);
 }
 
-llvm::ErrorOr toolchains::MinGW::findClangRelativeSysroot() {
+static llvm::ErrorOr
+findClangRelativeSysroot(const Driver , const llvm::Triple ,
+ std::string ) {
   llvm::SmallVector, 2> Subdirs;
-  Subdirs.emplace_back(getTriple().str());
-  Subdirs.emplace_back(getTriple().getArchName());
+  Subdirs.emplace_back(T.str());
+  Subdirs.emplace_back(T.getArchName());
   Subdirs[1] += "-w64-mingw32";
-  StringRef ClangRoot =
-  llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  StringRef ClangRoot = llvm::sys::path::parent_path(D.getInstalledDir());
   StringRef Sep = llvm::sys::path::get_separator();
   for (StringRef CandidateSubdir : Subdirs) {
 if (llvm::sys::fs::is_directory(ClangRoot + Sep + CandidateSubdir)) {
@@ -404,13 +405,16 @@
   RocmInstallation(D, Triple, Args) {
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
+  // The sequence for detecting a sysroot here should be kept in sync with
+  // the testTriple function below.
   if (getDriver().SysRoot.size())
 Base = getDriver().SysRoot;
   // Look for /../; if found, use /.. as the
   // base as it could still be a base for a gcc setup with libgcc.
-  else if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot())
+  else if (llvm::ErrorOr TargetSubdir =
+   findClangRelativeSysroot(getDriver(), getTriple(), SubdirName))
 Base = 

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/MinGW.h:63
 
+  static void FixTripleArch(const Driver , llvm::Triple ,
+const llvm::opt::ArgList );

Use `functionName` for newer functions. (It's unfortunate that Clang has much 
inconsistency but we should do the right thing for new methods.)



Comment at: clang/test/Driver/mingw-sysroot.cpp:49
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" 
%T/testroot-clang/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -m32 
-rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck 
-check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: 
"{{[^"]+}}/testroot-clang{{/|}}i686-w64-mingw32{{/|}}include"

Changing `PATH` seems unreliable for some test environments.

Any alternative way to test this?

`--target=` is preferred over `-target ` for driver options. `-target ` is 
legacy but we probably need to keep indefinitely for its wide usage.

`linux-cross.cpp` has some nice tests which may inspire this file ? :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111952

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 382801.
mstorsjo added a comment.

Rebased after renaming `SysrootName` to `SubdirName`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111952

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-sysroot.cpp

Index: clang/test/Driver/mingw-sysroot.cpp
===
--- clang/test/Driver/mingw-sysroot.cpp
+++ clang/test/Driver/mingw-sysroot.cpp
@@ -12,6 +12,7 @@
 // RUN: mkdir -p %T/testroot-clang/bin
 // RUN: ln -s %clang %T/testroot-clang/bin/x86_64-w64-mingw32-clang
 // RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32 %T/testroot-clang/x86_64-w64-mingw32
+// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32 %T/testroot-clang/i686-w64-mingw32
 
 
 // If we find a gcc in the path with the right triplet prefix, pick that as
@@ -36,3 +37,14 @@
 // the libgcc directory:
 
 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-gcc/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s
+
+
+// If the user requests a different arch via the -m32 option, which changes
+// x86_64 into i386, check that the driver notices that it can't find a
+// sysroot for i386 but there is one for i686, and uses that one.
+// (In practice, this usecase is when using an unprefixed native clang
+// that defaults to x86_64 mingw, but it's easier to test in cross setups
+// with symlinks, like the other tests here.)
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -m32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: "{{[^"]+}}/testroot-clang{{/|}}i686-w64-mingw32{{/|}}include"
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -60,6 +60,9 @@
   MinGW(const Driver , const llvm::Triple ,
 const llvm::opt::ArgList );
 
+  static void FixTripleArch(const Driver , llvm::Triple ,
+const llvm::opt::ArgList );
+
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
@@ -103,8 +106,6 @@
   mutable std::unique_ptr Preprocessor;
   mutable std::unique_ptr Compiler;
   void findGccLibDir();
-  llvm::ErrorOr findGcc();
-  llvm::ErrorOr findClangRelativeSysroot();
 
   bool NativeLLVMSupport;
 };
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -369,9 +369,9 @@
   }
 }
 
-llvm::ErrorOr toolchains::MinGW::findGcc() {
+static llvm::ErrorOr findGcc(const llvm::Triple ) {
   llvm::SmallVector, 2> Gccs;
-  Gccs.emplace_back(getTriple().getArchName());
+  Gccs.emplace_back(T.getArchName());
   Gccs[0] += "-w64-mingw32-gcc";
   Gccs.emplace_back("mingw32-gcc");
   // Please do not add "gcc" here
@@ -381,13 +381,14 @@
   return make_error_code(std::errc::no_such_file_or_directory);
 }
 
-llvm::ErrorOr toolchains::MinGW::findClangRelativeSysroot() {
+static llvm::ErrorOr
+findClangRelativeSysroot(const Driver , const llvm::Triple ,
+ std::string ) {
   llvm::SmallVector, 2> Subdirs;
-  Subdirs.emplace_back(getTriple().str());
-  Subdirs.emplace_back(getTriple().getArchName());
+  Subdirs.emplace_back(T.str());
+  Subdirs.emplace_back(T.getArchName());
   Subdirs[1] += "-w64-mingw32";
-  StringRef ClangRoot =
-  llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  StringRef ClangRoot = llvm::sys::path::parent_path(D.getInstalledDir());
   StringRef Sep = llvm::sys::path::get_separator();
   for (StringRef CandidateSubdir : Subdirs) {
 if (llvm::sys::fs::is_directory(ClangRoot + Sep + CandidateSubdir)) {
@@ -404,13 +405,16 @@
   RocmInstallation(D, Triple, Args) {
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
+  // The sequence for detecting a sysroot here should be kept in sync with
+  // the testTriple function below.
   if (getDriver().SysRoot.size())
 Base = getDriver().SysRoot;
   // Look for /../; if found, use /.. as the
   // base as it could still be a base for a gcc setup with libgcc.
-  else if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot())
+  else if (llvm::ErrorOr TargetSubdir =
+   findClangRelativeSysroot(getDriver(), getTriple(), SubdirName))
 Base = std::string(llvm::sys::path::parent_path(TargetSubdir.get()));
-  else if (llvm::ErrorOr GPPName = findGcc())
+  else if (llvm::ErrorOr 

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 381795.
mstorsjo added a comment.

Moved the fixup into `computeTargetTriple`, so it's only done after applying 
the `-m32`/`-m64` options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111952

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-sysroot.cpp

Index: clang/test/Driver/mingw-sysroot.cpp
===
--- clang/test/Driver/mingw-sysroot.cpp
+++ clang/test/Driver/mingw-sysroot.cpp
@@ -12,6 +12,7 @@
 // RUN: mkdir -p %T/testroot-clang/bin
 // RUN: ln -s %clang %T/testroot-clang/bin/x86_64-w64-mingw32-clang
 // RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32 %T/testroot-clang/x86_64-w64-mingw32
+// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32 %T/testroot-clang/i686-w64-mingw32
 
 
 // If we find a gcc in the path with the right triplet prefix, pick that as
@@ -36,3 +37,14 @@
 // the libgcc directory:
 
 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-gcc/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s
+
+
+// If the user requests a different arch via the -m32 option, which changes
+// x86_64 into i386, check that the driver notices that it can't find a
+// sysroot for i386 but there is one for i686, and uses that one.
+// (In practice, this usecase is when using an unprefixed native clang
+// that defaults to x86_64 mingw, but it's easier to test in cross setups
+// with symlinks, like the other tests here.)
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -m32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: "{{[^"]+}}/testroot-clang{{/|}}i686-w64-mingw32{{/|}}include"
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -60,6 +60,9 @@
   MinGW(const Driver , const llvm::Triple ,
 const llvm::opt::ArgList );
 
+  static void FixTripleArch(const Driver , llvm::Triple ,
+const llvm::opt::ArgList );
+
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
@@ -103,8 +106,6 @@
   mutable std::unique_ptr Preprocessor;
   mutable std::unique_ptr Compiler;
   void findGccLibDir();
-  llvm::ErrorOr findGcc();
-  llvm::ErrorOr findClangRelativeSysroot();
 
   bool NativeLLVMSupport;
 };
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -369,9 +369,9 @@
   }
 }
 
-llvm::ErrorOr toolchains::MinGW::findGcc() {
+static llvm::ErrorOr findGcc(const llvm::Triple ) {
   llvm::SmallVector, 2> Gccs;
-  Gccs.emplace_back(getTriple().getArchName());
+  Gccs.emplace_back(T.getArchName());
   Gccs[0] += "-w64-mingw32-gcc";
   Gccs.emplace_back("mingw32-gcc");
   // Please do not add "gcc" here
@@ -381,13 +381,14 @@
   return make_error_code(std::errc::no_such_file_or_directory);
 }
 
-llvm::ErrorOr toolchains::MinGW::findClangRelativeSysroot() {
+static llvm::ErrorOr
+findClangRelativeSysroot(const Driver , const llvm::Triple ,
+ std::string ) {
   llvm::SmallVector, 2> Subdirs;
-  Subdirs.emplace_back(getTriple().str());
-  Subdirs.emplace_back(getTriple().getArchName());
+  Subdirs.emplace_back(T.str());
+  Subdirs.emplace_back(T.getArchName());
   Subdirs[1] += "-w64-mingw32";
-  StringRef ClangRoot =
-  llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  StringRef ClangRoot = llvm::sys::path::parent_path(D.getInstalledDir());
   StringRef Sep = llvm::sys::path::get_separator();
   for (StringRef CandidateSubdir : Subdirs) {
 if (llvm::sys::fs::is_directory(ClangRoot + Sep + CandidateSubdir)) {
@@ -404,13 +405,16 @@
   RocmInstallation(D, Triple, Args) {
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
+  // The sequence for detecting a sysroot here should be kept in sync with
+  // the testTriple function below.
   if (getDriver().SysRoot.size())
 Base = getDriver().SysRoot;
   // Look for /../; if found, use /.. as the
   // base as it could still be a base for a gcc setup with libgcc.
-  else if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot())
+  else if (llvm::ErrorOr TargetSubdir =
+   findClangRelativeSysroot(getDriver(), getTriple(), SysrootName))
 Base = std::string(llvm::sys::path::parent_path(TargetSubdir.get()));
-  else if (llvm::ErrorOr 

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D111952#3068680 , @mstorsjo wrote:

> I guess a downside of this solution, is that if an `i686` sysroot exists next 
> to the clang binary, it becomes practically impossible to test codegen 
> differences when you run it with various `-target iX86-w64-mingw32` options, 
> as they'd all be corrected back to `i686`. (This wouldn't be an issue if the 
> autodetection was done in `computeTargetTriple` only when the `-m32`/`-m64` 
> options are used though.)

Ok, it turned out to not be that bad to move the fixup into 
`computeTargetTriple` to this location after all, it looks fairly neat and not 
that much out of line compared to the existing triple tweaks done there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111952

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

I guess a downside of this solution, is that if an `i686` sysroot exists next 
to the clang binary, it becomes practically impossible to test codegen 
differences when you run it with various `-target iX86-w64-mingw32` options, as 
they'd all be corrected back to `i686`. (This wouldn't be an issue if the 
autodetection was done in `computeTargetTriple` only when the `-m32`/`-m64` 
options are used though.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111952

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: phosek, MaskRay.
Herald added subscribers: pengfei, kristof.beyls.
mstorsjo requested review of this revision.
Herald added a project: clang.

For x86, most contempory mingw toolchains use i686 as 32 bit
x86 arch target.

As long as the target triple is set to the right form, this works
fine, either as the compiler's default target, or via e.g.
a triple prefix like i686-w64-mingw32-clang.

However, if the unprefixed toolchain targets x86_64, but the user
tries to switch it to target 32 bit by adding the -m32 option, the
computeTargetTriple function in Clang, together with
Triple::get32BitArchVariant, sets the arch to i386. This causes
the right sysroot to not be found.

When targeting an arch where there are potential spelling ambiguities
with respect to the sysroots (i386 and arm), check if the driver can
find a sysroot with the arch name - if not, try a couple other
candidates.

Other design ideas considered: It would fit in better with the design
to do this kind of arch name fixups in computeTargetTriple (it
already has a couple other OS/arch specific cases), but the
heuristics for detecting the potential right choice ties in quite
closely with how the toolchain looks for the implicit sysroot to use,
so I'd prefer to keep that logic in the toolchain::MinGW class/file.

This is done as a wrapper function before invoking the toolchain::MinGW()
constructor, because the MinGW() constructor can't run code of its own
until the superclass ToolChain() constructor returns. The fixups need to
be done before the ToolChain() constructor, because it uses the Triple
for per-target runtime directories.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111952

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-sysroot.cpp

Index: clang/test/Driver/mingw-sysroot.cpp
===
--- clang/test/Driver/mingw-sysroot.cpp
+++ clang/test/Driver/mingw-sysroot.cpp
@@ -12,6 +12,7 @@
 // RUN: mkdir -p %T/testroot-clang/bin
 // RUN: ln -s %clang %T/testroot-clang/bin/x86_64-w64-mingw32-clang
 // RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32 %T/testroot-clang/x86_64-w64-mingw32
+// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32 %T/testroot-clang/i686-w64-mingw32
 
 
 // If we find a gcc in the path with the right triplet prefix, pick that as
@@ -36,3 +37,14 @@
 // the libgcc directory:
 
 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-gcc/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s
+
+
+// If the user requests a different arch via the -m32 option, which changes
+// x86_64 into i386, check that the driver notices that it can't find a
+// sysroot for i386 but there is one for i686, and uses that one.
+// (In practice, this usecase is when using an unprefixed native clang
+// that defaults to x86_64 mingw, but it's easier to test in cross setups
+// with symlinks, like the other tests here.)
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -m32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: "{{[^"]+}}/testroot-clang{{/|}}i686-w64-mingw32{{/|}}include"
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -56,10 +56,15 @@
 namespace toolchains {
 
 class LLVM_LIBRARY_VISIBILITY MinGW : public ToolChain {
-public:
+private:
   MinGW(const Driver , const llvm::Triple ,
 const llvm::opt::ArgList );
 
+public:
+  static std::unique_ptr Create(const Driver ,
+   const llvm::Triple ,
+   const llvm::opt::ArgList );
+
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
@@ -103,8 +108,6 @@
   mutable std::unique_ptr Preprocessor;
   mutable std::unique_ptr Compiler;
   void findGccLibDir();
-  llvm::ErrorOr findGcc();
-  llvm::ErrorOr findClangRelativeSysroot();
 
   bool NativeLLVMSupport;
 };
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -369,9 +369,9 @@
   }
 }
 
-llvm::ErrorOr toolchains::MinGW::findGcc() {
+static llvm::ErrorOr findGcc(const llvm::Triple ) {
   llvm::SmallVector, 2> Gccs;
-  Gccs.emplace_back(getTriple().getArchName());
+  Gccs.emplace_back(T.getArchName());
   Gccs[0] += "-w64-mingw32-gcc";
   Gccs.emplace_back("mingw32-gcc");
   //