[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

(The first line of 0ffaffcaac97de31e1b0e7e80c4f7cab724eda20 
 should 
have mentioned `Lazyly initialize uncommon toolchain detector`. You can add 
`Reland` prefix or add the message in the body of the commit message.)
I fixed a `-fsanitize-address-stack-use-after-scope` issue in 
6ab9f1e59371fe96ca3fda1a26a28ae0b7caf637 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142606

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[serge via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0ffaffcaac97: Reapply 
6fa2abf90886f18472c87bc9bffbcdf4f73c465e (authored by serge-sans-paille).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142606

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/lib/Driver/ToolChains/LazyDetector.h
  clang/lib/Driver/ToolChains/Linux.cpp

Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -681,23 +681,23 @@
 
 void Linux::AddCudaIncludeArgs(const ArgList ,
ArgStringList ) const {
-  CudaInstallation.AddCudaIncludeArgs(DriverArgs, CC1Args);
+  CudaInstallation->AddCudaIncludeArgs(DriverArgs, CC1Args);
 }
 
 void Linux::AddHIPIncludeArgs(const ArgList ,
   ArgStringList ) const {
-  RocmInstallation.AddHIPIncludeArgs(DriverArgs, CC1Args);
+  RocmInstallation->AddHIPIncludeArgs(DriverArgs, CC1Args);
 }
 
 void Linux::AddHIPRuntimeLibArgs(const ArgList ,
  ArgStringList ) const {
   CmdArgs.push_back(
-  Args.MakeArgString(StringRef("-L") + RocmInstallation.getLibPath()));
+  Args.MakeArgString(StringRef("-L") + RocmInstallation->getLibPath()));
 
   if (Args.hasFlag(options::OPT_offload_add_rpath,
options::OPT_no_offload_add_rpath, false))
 CmdArgs.append(
-{"-rpath", Args.MakeArgString(RocmInstallation.getLibPath())});
+{"-rpath", Args.MakeArgString(RocmInstallation->getLibPath())});
 
   CmdArgs.push_back("-lamdhip64");
 }
