[Comgr] Remove HIP_PATH and ROCM_PATH logic, simplify LLVM_PATH (llvm#99)

Previously, a significant amount of code/logic in comgr-env.cpp was
dedicated to "guessing" the correct ROCM, HIP, and LLVM system paths for
HIP/OpenCL compilations.

However, these paths can change with restructuring to the ROCm directory
layout. As is, in many configurations the paths are currently incorrect.
Also, many of the includes/etc. the paths were used for have been
removed.

Author: lamb-j

amd/comgr/README.md

@@ -108,22 +108,11 @@ required. If the value is used, it is read once at the time it is needed, and
 then cached. The exact behavior when changing these values during the execution
 of a process after Comgr APIs have been invoked is undefined.
 
-Comgr supports some environment variables to locate parts of the ROCm stack.
-These include:
-
-* `ROCM_PATH`: If this is set it is used as an absolute path to the root of the
-  ROCm installation, which is used when determining the default values for
-  `HIP_PATH` and `LLVM_PATH` (see below). If this is not set and if a ROCM
-  package is provided during the build, ROCM path is set to it. Otherwise Comgr
-  tries to construct the ROCM path from the path where amd_comgr library
-  is located.
-* `HIP_PATH`: If this is set it is used as an absolute path to the root of the
-  HIP installation. If this is not set, it has a default value of
-  "${ROCM_PATH}/hip".
-* `LLVM_PATH`: If this is set it is used as an absolute path to the root of the
-  LLVM installation, which is currently used for HIP compilation to locate
-  certain runtime headers. If this is not set, it has a default value of
-  "${ROCM_PATH}/llvm".
+Comgr supports an environment variable to help locate LLVM:
+
+* `LLVM_PATH`: If set, it is used as an absolute path to the root of the LLVM
+  installation, which is currently used to locate the clang resource directory
+  and clang binary path, allowing for additional optimizations.
 
 Comgr also supports some environment variables to aid in debugging. These
 include:

amd/comgr/docs/ReleaseNotes.md

@@ -213,6 +213,8 @@ directly relying on HIP
 validate generated artifacts with command-line tools like llvm-dis,
 llvm-objdump, etc. Moving forward, most new Comgr tests should be written as
 lit tests, and tests in comgr/test should be transitioned to comgr/test-lit.
+- Removed HIP\_PATH and ROCM\_PATH environment variables. These were used for
+now-removed Comgr actions, such as \*COMPILE\_SOURCE\_TO\_FATBIN.
 
 New Targets
 -----------

amd/comgr/src/comgr-compiler.cpp

@@ -1001,10 +1001,6 @@ AMDGPUCompiler::addTargetIdentifierFlags(llvm::StringRef IdentStr,
 }
 
 amd_comgr_status_t AMDGPUCompiler::addCompilationFlags() {
-  HIPIncludePath = (Twine(env::getHIPPath()) + "/include").str();
-  // HIP headers depend on hsa.h which is in ROCM_DIR/include.
-  ROCMIncludePath = (Twine(env::getROCMPath()) + "/include").str();
-
   // Default to O3 for all contexts
   Args.push_back("-O3");
 
@@ -1027,10 +1023,6 @@ amd_comgr_status_t AMDGPUCompiler::addCompilationFlags() {
   case AMD_COMGR_LANGUAGE_HIP:
     Args.push_back("hip");
     Args.push_back("--offload-device-only");
-    Args.push_back("-isystem");
-    Args.push_back(ROCMIncludePath.c_str());
-    Args.push_back("-isystem");
-    Args.push_back(HIPIncludePath.c_str());
     break;
   default:
     return AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT;

amd/comgr/src/comgr-compiler.h

@@ -52,12 +52,6 @@ class AMDGPUCompiler {
   DataAction *ActionInfo;
   DataSet *InSet;
   amd_comgr_data_set_t OutSetT;
-  /// ROCM include Path
-  std::string ROCMIncludePath;
-  /// HIP and Clang Include Paths
-  std::string HIPIncludePath;
-  std::string ClangIncludePath;
-  std::string ClangIncludePath2;
   /// Precompiled header file paths.
   llvm::SmallVector<llvm::SmallString<128>, 2> PrecompiledHeaders;
   /// Arguments common to all driver invocations in the current action.

amd/comgr/src/comgr-env.cpp

@@ -69,214 +69,10 @@ bool shouldEmitVerboseLogs() {
   return VerboseLogs && StringRef(VerboseLogs) != "0";
 }
 
-StringRef StripGNUInstallLibDir(StringRef Path) {
-  // Comgr library may be installed under lib or lib64 or
-  // lib/<multiarch-tuple> on Debian.
-  StringRef ParentDir = sys::path::parent_path(Path);
-  StringRef ParentName = sys::path::filename(ParentDir);
-
-  StringRef SecondLevelParentDir = sys::path::parent_path(ParentDir);
-  StringRef SecondLevelParentName = sys::path::filename(SecondLevelParentDir);
-
-  if (ParentName == "lib" || ParentName == "lib64") {
-    ParentDir = sys::path::parent_path(ParentDir);
-  } else if (SecondLevelParentName == "lib") {
-    ParentDir = sys::path::parent_path(SecondLevelParentDir);
-  }
-
-  return ParentDir;
-}
-
-std::string getComgrInstallPathFromExecutable() {
-
-#if !defined(_WIN32) && !defined(_WIN64)
-  FILE *ProcMaps = fopen("/proc/self/maps", "r");
-  if (ProcMaps == NULL)
-    return "";
-
-  char *Line = NULL;
-  size_t len = 0;
-  uintptr_t Address = reinterpret_cast<uintptr_t>(getROCMPath);
-
-  // TODO: switch POSIX getline() to C++-based getline() once Pytorch resolves
-  // build issues with libstdc++ ABI
-  while (getline(&Line, &len, ProcMaps) != -1) {
-    SmallVector<StringRef, 6> Tokens;
-    StringRef(Line).split(Tokens, ' ', -1 /* MaxSplit */,
-                          false /* KeepEmpty */);
-
-    unsigned long long LowAddress, HighAddress;
-    if (consumeUnsignedInteger(Tokens[0], 16 /* Radix */, LowAddress)) {
-      fclose(ProcMaps);
-      free(Line);
-      return "";
-    }
-
-    if (!Tokens[0].consume_front("-")) {
-      fclose(ProcMaps);
-      free(Line);
-      return "";
-    }
-
-    if (consumeUnsignedInteger(Tokens[0], 16 /* Radix */, HighAddress)) {
-      fclose(ProcMaps);
-      free(Line);
-      return "";
-    }
-
-    if ((Address >= LowAddress && Address <= HighAddress)) {
-      StringRef Path = Tokens[5].ltrim();
-      /* Not a mapped file or File path empty */
-      if (Tokens[4] == "0" || Path == "") {
-        fclose(ProcMaps);
-        free(Line);
-        return "";
-      }
-
-      std::string rv = StripGNUInstallLibDir(Path).str();
-      fclose(ProcMaps);
-      free(Line);
-      return rv;
-    }
-  }
-
-  fclose(ProcMaps);
-  free(Line);
-#endif
-
-  return "";
+llvm::StringRef getLLVMPath() {
+  static const char *EnvLLVMPath = std::getenv("LLVM_PATH");
+  return EnvLLVMPath;
 }
 
-class InstallationDetector {
-public:
-  InstallationDetector(StringRef ROCmPath, bool isComgrPath)
-      : ROCmInstallPath(ROCmPath) {}
-  virtual ~InstallationDetector() = default;
-
-  const StringRef getROCmPath() const { return ROCmInstallPath; }
-  void setROCmInstallPath(StringRef Path) { ROCmInstallPath = Path; }
-
-  virtual SmallString<128> getLLVMPathImpl() {
-    SmallString<128> LLVMPath = getROCmPath();
-    sys::path::append(LLVMPath, "llvm");
-
-    return LLVMPath;
-  }
-
-  virtual SmallString<128> getHIPPathImpl() {
-    SmallString<128> HIPPath = getROCmPath();
-    sys::path::append(HIPPath, "hip");
-
-    return HIPPath;
-  }
-
-  StringRef getLLVMPath() {
-    static const char *EnvLLVMPath = std::getenv("LLVM_PATH");
-    if (EnvLLVMPath) {
-      return EnvLLVMPath;
-    }
-
-    if (LLVMInstallationPath.empty()) {
-      LLVMInstallationPath = getLLVMPathImpl();
-    }
-
-    return LLVMInstallationPath;
-  }
-
-  StringRef getHIPPath() {
-    static const char *EnvHIPPath = std::getenv("HIP_PATH");
-    if (EnvHIPPath) {
-      return EnvHIPPath;
-    }
-
-    if (HIPInstallationPath.empty()) {
-      HIPInstallationPath = getHIPPathImpl();
-    }
-
-    return HIPInstallationPath;
-  }
-
-  SmallString<128> getSiblingDirWithPrefix(StringRef DirName,
-                                           StringRef Prefix) {
-    StringRef ParentDir = sys::path::parent_path(DirName);
-    std::error_code EC;
-
-    for (sys::fs::directory_iterator Dir(ParentDir, EC), DirEnd;
-         Dir != DirEnd && !EC; Dir.increment(EC)) {
-      const StringRef Path = sys::path::filename(Dir->path());
-      if (Path.starts_with(Prefix)) {
-        return StringRef(Dir->path());
-      }
-    }
-
-    return SmallString<128>();
-  }
-
-private:
-  SmallString<128> ROCmInstallPath;
-  SmallString<128> HIPInstallationPath;
-  SmallString<128> LLVMInstallationPath;
-};
-
-// If the ROCmInstallPath is Spack based it should be in the format
-// rocm-cmake-${rocm-version}-${hash}. Detect corresponding LLVM and HIP
-// Paths existing at the same level at ROCM. It should be in the format
-// llvm-amdgpu-${rocm-version}-${hash} and hip-${rocm-version}-${hash}.
-class SpackInstallationDetector : public InstallationDetector {
-public:
-  SpackInstallationDetector(StringRef Path, bool isComgrPath)
-      : InstallationDetector(Path, isComgrPath) {
-    if (isComgrPath) {
-      auto ROCmInstallPath = getSiblingDirWithPrefix(Path, "rocm-cmake-");
-      setROCmInstallPath(ROCmInstallPath);
-    }
-  }
-
-  virtual SmallString<128> getLLVMPathImpl() override {
-    return getSiblingDirWithPrefix(getROCmPath(), "llvm-amdgpu-");
-  }
-
-  virtual SmallString<128> getHIPPathImpl() override {
-    return getSiblingDirWithPrefix(getROCmPath(), "hip-");
-  }
-};
-
-std::shared_ptr<InstallationDetector>
-CreatePathDetector(StringRef Path, bool isComgrPath = false) {
-  StringRef DirName = sys::path::filename(Path);
-  if ((!isComgrPath && DirName.starts_with("rocm-cmake-")) ||
-      (isComgrPath && DirName.starts_with("comgr-"))) {
-    return std::make_shared<SpackInstallationDetector>(Path, isComgrPath);
-  }
-
-  return std::make_shared<InstallationDetector>(Path, isComgrPath);
-}
-
-std::shared_ptr<InstallationDetector> getDetectorImpl() {
-  SmallString<128> ROCmInstallPath;
-
-  static const char *EnvROCMPath = std::getenv("ROCM_PATH");
-  if (EnvROCMPath) {
-    ROCmInstallPath = EnvROCMPath;
-  }
-
-  if (ROCmInstallPath == "") {
-    std::string ComgrInstallationPath = getComgrInstallPathFromExecutable();
-    return CreatePathDetector(ComgrInstallationPath, true /* isComgrPath */);
-  }
-  return CreatePathDetector(ROCmInstallPath);
-}
-
-InstallationDetector *getDetector() {
-  static auto Detector = getDetectorImpl();
-  return Detector.get();
-}
-
-StringRef getROCMPath() { return getDetector()->getROCmPath(); }
-
-StringRef getHIPPath() { return getDetector()->getHIPPath(); }
-
-StringRef getLLVMPath() { return getDetector()->getLLVMPath(); }
-
 } // namespace env
 } // namespace COMGR

amd/comgr/src/comgr-env.h

@@ -54,14 +54,6 @@ bool shouldEmitVerboseLogs();
 /// Return whether the environment requests time statistics collection.
 bool needTimeStatistics();
 
-/// If environment variable ROCM_PATH is set, return the environment varaible,
-/// otherwise return the default ROCM path.
-llvm::StringRef getROCMPath();
-
-/// If environment variable HIP_PATH is set, return the environment variable,
-/// otherwise return the default HIP path.
-llvm::StringRef getHIPPath();
-
 /// If environment variable LLVM_PATH is set, return the environment variable,
 /// otherwise return the default LLVM path.
 llvm::StringRef getLLVMPath();