[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

[Rainer Orth via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGae84ad15efd7: [Driver] Select newest GCC installation on 
Solaris (authored by ro).

Changed prior to commit:
  https://reviews.llvm.org/D157275?vs=550226=550651#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157275

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/unittests/Driver/ToolChainTest.cpp

Index: clang/unittests/Driver/ToolChainTest.cpp
===
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -168,6 +168,155 @@
 S);
 }
 
+TEST(ToolChainTest, VFSSolarisMultiGCCInstallation) {
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+  struct TestDiagnosticConsumer : public DiagnosticConsumer {};
+  IntrusiveRefCntPtr InMemoryFileSystem(
+  new llvm::vfs::InMemoryFileSystem);
+
+  const char *EmptyFiles[] = {
+  // Sort entries so the latest version doesn't come first.
+  "/usr/gcc/7/lib/gcc/sparcv9-sun-solaris2.11/7.5.0/32/crtbegin.o",
+  "/usr/gcc/7/lib/gcc/sparcv9-sun-solaris2.11/7.5.0/crtbegin.o",
+  "/usr/gcc/7/lib/gcc/x86_64-pc-solaris2.11/7.5.0/32/crtbegin.o",
+  "/usr/gcc/7/lib/gcc/x86_64-pc-solaris2.11/7.5.0/crtbegin.o",
+  "/usr/gcc/11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0/crtbegin.o",
+  "/usr/gcc/11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0/sparcv8plus/"
+  "crtbegin.o",
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0/32/crtbegin.o",
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0/crtbegin.o",
+  "/usr/gcc/4.7/lib/gcc/i386-pc-solaris2.11/4.7.3/amd64/crtbegin.o",
+  "/usr/gcc/4.7/lib/gcc/i386-pc-solaris2.11/4.7.3/crtbegin.o",
+  "/usr/gcc/4.7/lib/gcc/sparc-sun-solaris2.11/4.7.3/crtbegin.o",
+  "/usr/gcc/4.7/lib/gcc/sparc-sun-solaris2.11/4.7.3/sparcv9/crtbegin.o",
+  };
+
+  for (const char *Path : EmptyFiles)
+InMemoryFileSystem->addFile(Path, 0,
+llvm::MemoryBuffer::getMemBuffer("\n"));
+
+  {
+DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer);
+Driver TheDriver("/bin/clang", "i386-pc-solaris2.11", Diags,
+ "clang LLVM compiler", InMemoryFileSystem);
+std::unique_ptr C(
+TheDriver.BuildCompilation({"-v", "--gcc-toolchain=", "--sysroot="}));
+ASSERT_TRUE(C);
+std::string S;
+{
+  llvm::raw_string_ostream OS(S);
+  C->getDefaultToolChain().printVerboseInfo(OS);
+}
+if (is_style_windows(llvm::sys::path::Style::native))
+  std::replace(S.begin(), S.end(), '\\', '/');
+EXPECT_EQ("Found candidate GCC installation: "
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0\n"
+  "Selected GCC installation: "
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0\n"
+  "Candidate multilib: .;@m64\n"
+  "Candidate multilib: 32;@m32\n"
+  "Selected multilib: 32;@m32\n",
+  S);
+  }
+
+  {
+DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer);
+Driver TheDriver("/bin/clang", "amd64-pc-solaris2.11", Diags,
+ "clang LLVM compiler", InMemoryFileSystem);
+std::unique_ptr C(
+TheDriver.BuildCompilation({"-v", "--gcc-toolchain=", "--sysroot="}));
+ASSERT_TRUE(C);
+std::string S;
+{
+  llvm::raw_string_ostream OS(S);
+  C->getDefaultToolChain().printVerboseInfo(OS);
+}
+if (is_style_windows(llvm::sys::path::Style::native))
+  std::replace(S.begin(), S.end(), '\\', '/');
+EXPECT_EQ("Found candidate GCC installation: "
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0\n"
+  "Selected GCC installation: "
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0\n"
+  "Candidate multilib: .;@m64\n"
+  "Candidate multilib: 32;@m32\n"
+  "Selected multilib: .;@m64\n",
+  S);
+  }
+
+  {
+DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer);
+Driver TheDriver("/bin/clang", "x86_64-pc-solaris2.11", Diags,
+ "clang LLVM compiler", InMemoryFileSystem);
+std::unique_ptr C(
+TheDriver.BuildCompilation({"-v", "--gcc-toolchain=", "--sysroot="}));
+ASSERT_TRUE(C);
+std::string S;
+{
+  llvm::raw_string_ostream OS(S);
+  C->getDefaultToolChain().printVerboseInfo(OS);
+}
+if (is_style_windows(llvm::sys::path::Style::native))
+  std::replace(S.begin(), S.end(), '\\', '/');
+EXPECT_EQ("Found candidate GCC installation: "
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0\n"
+  "Selected GCC installation: "
+  

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Thanks!




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2258
 
-  Prefixes.push_back(CandidatePrefix);
+  SolarisPrefixes.push_back(CandidatePrefix);
 }