Index: clang/lib/Driver/ToolChains/LazyDetector.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/LazyDetector.h
@@ -0,0 +1,45 @@
+//===--- LazyDetector.h - Lazy ToolChain Detection --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
+
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+#include 
+
+namespace clang {
+
+/// Simple wrapper for toolchain detector with costly initialization. This
+/// delays the creation of the actual detector until its first usage.
+
+template  class LazyDetector {
+  const driver::Driver 
+  const llvm::Triple 
+  const llvm::opt::ArgList 
+
+  std::optional Detector;
+
+public:
+  LazyDetector(const driver::Driver , const llvm::Triple ,
+   const llvm::opt::ArgList )
+  : D(D), Triple(Triple), Args(Args) {}
+  T *operator->() {
+if (!Detector)
+  Detector.emplace(D, Triple, Args);
+return &*Detector;
+  }
+  const T *operator->() const {
+return const_cast(
+const_cast(*this).operator->());
+  }
+};
+
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -314,7 +314,7 @@
 
 void HIPAMDToolChain::AddHIPIncludeArgs(const ArgList ,
 ArgStringList ) const {
-  RocmInstallation.AddHIPIncludeArgs(DriverArgs, CC1Args);
+  RocmInstallation->AddHIPIncludeArgs(DriverArgs, CC1Args);
 }
 
 SanitizerMask HIPAMDToolChain::getSupportedSanitizers() const {
@@ -343,7 +343,7 @@
   ArgStringList LibraryPaths;
 
   // Find in --hip-device-lib-path and HIP_LIBRARY_PATH.
-  for (StringRef Path : RocmInstallation.getRocmDeviceLibPathArg())
+  for (StringRef Path : RocmInstallation->getRocmDeviceLibPathArg())
 LibraryPaths.push_back(DriverArgs.MakeArgString(Path));
 
   addDirectoryList(DriverArgs, LibraryPaths, "", "HIP_DEVICE_LIB_PATH");
@@ -365,7 +365,7 @@
   getDriver().Diag(diag::err_drv_no_such_file) << BCName;
 });
   } else {
-if (!RocmInstallation.hasDeviceLibrary()) {
+if (!RocmInstallation->hasDeviceLibrary()) {
   getDriver().Diag(diag::err_drv_no_rocm_device_lib) << 0;
   return {};
 }
@@ -376,7 +376,7 @@
 if (DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
options::OPT_fno_gpu_sanitize, true) &&
 

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld reopened this revision.
Hahnfeld added a comment.

This broke `clang/test/Driver/rocm-detect.hip` on a number of platforms 
(including the pre-merge checks on this PR), I've reverted for now in 
b5ee4f755fcff56243f6ff0cea9e7a722259304a 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142606

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[Tom Weaver via Phabricator via cfe-commits]

TWeaver added a comment.

Hello all,

sorry to be a bother, but I believe this patch has caused a failure in this 
buildbot (and potentially others).

https://lab.llvm.org/buildbot/#/builders/109/builds/57277


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142606

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[serge via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6fa2abf90886: Lazyly initialize uncommon toolchain detector 
(authored by serge-sans-paille).

Changed prior to commit:
  https://reviews.llvm.org/D142606?vs=494927=495058#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142606

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/lib/Driver/ToolChains/LazyDetector.h
  clang/lib/Driver/ToolChains/Linux.cpp

Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -681,23 +681,23 @@
 
 void Linux::AddCudaIncludeArgs(const ArgList ,
ArgStringList ) const {
-  CudaInstallation.AddCudaIncludeArgs(DriverArgs, CC1Args);
+  CudaInstallation->AddCudaIncludeArgs(DriverArgs, CC1Args);
 }
 
 void Linux::AddHIPIncludeArgs(const ArgList ,
   ArgStringList ) const {
-  RocmInstallation.AddHIPIncludeArgs(DriverArgs, CC1Args);
+  RocmInstallation->AddHIPIncludeArgs(DriverArgs, CC1Args);
 }
 
 void Linux::AddHIPRuntimeLibArgs(const ArgList ,
  ArgStringList ) const {
   CmdArgs.push_back(
-  Args.MakeArgString(StringRef("-L") + RocmInstallation.getLibPath()));
+  Args.MakeArgString(StringRef("-L") + RocmInstallation->getLibPath()));
 
   if (Args.hasFlag(options::OPT_offload_add_rpath,
options::OPT_no_offload_add_rpath, false))
 CmdArgs.append(
-{"-rpath", Args.MakeArgString(RocmInstallation.getLibPath())});
+{"-rpath", Args.MakeArgString(RocmInstallation->getLibPath())});
 
   CmdArgs.push_back("-lamdhip64");
 }
Index: clang/lib/Driver/ToolChains/LazyDetector.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/LazyDetector.h
@@ -0,0 +1,45 @@
+//===--- LazyDetector.h - Lazy ToolChain Detection --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
+
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+#include 
+
+namespace clang {
+
+/// Simple wrapper for toolchain detector with costly initialization. This
+/// delays the creation of the actual detector until its first usage.
+
+template  class LazyDetector {
+  const driver::Driver 
+  const llvm::Triple 
+  const llvm::opt::ArgList 
+
+  std::optional Detector;
+
+public:
+  LazyDetector(const driver::Driver , const llvm::Triple ,
+   const llvm::opt::ArgList )
+  : D(D), Triple(Triple), Args(Args) {}
+  T *operator->() {
+if (!Detector)
+  Detector.emplace(D, Triple, Args);
+return &*Detector;
+  }
+  const T *operator->() const {
+return const_cast(
+const_cast(*this).operator->());
+  }
+};
+
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -314,7 +314,7 @@
 
 void HIPAMDToolChain::AddHIPIncludeArgs(const ArgList ,
 ArgStringList ) const {
-  RocmInstallation.AddHIPIncludeArgs(DriverArgs, CC1Args);
+  RocmInstallation->AddHIPIncludeArgs(DriverArgs, CC1Args);
 }
 
 SanitizerMask HIPAMDToolChain::getSupportedSanitizers() const {
@@ -343,7 +343,7 @@
   ArgStringList LibraryPaths;
 
   // Find in --hip-device-lib-path and HIP_LIBRARY_PATH.
-  for (StringRef Path : RocmInstallation.getRocmDeviceLibPathArg())
+  for (StringRef Path : RocmInstallation->getRocmDeviceLibPathArg())
 LibraryPaths.push_back(DriverArgs.MakeArgString(Path));
 
   addDirectoryList(DriverArgs, LibraryPaths, "", "HIP_DEVICE_LIB_PATH");
@@ -365,7 +365,7 @@
   getDriver().Diag(diag::err_drv_no_such_file) << BCName;
 });
   } else {
-if (!RocmInstallation.hasDeviceLibrary()) {
+if (!RocmInstallation->hasDeviceLibrary()) {
   getDriver().Diag(diag::err_drv_no_rocm_device_lib) << 0;
   return {};
 }
@@ -376,7 +376,7 @@
 if 

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

LGTM




Comment at: clang/lib/Driver/ToolChains/LazyDetector.h:36
+}
+return ();
+  }

