Remove Linux path names in ROCm search paths on Windows

[david-salinas]

  When target triple indicates we are on Windows, do not add linux
  paths to search path in ROCm Installation Detection.

Change-Id: I18effb8c20252de3d84ea37ef562124695c5a570

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-amdgpu

Author: David Salinas (david-salinas)

Changes

  When target triple indicates we are on Windows, do not add linux
  paths to search path in ROCm Installation Detection.

Change-Id: I18effb8c20252de3d84ea37ef562124695c5a570

---

Full diff: https://github.com/llvm/llvm-project/pull/97668.diff

2 Files Affected:

• (modified) clang/lib/Driver/ToolChains/AMDGPU.cpp (+10-8)
• (modified) clang/lib/Driver/ToolChains/ROCm.h (+5)

diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index 453daed7cc7d5..e05b9202599c7 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -306,14 +306,16 @@ RocmInstallationDetector::getInstallationPathCandidates() {
       LatestVer = Ver;
     }
   }
-  if (!LatestROCm.empty())
-    ROCmSearchDirs.emplace_back(D.SysRoot + "/opt/" + LatestROCm,
-                                /*StrictChecking=*/true);
+  if (!isWindows()) {
+    if (!LatestROCm.empty())
+      ROCmSearchDirs.emplace_back(D.SysRoot + "/opt/" + LatestROCm,
+                                  /*StrictChecking=*/true);
 
-  ROCmSearchDirs.emplace_back(D.SysRoot + "/usr/local",
-                              /*StrictChecking=*/true);
-  ROCmSearchDirs.emplace_back(D.SysRoot + "/usr",
-                              /*StrictChecking=*/true);
+    ROCmSearchDirs.emplace_back(D.SysRoot + "/usr/local",
+                                /*StrictChecking=*/true);
+    ROCmSearchDirs.emplace_back(D.SysRoot + "/usr",
+                                /*StrictChecking=*/true);
+  }
 
   DoPrintROCmSearchDirs();
   return ROCmSearchDirs;