ro wrote:
> MaskRay wrote:
> > It's better to use https://en.wikipedia.org/wiki/Schwartzian_transform here 
> > to avoid calling `llvm::sys::path::filename` and 
> > `Generic_GCC::GCCVersion::Parse` in the comparator. Then the comparator is 
> > just `A < B` and can just be removed.
> This is way shorter and efficient indeed.  Patch updated.
Use `emplace_back`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157275

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

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro updated this revision to Diff 550226.
ro marked an inline comment as done.
ro added a comment.
Herald added a subscriber: ormris.

- Simplify `SolarisPrefixes` sorting.
- Switch to unitest`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157275

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/unittests/Driver/ToolChainTest.cpp

Index: clang/unittests/Driver/ToolChainTest.cpp
===
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -168,6 +168,155 @@
 S);
 }
 
+TEST(ToolChainTest, VFSSolarisMultiGCCInstallation) {
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+  struct TestDiagnosticConsumer : public DiagnosticConsumer {};
+  IntrusiveRefCntPtr InMemoryFileSystem(
+  new llvm::vfs::InMemoryFileSystem);
+
+  const char *EmptyFiles[] = {
+  // Sort entries so the latest version doesn't come first.
+  "/usr/gcc/7/lib/gcc/sparcv9-sun-solaris2.11/7.5.0/32/crtbegin.o",
+  "/usr/gcc/7/lib/gcc/sparcv9-sun-solaris2.11/7.5.0/crtbegin.o",
+  "/usr/gcc/7/lib/gcc/x86_64-pc-solaris2.11/7.5.0/32/crtbegin.o",
+  "/usr/gcc/7/lib/gcc/x86_64-pc-solaris2.11/7.5.0/crtbegin.o",
+  "/usr/gcc/11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0/crtbegin.o",
+  "/usr/gcc/11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0/sparcv8plus/"
+  "crtbegin.o",
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0/32/crtbegin.o",
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0/crtbegin.o",
+  "/usr/gcc/4.7/lib/gcc/i386-pc-solaris2.11/4.7.3/amd64/crtbegin.o",
+  "/usr/gcc/4.7/lib/gcc/i386-pc-solaris2.11/4.7.3/crtbegin.o",
+  "/usr/gcc/4.7/lib/gcc/sparc-sun-solaris2.11/4.7.3/crtbegin.o",
+  "/usr/gcc/4.7/lib/gcc/sparc-sun-solaris2.11/4.7.3/sparcv9/crtbegin.o",
+  };
+
+  for (const char *Path : EmptyFiles)
+InMemoryFileSystem->addFile(Path, 0,
+llvm::MemoryBuffer::getMemBuffer("\n"));
+
+  {
+DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer);
+Driver TheDriver("/bin/clang", "i386-pc-solaris2.11", Diags,
+ "clang LLVM compiler", InMemoryFileSystem);
+std::unique_ptr C(
+TheDriver.BuildCompilation({"-v", "--gcc-toolchain=", "--sysroot="}));
+ASSERT_TRUE(C);
+std::string S;
+{
+  llvm::raw_string_ostream OS(S);
+  C->getDefaultToolChain().printVerboseInfo(OS);
+}
+if (is_style_windows(llvm::sys::path::Style::native))
+  std::replace(S.begin(), S.end(), '\\', '/');
+EXPECT_EQ("Found candidate GCC installation: "
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0\n"
+  "Selected GCC installation: "
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0\n"
+  "Candidate multilib: .;@m64\n"
+  "Candidate multilib: 32;@m32\n"
+  "Selected multilib: 32;@m32\n",
+  S);
+  }
+
+  {
+DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer);
+Driver TheDriver("/bin/clang", "amd64-pc-solaris2.11", Diags,
+ "clang LLVM compiler", InMemoryFileSystem);
+std::unique_ptr C(
+TheDriver.BuildCompilation({"-v", "--gcc-toolchain=", "--sysroot="}));
+ASSERT_TRUE(C);
+std::string S;
+{
+  llvm::raw_string_ostream OS(S);
+  C->getDefaultToolChain().printVerboseInfo(OS);
+}
+if (is_style_windows(llvm::sys::path::Style::native))
+  std::replace(S.begin(), S.end(), '\\', '/');
+EXPECT_EQ("Found candidate GCC installation: "
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0\n"
+  "Selected GCC installation: "
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0\n"
+  "Candidate multilib: .;@m64\n"
+  "Candidate multilib: 32;@m32\n"
+  "Selected multilib: .;@m64\n",
+  S);
+  }
+
+  {
+DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer);
+Driver TheDriver("/bin/clang", "x86_64-pc-solaris2.11", Diags,
+ "clang LLVM compiler", InMemoryFileSystem);
+std::unique_ptr C(
+TheDriver.BuildCompilation({"-v", "--gcc-toolchain=", "--sysroot="}));
+ASSERT_TRUE(C);
+std::string S;
+{
+  llvm::raw_string_ostream OS(S);
+  C->getDefaultToolChain().printVerboseInfo(OS);
+}
+if (is_style_windows(llvm::sys::path::Style::native))
+  std::replace(S.begin(), S.end(), '\\', '/');
+EXPECT_EQ("Found candidate GCC installation: "
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0\n"
+  "Selected GCC installation: "
+  "/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0\n"
+  "Candidate multilib: .;@m64\n"
+  "Candidate multilib: 32;@m32\n"
+  

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro marked an inline comment as done.
ro added a comment.

In D157275#4585184 , @MaskRay wrote:

> Thanks for the update. Regarding testing, it seems that unittests will be 
> more convenience than creating so many placeholder files in `Inputs/`. 
> `clang/unittests/Driver/ToolChainTest.cpp` `TEST(ToolChainTest, 
> VFSGCCInstallation)` has an example for Linux. You can add another for 
> Solaris.

TBH, I'm quite sceptical about the unittest framework here: we already have 
2300+ empty files in `clang/test/Driver`, so adding 12 more shouldn't be such a 
big deal.  With a real in-filesystem tree, I can quickly test various 
scenarious with `clang -v --sysroot`, while this isn't possible with the 
unittest framework (which I find way harder to read).  I'm adding the unittest 
nonetheless, but would prefer to keep the `lit`-based test instead.




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2258
 
-  Prefixes.push_back(CandidatePrefix);
+  SolarisPrefixes.push_back(CandidatePrefix);
 }

MaskRay wrote:
> It's better to use https://en.wikipedia.org/wiki/Schwartzian_transform here 
> to avoid calling `llvm::sys::path::filename` and 
> `Generic_GCC::GCCVersion::Parse` in the comparator. Then the comparator is 
> just `A < B` and can just be removed.
This is way shorter and efficient indeed.  Patch updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157275

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

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Thanks for the update. Regarding testing, it seems that unittests will be more 
convenience than creating so many placeholder files in `Inputs/`. 
`clang/unittests/Driver/ToolChainTest.cpp` `TEST(ToolChainTest, 
VFSGCCInstallation)` has an example for Linux. You can add another for Solaris.




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2258
 
-  Prefixes.push_back(CandidatePrefix);
+  SolarisPrefixes.push_back(CandidatePrefix);
 }

It's better to use https://en.wikipedia.org/wiki/Schwartzian_transform here to 
avoid calling `llvm::sys::path::filename` and `Generic_GCC::GCCVersion::Parse` 
in the comparator. Then the comparator is just `A < B` and can just be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157275

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

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro updated this revision to Diff 549925.
ro added a comment.
Herald added a subscriber: mgrang.

Updated based on discussions in Issue #53709:

- Sort Solaris GCC prefixes in reverse version order so the latest version is 
picked.
- Update testcase to match.

Tested on `amd64-pc-solaris2.11` and `x86_64-pc-linux-gnu`.

I'm seeing weird testsuite failures in 2-stage builds on Solaris/amd64 with gcc 
12, but not with clang 16, which I don't yet understand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157275

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0/sparcv8plus/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0/32/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/i386-pc-solaris2.11/4.7.3/amd64/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/i386-pc-solaris2.11/4.7.3/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/sparc-sun-solaris2.11/4.7.3/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/sparc-sun-solaris2.11/4.7.3/sparcv9/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/sparcv9-sun-solaris2.11/7.5.0/32/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/sparcv9-sun-solaris2.11/7.5.0/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/x86_64-pc-solaris2.11/7.5.0/32/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/x86_64-pc-solaris2.11/7.5.0/crtbegin.o
  clang/test/Driver/solaris-multi-gcc-search.test


Index: clang/test/Driver/solaris-multi-gcc-search.test
===
--- /dev/null
+++ clang/test/Driver/solaris-multi-gcc-search.test
@@ -0,0 +1,51 @@
+/// Check that clang uses the latest available Solaris GCC installation.
+
+/// Check sparc-sun-solaris2.11, 32-bit
+// RUN: %clang -v 2>&1 --target=sparc-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SPARC %s
+// CHECK-SPARC: Found candidate GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARC-NEXT: Selected GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARC-NEXT: Candidate multilib: .;@m64
+// CHECK-SPARC-NEXT: Candidate multilib: sparcv8plus;@m32
+// CHECK-SPARC-NEXT: Selected multilib: sparcv8plus;@m32
+
+/// Check i386-pc-solaris2.11, 32-bit
+// RUN: %clang -v 2>&1 --target=i386-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-I386 %s
+// CHECK-I386: Found candidate GCC installation: 
{{.*}}11/lib/gcc/x86_64-pc-solaris2.11/11.4.0
+// CHECK-I386-NEXT: Selected GCC installation: 
{{.*}}11/lib/gcc/x86_64-pc-solaris2.11/11.4.0
+// CHECK-I386-NEXT: Candidate multilib: .;@m64
+// CHECK-I386-NEXT: Candidate multilib: 32;@m32
+// CHECK-I386-NEXT: Selected multilib: 32;@m32
+
+/// Check sparcv9-sun-solaris2.11, 64-bit
+// RUN: %clang -v 2>&1 --target=sparcv9-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SPARCV9 %s
+// CHECK-SPARCV9: Found candidate GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARCV9-NEXT: Selected GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARCV9-NEXT: Candidate multilib: .;@m64
+// CHECK-SPARCV9-NEXT: Candidate multilib: sparcv8plus;@m32
+// CHECK-SPARCV9-NEXT: Selected multilib: .;@m64
+
+/// Check x86_64-pc-solaris2.11, 64-bit
+// RUN: %clang -v 2>&1 --target=x86_64-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-X86_64 %s
+
+/// Check amd64-pc-solaris2.11, 64-bit
+// RUN: %clang -v 2>&1 --target=amd64-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-X86_64 %s
+// CHECK-X86_64: Found candidate GCC installation: 
{{.*}}11/lib/gcc/x86_64-pc-solaris2.11/11.4.0
+// CHECK-X86_64-NEXT: Selected GCC installation: 
{{.*}}11/lib/gcc/x86_64-pc-solaris2.11/11.4.0
+// CHECK-X86_64-NEXT: Candidate multilib: .;@m64
+// CHECK-X86_64-NEXT: Candidate multilib: 32;@m32
+// CHECK-X86_64-NEXT: Selected multilib: .;@m64
Index: 

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2193
 // Skip other prefixes once a GCC installation is found.
-if (Version > VersionZero)
+// Solaris GCC installations live in separate Prefixes per Version
+// (/usr/gcc/) that arrive in directory order, so check all

MaskRay wrote:
> Adding a Solaris special case here seems strange.
> 
> Do you know what the typical `Prefixes` values are on Solaris? Is it possible 
> to remove some elements as a special case for Solaris instead?
Agreed.  This patch in its current form is just meant as a stop-gap measure to 
unbreak basic Solaris functionality while avoiding to affect other targets.

I've described the value of `Prefixes` on Solaris in the Issue: its an array of 
`/usr/gcc/` for each installed version of GCC, however in directory order 
and thus random.  The exact set is variable since every version can be 
installed or uninstalled at any time, old versions get deprecated and removed, 
and new ones added over time.

My fundamental problem here is that I haven't yet been able to reproduce the 
failure your original patch is meant to fix, so I don't really know what to 
guard against.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157275

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

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2193
 // Skip other prefixes once a GCC installation is found.
-if (Version > VersionZero)
+// Solaris GCC installations live in separate Prefixes per Version
+// (/usr/gcc/) that arrive in directory order, so check all

Adding a Solaris special case here seems strange.

Do you know what the typical `Prefixes` values are on Solaris? Is it possible 
to remove some elements as a special case for Solaris instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157275

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

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro created this revision.
ro added a reviewer: MaskRay.
ro added a project: clang.
Herald added subscribers: pengfei, jrtc27, fedor.sergeev, jyknight.
Herald added a project: All.
ro requested review of this revision.

As described in Issue #53709 
, since 
28d58d8fe2094af6902dee7b4d68ec30a3e9d737 
 `clang` 
doesn't find the latest of several parallel GCC installations on Solaris, but 
only the first in directory order, which is pretty random.

Since this breaks basic functionality of the Solaris port, this patch restores 
scanning all prefixes for GCC installations.

Tested on `sparcv9-sun-solaris2.11`, `amd64-pc-solaris2.11`, and 
`x86_64-pc-linux-gnu`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157275

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0/sparcv8plus/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0/32/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/i386-pc-solaris2.11/4.7.3/amd64/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/i386-pc-solaris2.11/4.7.3/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/sparc-sun-solaris2.11/4.7.3/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/sparc-sun-solaris2.11/4.7.3/sparcv9/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/sparcv9-sun-solaris2.11/7.5.0/32/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/sparcv9-sun-solaris2.11/7.5.0/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/x86_64-pc-solaris2.11/7.5.0/32/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/x86_64-pc-solaris2.11/7.5.0/crtbegin.o
  clang/test/Driver/solaris-multi-gcc-search.test


Index: clang/test/Driver/solaris-multi-gcc-search.test
===
--- /dev/null
+++ clang/test/Driver/solaris-multi-gcc-search.test
@@ -0,0 +1,60 @@
+/// Check that clang uses the latest available Solaris GCC installation.
+
+/// Check sparc-sun-solaris2.11, 32-bit
+// RUN: %clang -v 2>&1 --target=sparc-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SPARC %s
+// CHECK-SPARC-DAG: Found candidate GCC installation: 
{{.*}}4.7/lib/gcc/sparc-sun-solaris2.11/4.7.3
+// CHECK-SPARC-DAG: Found candidate GCC installation: 
{{.*}}7/lib/gcc/sparcv9-sun-solaris2.11/7.5.0
+// CHECK-SPARC-DAG: Found candidate GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARC: Selected GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARC-NEXT: Candidate multilib: .;@m64
+// CHECK-SPARC-NEXT: Candidate multilib: sparcv8plus;@m32
+// CHECK-SPARC-NEXT: Selected multilib: sparcv8plus;@m32
+
+/// Check i386-pc-solaris2.11, 32-bit
+// RUN: %clang -v 2>&1 --target=i386-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-I386 %s
+// CHECK-I386-DAG: Found candidate GCC installation: 
{{.*}}4.7/lib/gcc/i386-pc-solaris2.11/4.7.3
+// CHECK-I386-DAG: Found candidate GCC installation: 
{{.*}}7/lib/gcc/x86_64-pc-solaris2.11/7.5.0
+// CHECK-I386-DAG: Found candidate GCC installation: 
{{.*}}11/lib/gcc/x86_64-pc-solaris2.11/11.4.0
+// CHECK-I386: Selected GCC installation: 
{{.*}}11/lib/gcc/x86_64-pc-solaris2.11/11.4.0
+// CHECK-I386-NEXT: Candidate multilib: .;@m64
+// CHECK-I386-NEXT: Candidate multilib: 32;@m32
+// CHECK-I386-NEXT: Selected multilib: 32;@m32
+
+/// Check sparcv9-sun-solaris2.11, 64-bit
+// RUN: %clang -v 2>&1 --target=sparcv9-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SPARCV9 %s
+// CHECK-SPARCV9-DAG: Found candidate GCC installation: 
{{.*}}4.7/lib/gcc/sparc-sun-solaris2.11/4.7.3
+// CHECK-SPARCV9-DAG: Found candidate GCC installation: 
{{.*}}7/lib/gcc/sparcv9-sun-solaris2.11/7.5.0
+// CHECK-SPARCV9-DAG: Found candidate GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARCV9: Selected GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARCV9-NEXT: Candidate multilib: .;@m64
+// CHECK-SPARCV9-NEXT: Candidate multilib: sparcv8plus;@m32
+// CHECK-SPARCV9-NEXT: Selected multilib: .;@m64
+