tbaeder wrote:
> Is the `.value()` here permitted? AFAIK the convention is to always use 
> `operator*`.
> 
> And I think @aaron.ballman would tell you to drop the `{}` around that 
> single-statement `if` :)
llvm style prefers `const T` to `T const`


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

https://reviews.llvm.org/D142606

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 494927.
serge-sans-paille added a comment.

Address @tbader's comment


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

https://reviews.llvm.org/D142606

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/lib/Driver/ToolChains/LazyDetector.h
  clang/lib/Driver/ToolChains/Linux.cpp

Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -681,23 +681,23 @@
 
 void Linux::AddCudaIncludeArgs(const ArgList ,
ArgStringList ) const {
-  CudaInstallation.AddCudaIncludeArgs(DriverArgs, CC1Args);
+  CudaInstallation->AddCudaIncludeArgs(DriverArgs, CC1Args);
 }
 
 void Linux::AddHIPIncludeArgs(const ArgList ,
   ArgStringList ) const {
-  RocmInstallation.AddHIPIncludeArgs(DriverArgs, CC1Args);
+  RocmInstallation->AddHIPIncludeArgs(DriverArgs, CC1Args);
 }
 
 void Linux::AddHIPRuntimeLibArgs(const ArgList ,
  ArgStringList ) const {
   CmdArgs.push_back(
-  Args.MakeArgString(StringRef("-L") + RocmInstallation.getLibPath()));
+  Args.MakeArgString(StringRef("-L") + RocmInstallation->getLibPath()));
 
   if (Args.hasFlag(options::OPT_offload_add_rpath,
options::OPT_no_offload_add_rpath, false))
 CmdArgs.append(
-{"-rpath", Args.MakeArgString(RocmInstallation.getLibPath())});
+{"-rpath", Args.MakeArgString(RocmInstallation->getLibPath())});
 
   CmdArgs.push_back("-lamdhip64");
 }
Index: clang/lib/Driver/ToolChains/LazyDetector.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/LazyDetector.h
@@ -0,0 +1,45 @@
+//===--- LazyDetector.h - Lazy ToolChain Detection --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
+
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+#include 
+
+namespace clang {
+
+/// Simple wrapper for toolchain detector with costly initialization. This
+/// delays the creation of the actual detector until its first usage.
+
+template  class LazyDetector {
+  const driver::Driver 
+  const llvm::Triple 
+  const llvm::opt::ArgList 
+
+  std::optional Detector;
+
+public:
+  LazyDetector(const driver::Driver , const llvm::Triple ,
+   const llvm::opt::ArgList )
+  : D(D), Triple(Triple), Args(Args) {}
+  T *operator->() {
+if (!Detector)
+  Detector.emplace(D, Triple, Args);
+return &*Detector;
+  }
+  T const *operator->() const {
+return const_cast(
+const_cast(*this).operator->());
+  }
+};
+
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -315,7 +315,7 @@
 
 void HIPAMDToolChain::AddHIPIncludeArgs(const ArgList ,
 ArgStringList ) const {
-  RocmInstallation.AddHIPIncludeArgs(DriverArgs, CC1Args);
+  RocmInstallation->AddHIPIncludeArgs(DriverArgs, CC1Args);
 }
 
 SanitizerMask HIPAMDToolChain::getSupportedSanitizers() const {
@@ -344,7 +344,7 @@
   ArgStringList LibraryPaths;
 
   // Find in --hip-device-lib-path and HIP_LIBRARY_PATH.
-  for (StringRef Path : RocmInstallation.getRocmDeviceLibPathArg())
+  for (StringRef Path : RocmInstallation->getRocmDeviceLibPathArg())
 LibraryPaths.push_back(DriverArgs.MakeArgString(Path));
 
   addDirectoryList(DriverArgs, LibraryPaths, "", "HIP_DEVICE_LIB_PATH");
@@ -366,7 +366,7 @@
   getDriver().Diag(diag::err_drv_no_such_file) << BCName;
 });
   } else {
-if (!RocmInstallation.hasDeviceLibrary()) {
+if (!RocmInstallation->hasDeviceLibrary()) {
   getDriver().Diag(diag::err_drv_no_rocm_device_lib) << 0;
   return {};
 }
@@ -377,7 +377,7 @@
 if (DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
options::OPT_fno_gpu_sanitize, true) &&
 getSanitizerArgs(DriverArgs).needsAsanRt()) {
-  auto AsanRTL = RocmInstallation.getAsanRTLPath();
+  auto AsanRTL = RocmInstallation->getAsanRTLPath();
   if (AsanRTL.empty()) {
 unsigned DiagID = 

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/Driver/ToolChains/LazyDetector.h:36
+}
+return ();
+  }

Is the `.value()` here permitted? AFAIK the convention is to always use 
`operator*`.

And I think @aaron.ballman would tell you to drop the `{}` around that 
single-statement `if` :)


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