@@ -322,7 +324,7 @@ RocmInstallationDetector::getInstallationPathCandidates() {
 RocmInstallationDetector::RocmInstallationDetector(
     const Driver &D, const llvm::Triple &HostTriple,
     const llvm::opt::ArgList &Args, bool DetectHIPRuntime, bool DetectDeviceLib)
-    : D(D) {
+    : D(D), hostTriple(HostTriple) {
   Verbose = Args.hasArg(options::OPT_v);
   RocmPathArg = Args.getLastArgValue(clang::driver::options::OPT_rocm_path_EQ);
   PrintROCmSearchDirs =
diff --git a/clang/lib/Driver/ToolChains/ROCm.h b/clang/lib/Driver/ToolChains/ROCm.h
index dceb0ab036693..e2492b1630238 100644
--- a/clang/lib/Driver/ToolChains/ROCm.h
+++ b/clang/lib/Driver/ToolChains/ROCm.h
@@ -111,6 +111,8 @@ class RocmInstallationDetector {
   // Wheter -nogpulib is specified.
   bool NoBuiltinLibs = false;
 
+  llvm::Triple hostTriple;
+
   // Paths
   SmallString<0> InstallPath;
   SmallString<0> BinPath;
@@ -193,6 +195,9 @@ class RocmInstallationDetector {
   /// Check whether we detected a valid HIP STDPAR Acceleration library.
   bool hasHIPStdParLibrary() const { return HasHIPStdParLibrary; }
 
+  /// Check whether the target triple is for Windows.
+  bool isWindows() const { return hostTriple.isOSWindows();}
+
   /// Print information about the detected ROCm installation.
   void print(raw_ostream &OS) const;
 

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 7b9f988a535c3549b71025e951e3a36a2bf0fa03 09b7865558776b565f7fff0a8d534b5a77bb6252 --extensions cpp,h -- clang/lib/Driver/ToolChains/AMDGPU.cpp clang/lib/Driver/ToolChains/AMDGPU.h clang/lib/Driver/ToolChains/HIPAMD.cpp clang/lib/Driver/ToolChains/MSVC.cpp clang/lib/Driver/ToolChains/ROCm.h

View the diff from clang-format here.

diff --git a/clang/lib/Driver/ToolChains/AMDGPU.h b/clang/lib/Driver/ToolChains/AMDGPU.h
index 73409a725a..b95fbedec7 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.h
+++ b/clang/lib/Driver/ToolChains/AMDGPU.h
@@ -135,7 +135,7 @@ protected:
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
 public:
   ROCMToolChain(const Driver &D, const llvm::Triple &Triple,
-                const llvm::opt::ArgList &Args, bool isHostTCMSVC=false);
+                const llvm::opt::ArgList &Args, bool isHostTCMSVC = false);
   void
   addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
                         llvm::opt::ArgStringList &CC1Args,
diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp b/clang/lib/Driver/ToolChains/HIPAMD.cpp
index 4c2ca949f8..f338fca09f 100644
--- a/clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -213,7 +213,9 @@ void AMDGCN::Linker::ConstructJob(Compilation &C, const JobAction &JA,
 
 HIPAMDToolChain::HIPAMDToolChain(const Driver &D, const llvm::Triple &Triple,
                                  const ToolChain &HostTC, const ArgList &Args)
-    : ROCMToolChain(D, Triple, Args, HostTC.getTriple().isWindowsMSVCEnvironment()), HostTC(HostTC) {
+    : ROCMToolChain(D, Triple, Args,
+                    HostTC.getTriple().isWindowsMSVCEnvironment()),
+      HostTC(HostTC) {
   // Lookup binaries into the driver directory, this is used to
   // discover the clang-offload-bundler executable.
   getProgramPaths().push_back(getDriver().Dir);
diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp
index b683682ad3..2929747758 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -425,7 +425,7 @@ MSVCToolChain::MSVCToolChain(const Driver &D, const llvm::Triple &Triple,
                              const ArgList &Args)
     : ToolChain(D, Triple, Args), CudaInstallation(D, Triple, Args),
       RocmInstallation(D, Triple, Args) {
-  
+
   RocmInstallation->setHostWindows(true);
 
   getProgramPaths().push_back(getDriver().Dir);
@@ -453,7 +453,6 @@ MSVCToolChain::MSVCToolChain(const Driver &D, const llvm::Triple &Triple,
       llvm::findVCToolChainViaSetupConfig(getVFS(), VCToolsVersion,
                                           VCToolChainPath, VSLayout) ||
       llvm::findVCToolChainViaRegistry(VCToolChainPath, VSLayout);
-
 }
 
 Tool *MSVCToolChain::buildLinker() const {
diff --git a/clang/lib/Driver/ToolChains/ROCm.h b/clang/lib/Driver/ToolChains/ROCm.h
index d766422396..4024ec564d 100644
--- a/clang/lib/Driver/ToolChains/ROCm.h
+++ b/clang/lib/Driver/ToolChains/ROCm.h
@@ -198,7 +198,7 @@ public:
 
   /// Check whether the target triple is for Windows.
   bool isHostWindows() const { return IsHostMSVC; }
-  void setHostWindows(bool val) { IsHostMSVC=val; }
+  void setHostWindows(bool val) { IsHostMSVC = val; }
 
   /// Print information about the detected ROCm installation.
   void print(raw_ostream &OS) const;

[yxsamliu]

better add a lit test like https://github.com/llvm/llvm-project/blob/main/clang/test/Driver/rocm-detect.hip , but for windows only (REQUIRES: system-windows), using --print-rocm-search-dirs, and checks ROCm installation search path.

[jayfoad]

Compiler messages on HIP SDK for Windows

Please rewrite this to say what the patch does or what problem it fixes.

[david-salinas]

Compiler messages on HIP SDK for Windows

Please rewrite this to say what the patch does or what problem it fixes.

Changed the PR (and commit) subject to something more meaningful/accurate.