https://reviews.llvm.org/D142606

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: clang/lib/Driver/ToolChains/LazyDetector.h:26
+
+  mutable std::optional Detector;
+

zero9178 wrote:
> serge-sans-paille wrote:
> > zero9178 wrote:
> > > Any reason this is `mutable`? I don't see it being used a non-const way 
> > > in a const method
> > yeah, `T const *operator->() const` updates the optional by actually 
> > creating the value. (`std::optional::empalce` is not const)
> Your `T const *operator->() const` method does not do so, `T *operator->()` 
> does, which is non-const and hence can call `emplace` without issues. 
> So as far as C++ goes it is not required. 
gotcha :-)


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

https://reviews.llvm.org/D142606

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 494877.
serge-sans-paille added a comment.

remove mutable qualifier


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

https://reviews.llvm.org/D142606

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/lib/Driver/ToolChains/LazyDetector.h
  clang/lib/Driver/ToolChains/Linux.cpp

Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -681,23 +681,23 @@
 
 void Linux::AddCudaIncludeArgs(const ArgList ,
ArgStringList ) const {
-  CudaInstallation.AddCudaIncludeArgs(DriverArgs, CC1Args);
+  CudaInstallation->AddCudaIncludeArgs(DriverArgs, CC1Args);
 }
 
 void Linux::AddHIPIncludeArgs(const ArgList ,
   ArgStringList ) const {
-  RocmInstallation.AddHIPIncludeArgs(DriverArgs, CC1Args);
+  RocmInstallation->AddHIPIncludeArgs(DriverArgs, CC1Args);
 }
 
 void Linux::AddHIPRuntimeLibArgs(const ArgList ,
  ArgStringList ) const {
   CmdArgs.push_back(
-  Args.MakeArgString(StringRef("-L") + RocmInstallation.getLibPath()));
+  Args.MakeArgString(StringRef("-L") + RocmInstallation->getLibPath()));
 
   if (Args.hasFlag(options::OPT_offload_add_rpath,
options::OPT_no_offload_add_rpath, false))
 CmdArgs.append(
-{"-rpath", Args.MakeArgString(RocmInstallation.getLibPath())});
+{"-rpath", Args.MakeArgString(RocmInstallation->getLibPath())});
 
   CmdArgs.push_back("-lamdhip64");
 }
Index: clang/lib/Driver/ToolChains/LazyDetector.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/LazyDetector.h
@@ -0,0 +1,46 @@
+//===--- LazyDetector.h - Lazy ToolChain Detection --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
+
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+#include 
+
+namespace clang {
+
+/// Simple wrapper for toolchain detector with costly initialization. This
+/// delays the creation of the actual detector until its first usage.
+
+template  class LazyDetector {
+  const driver::Driver 
+  const llvm::Triple 
+  const llvm::opt::ArgList 
+
+  std::optional Detector;
+
+public:
+  LazyDetector(const driver::Driver , const llvm::Triple ,
+   const llvm::opt::ArgList )
+  : D(D), Triple(Triple), Args(Args) {}
+  T *operator->() {
+if (!Detector) {
+  Detector.emplace(D, Triple, Args);
+}
+return ();
+  }
+  T const *operator->() const {
+return const_cast(
+const_cast(*this).operator->());
+  }
+};
+
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -315,7 +315,7 @@
 
 void HIPAMDToolChain::AddHIPIncludeArgs(const ArgList ,
 ArgStringList ) const {
-  RocmInstallation.AddHIPIncludeArgs(DriverArgs, CC1Args);
+  RocmInstallation->AddHIPIncludeArgs(DriverArgs, CC1Args);
 }
 
 SanitizerMask HIPAMDToolChain::getSupportedSanitizers() const {
@@ -344,7 +344,7 @@
   ArgStringList LibraryPaths;
 
   // Find in --hip-device-lib-path and HIP_LIBRARY_PATH.
-  for (StringRef Path : RocmInstallation.getRocmDeviceLibPathArg())
+  for (StringRef Path : RocmInstallation->getRocmDeviceLibPathArg())
 LibraryPaths.push_back(DriverArgs.MakeArgString(Path));
 
   addDirectoryList(DriverArgs, LibraryPaths, "", "HIP_DEVICE_LIB_PATH");
@@ -366,7 +366,7 @@
   getDriver().Diag(diag::err_drv_no_such_file) << BCName;
 });
   } else {
-if (!RocmInstallation.hasDeviceLibrary()) {
+if (!RocmInstallation->hasDeviceLibrary()) {
   getDriver().Diag(diag::err_drv_no_rocm_device_lib) << 0;
   return {};
 }
@@ -377,7 +377,7 @@
 if (DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
options::OPT_fno_gpu_sanitize, true) &&
 getSanitizerArgs(DriverArgs).needsAsanRt()) {
-  auto AsanRTL = RocmInstallation.getAsanRTLPath();
+  auto AsanRTL = RocmInstallation->getAsanRTLPath();
   if (AsanRTL.empty()) {
 unsigned DiagID = 

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[Markus Böck via Phabricator via cfe-commits]

zero9178 added inline comments.



Comment at: clang/lib/Driver/ToolChains/LazyDetector.h:26
+
+  mutable std::optional Detector;
+

serge-sans-paille wrote:
> zero9178 wrote:
> > Any reason this is `mutable`? I don't see it being used a non-const way in 
> > a const method
> yeah, `T const *operator->() const` updates the optional by actually creating 
> the value. (`std::optional::empalce` is not const)
Your `T const *operator->() const` method does not do so, `T *operator->()` 
does, which is non-const and hence can call `emplace` without issues. 
So as far as C++ goes it is not required. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142606

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: clang/lib/Driver/ToolChains/LazyDetector.h:26
+
+  mutable std::optional Detector;
+

zero9178 wrote:
> Any reason this is `mutable`? I don't see it being used a non-const way in a 
> const method
yeah, `T const *operator->() const` updates the optional by actually creating 
the value. (`std::optional::empalce` is not const)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142606

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[Markus Böck via Phabricator via cfe-commits]

zero9178 added inline comments.



Comment at: clang/lib/Driver/ToolChains/LazyDetector.h:26
+
+  mutable std::optional Detector;
+

Any reason this is `mutable`? I don't see it being used a non-const way in a 
const method


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142606

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D142606#4082783 , 
@serge-sans-paille wrote:

> In D142606#4082753 , @tstellar 
> wrote:
>
>> Does this mean that clang will no longer search for the ROCM and CUDA 
>> library paths for every C compile?
>
> That's the goal, yes.

That's great!  Thanks for working on this.  I think this looks OK, but someone 
with more C++ knowledge than me should probably look over the LazyDetector 
class implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142606

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

In D142606#4082753 , @tstellar wrote:

> Does this mean that clang will no longer search for the ROCM and CUDA library 
> paths for every C compile?

That's the goal, yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142606

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

Does this mean that clang will no longer search for the ROCM and CUDA library 
paths for every C compile?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142606

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
Herald added subscribers: kosarev, kerbowa, jvesely, emaste.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Cuda and rocm toolchain detectors are currently run unconditionally, while 
their result may not be used at all. Make their initialization lazy so that the 
discovery code is not run in common cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142606

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/lib/Driver/ToolChains/LazyDetector.h
  clang/lib/Driver/ToolChains/Linux.cpp

Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -681,23 +681,23 @@
 
 void Linux::AddCudaIncludeArgs(const ArgList ,
ArgStringList ) const {
-  CudaInstallation.AddCudaIncludeArgs(DriverArgs, CC1Args);
+  CudaInstallation->AddCudaIncludeArgs(DriverArgs, CC1Args);
 }
 
 void Linux::AddHIPIncludeArgs(const ArgList ,
   ArgStringList ) const {
-  RocmInstallation.AddHIPIncludeArgs(DriverArgs, CC1Args);
+  RocmInstallation->AddHIPIncludeArgs(DriverArgs, CC1Args);
 }
 
 void Linux::AddHIPRuntimeLibArgs(const ArgList ,
  ArgStringList ) const {
   CmdArgs.push_back(
-  Args.MakeArgString(StringRef("-L") + RocmInstallation.getLibPath()));
+  Args.MakeArgString(StringRef("-L") + RocmInstallation->getLibPath()));
 
   if (Args.hasFlag(options::OPT_offload_add_rpath,
options::OPT_no_offload_add_rpath, false))
 CmdArgs.append(
-{"-rpath", Args.MakeArgString(RocmInstallation.getLibPath())});
+{"-rpath", Args.MakeArgString(RocmInstallation->getLibPath())});
 
   CmdArgs.push_back("-lamdhip64");
 }
Index: clang/lib/Driver/ToolChains/LazyDetector.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/LazyDetector.h
@@ -0,0 +1,46 @@
+//===--- LazyDetector.h - Lazy ToolChain Detection --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
+
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+#include 
+
+namespace clang {
+
+/// Simple wrapper for toolchain detector with costly initialization. This
+/// delays the creation of the actual detector until its first usage.
+
+template  class LazyDetector {
+  const driver::Driver 
+  const llvm::Triple 
+  const llvm::opt::ArgList 
+
+  mutable std::optional Detector;
+
+public:
+  LazyDetector(const driver::Driver , const llvm::Triple ,
+   const llvm::opt::ArgList )
+  : D(D), Triple(Triple), Args(Args) {}
+  T *operator->() {
+if (!Detector) {
+  Detector.emplace(D, Triple, Args);
+}
+return ();
+  }
+  T const *operator->() const {
+return const_cast(
+const_cast(*this).operator->());
+  }
+};
+
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_LAZYDETECTOR_H
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -315,7 +315,7 @@
 
 void HIPAMDToolChain::AddHIPIncludeArgs(const ArgList ,
 ArgStringList ) const {
-  RocmInstallation.AddHIPIncludeArgs(DriverArgs, CC1Args);
+  RocmInstallation->AddHIPIncludeArgs(DriverArgs, CC1Args);
 }
 
 SanitizerMask HIPAMDToolChain::getSupportedSanitizers() const {
@@ -344,7 +344,7 @@
   ArgStringList LibraryPaths;
 
   // Find in --hip-device-lib-path and HIP_LIBRARY_PATH.
-  for (StringRef Path : RocmInstallation.getRocmDeviceLibPathArg())
+  for (StringRef Path : RocmInstallation->getRocmDeviceLibPathArg())
 LibraryPaths.push_back(DriverArgs.MakeArgString(Path));
 
   addDirectoryList(DriverArgs, LibraryPaths, "", "HIP_DEVICE_LIB_PATH");
@@ -366,7 +366,7 @@
   getDriver().Diag(diag::err_drv_no_such_file) << BCName;
 });
   } else {
-if (!RocmInstallation.hasDeviceLibrary()) {
+if (!RocmInstallation->hasDeviceLibrary()) {
   getDriver().Diag(diag::err_drv_no_rocm_device_lib) << 0;
   return {};
 }
@@ -377,7 +377,7 @@
 if