[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

2021-09-16 Thread DineshKumar Bhaskaran via Phabricator via cfe-commits 
dbhaskaran created this revision.
dbhaskaran added reviewers: pdhaliwal, csigg, JonChesterfield.
Herald added subscribers: wenzhicui, wrengr, Chia-hungDuan, dcaballe, cota, 
teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, 
Joonsoo, kerbowa, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, guansong, t-tye, tpr, dstuttard, 
yaxunl, mgorny, nhaehnle, jvesely, kzhuravl.
dbhaskaran requested review of this revision.
Herald added subscribers: openmp-commits, cfe-commits, sstefan1, 
stephenneuendorffer, nicolasvasilache, wdng.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: herhut.
Herald added projects: clang, OpenMP, MLIR.

This avoids undefined behavior/issues resulting out of unconventional
or co-existing ROCm installations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109885

Files:
  clang/tools/amdgpu-arch/CMakeLists.txt
  mlir/lib/Dialect/GPU/CMakeLists.txt
  mlir/lib/ExecutionEngine/CMakeLists.txt
  openmp/libomptarget/plugins/amdgpu/CMakeLists.txt


Index: openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
===
--- openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
+++ openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
@@ -14,7 +14,7 @@
 

 
 # as of rocm-3.7, hsa is installed with cmake packages and kmt is found via hsa
-find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX} PATHS 
/opt/rocm)
+find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX})
 
 if(NOT LIBOMPTARGET_DEP_LIBELF_FOUND)
   libomptarget_say("Not building AMDGPU plugin: LIBELF not found")
Index: mlir/lib/ExecutionEngine/CMakeLists.txt
===
--- mlir/lib/ExecutionEngine/CMakeLists.txt
+++ mlir/lib/ExecutionEngine/CMakeLists.txt
@@ -142,7 +142,7 @@
   # Configure ROCm support.
   if (NOT DEFINED ROCM_PATH)
 if (NOT DEFINED ENV{ROCM_PATH})
-  set(ROCM_PATH "/opt/rocm" CACHE PATH "Path to which ROCm has been 
installed")
+  message(SEND_ERROR "Building mlir with ROCm support requires a working 
ROCm")
 else()
   set(ROCM_PATH $ENV{ROCM_PATH} CACHE PATH "Path to which ROCm has been 
installed")
 endif()
Index: mlir/lib/Dialect/GPU/CMakeLists.txt
===
--- mlir/lib/Dialect/GPU/CMakeLists.txt
+++ mlir/lib/Dialect/GPU/CMakeLists.txt
@@ -127,7 +127,7 @@
   # Configure ROCm support.
   if (NOT DEFINED ROCM_PATH)
 if (NOT DEFINED ENV{ROCM_PATH})
-  set(ROCM_PATH "/opt/rocm" CACHE PATH "Path to which ROCm has been 
installed")
+  message(SEND_ERROR "Building mlir with ROCm support requires a working 
ROCm")
 else()
   set(ROCM_PATH $ENV{ROCM_PATH} CACHE PATH "Path to which ROCm has been 
installed")
 endif()
Index: clang/tools/amdgpu-arch/CMakeLists.txt
===
--- clang/tools/amdgpu-arch/CMakeLists.txt
+++ clang/tools/amdgpu-arch/CMakeLists.txt
@@ -6,7 +6,7 @@
 # //
 # 
//===--===//
 
-find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX} PATHS 
/opt/rocm)
+find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX})
 if (NOT ${hsa-runtime64_FOUND})
   message(STATUS "Not building amdgpu-arch: hsa-runtime64 not found")
   return()


Index: openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
===
--- openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
+++ openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
@@ -14,7 +14,7 @@
 
 
 # as of rocm-3.7, hsa is installed with cmake packages and kmt is found via hsa
-find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX} PATHS /opt/rocm)
+find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX})
 
 if(NOT LIBOMPTARGET_DEP_LIBELF_FOUND)
   libomptarget_say("Not building AMDGPU plugin: LIBELF not found")
Index: mlir/lib/ExecutionEngine/CMakeLists.txt
===
--- mlir/lib/ExecutionEngine/CMakeLists.txt
+++ mlir/lib/ExecutionEngine/CMakeLists.txt
@@ -142,7 +142,7 @@
   # Configure ROCm support.
   if (NOT DEFINED ROCM_PATH)
 if (NOT DEFINED ENV{ROCM_PATH})
-  set(ROCM_PATH "/opt/rocm" CACHE PATH "Path to which ROCm has been installed")
+  message(SEND_ERROR "Building mlir with ROCm support requires a working ROCm")
 else()
   set(ROCM_PATH $ENV{ROCM_PATH} CACHE PATH "Path to which ROCm has been installed")
 endif()
Index: mlir/lib/Dialect/GPU/CMakeLists.txt
===
--- mlir/lib/Dialect/GPU/CMakeLists.txt
+++ mlir/lib/Di

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

2021-11-08 Thread DineshKumar Bhaskaran via Phabricator via cfe-commits 
dbhaskaran added a comment.

In D109885#3004269 , @JonChesterfield 
wrote:

> Dropping PATHS /opt/rocm from the openmp parts will break people using an 
> existing rocm install with trunk llvm. I think it would be reasonable to look 
> at whatever ROCM_PATH is set to instead of assuming /opt/rocm. Is that a 
> variable we can pass to find_package?

Added ROCM_PATH environment variable.




Comment at: mlir/lib/Dialect/GPU/CMakeLists.txt:137
   set(CMAKE_MODULE_PATH "${HIP_PATH}/cmake" ${CMAKE_MODULE_PATH})
   find_package(HIP)
   if (NOT HIP_FOUND)

ye-luo wrote:
> Both ROCM_PATH HIP_PATH are used as hints without verification.
> But they are used later for generating include paths. Overall logic is broken.
> 
> if ROCM_PATH takes the precedence over everything else
> You can do this
> if ROCM_PATH defined
> find_path(
>   HIP_MODULE_FILE_DIR FindHIP.cmake
>   HINTS ${ROCM_PATH}
>   PATH_SUFFIXES hip/cmake REQUIRED
>   NO_DEFAULT_PATH)
> else
> find_path(
>   HIP_MODULE_FILE_DIR FindHIP.cmake
>   HINTS $ENV{ROCM_PATH} /opt/rocm
>   PATH_SUFFIXES hip/cmake REQUIRED)
> endif
> 
> by doing this, you can verify that ROCM_PATH is correct if provided and the 
> path the hip module file has been verified. then it is safe to do
> set(CMAKE_MODULE_PATH "${HIP_MODULE_FILE_DIR}" ${CMAKE_MODULE_PATH})
> find_package(HIP)
The primary intention here is to avoid fallback to a default ROCM path  to 
avoid unwanted issues related to multiple installation, however I agree the we 
should use paths with verification so I have accommodated the changes for HIP 
excluding the hint. Thanks for the code snippets. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

2021-11-08 Thread DineshKumar Bhaskaran via Phabricator via cfe-commits 
dbhaskaran updated this revision to Diff 385398.
dbhaskaran added a comment.

Updated with review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

Files:
  clang/tools/amdgpu-arch/CMakeLists.txt
  mlir/lib/Dialect/GPU/CMakeLists.txt
  mlir/lib/ExecutionEngine/CMakeLists.txt
  openmp/libomptarget/plugins/amdgpu/CMakeLists.txt

Index: openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
===
--- openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
+++ openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
@@ -14,7 +14,11 @@
 
 
 # as of rocm-3.7, hsa is installed with cmake packages and kmt is found via hsa
-find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX} PATHS /opt/rocm)
+if (DEFINED ENV{ROCM_PATH})
+  set(ROCM_PATH $ENV{ROCM_PATH} CACHE PATH "Path to which ROCm has been installed")
+endif()
+
+find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX} PATH ${ROCM_PATH})
 
 if(NOT LIBOMPTARGET_DEP_LIBELF_FOUND)
   libomptarget_say("Not building AMDGPU plugin: LIBELF not found")
Index: mlir/lib/ExecutionEngine/CMakeLists.txt
===
--- mlir/lib/ExecutionEngine/CMakeLists.txt
+++ mlir/lib/ExecutionEngine/CMakeLists.txt
@@ -142,13 +142,17 @@
   # Configure ROCm support.
   if (NOT DEFINED ROCM_PATH)
 if (NOT DEFINED ENV{ROCM_PATH})
-  set(ROCM_PATH "/opt/rocm" CACHE PATH "Path to which ROCm has been installed")
+  message(SEND_ERROR "Building mlir with ROCm support requires a working ROCm")
 else()
   set(ROCM_PATH $ENV{ROCM_PATH} CACHE PATH "Path to which ROCm has been installed")
 endif()
-set(HIP_PATH "${ROCM_PATH}/hip" CACHE PATH "Path to which HIP has been installed")
   endif()
-  set(CMAKE_MODULE_PATH "${HIP_PATH}/cmake" ${CMAKE_MODULE_PATH})
+
+  find_path(HIP_MODULE_FILE_DIR FindHIP.cmake
+HINTS ${ROCM_PATH}
+PATH_SUFFIXES hip/cmake REQUIRED
+NO_DEFAULT_PATH)
+  set(CMAKE_MODULE_PATH "${HIP_MODULE_FILE_DIR}" ${CMAKE_MODULE_PATH})
   find_package(HIP)
   if (NOT HIP_FOUND)
 message(SEND_ERROR "Building mlir with ROCm support requires a working ROCm and HIP install")
@@ -158,7 +162,8 @@
 
   # Locate HIP runtime library.
   find_library(ROCM_RUNTIME_LIBRARY amdhip64
-   PATHS "${HIP_PATH}/lib")
+  PATHS "${ROCM_PATH}"
+  PATH_SUFFIXES hip/lib)
   if (NOT ROCM_RUNTIME_LIBRARY)
 message(SEND_ERROR "Could not locate ROCm HIP runtime library")
   else()
@@ -177,7 +182,6 @@
   )
   target_include_directories(mlir_rocm_runtime
 PRIVATE
-${HIP_PATH}/include
 ${ROCM_PATH}/include
   )
   target_link_libraries(mlir_rocm_runtime
Index: mlir/lib/Dialect/GPU/CMakeLists.txt
===
--- mlir/lib/Dialect/GPU/CMakeLists.txt
+++ mlir/lib/Dialect/GPU/CMakeLists.txt
@@ -130,13 +130,17 @@
   # Configure ROCm support.
   if (NOT DEFINED ROCM_PATH)
 if (NOT DEFINED ENV{ROCM_PATH})
-  set(ROCM_PATH "/opt/rocm" CACHE PATH "Path to which ROCm has been installed")
+  message(SEND_ERROR "Building mlir with ROCm support requires a working ROCm")
 else()
   set(ROCM_PATH $ENV{ROCM_PATH} CACHE PATH "Path to which ROCm has been installed")
 endif()
-set(HIP_PATH "${ROCM_PATH}/hip" CACHE PATH " Path to which HIP has been installed")
   endif()
-  set(CMAKE_MODULE_PATH "${HIP_PATH}/cmake" ${CMAKE_MODULE_PATH})
+
+  find_path(HIP_MODULE_FILE_DIR FindHIP.cmake
+HINTS ${ROCM_PATH}
+PATH_SUFFIXES hip/cmake REQUIRED
+NO_DEFAULT_PATH)
+  set(CMAKE_MODULE_PATH "${HIP_MODULE_FILE_DIR}" ${CMAKE_MODULE_PATH})
   find_package(HIP)
   if (NOT HIP_FOUND)
 message(SEND_ERROR "Building mlir with ROCm support requires a working ROCm and HIP install")
@@ -154,7 +158,6 @@
   target_include_directories(obj.MLIRGPUOps
 PRIVATE
 ${MLIR_SOURCE_DIR}/../lld/include
-${HIP_PATH}/include
 ${ROCM_PATH}/include
   )
 
Index: clang/tools/amdgpu-arch/CMakeLists.txt
===
--- clang/tools/amdgpu-arch/CMakeLists.txt
+++ clang/tools/amdgpu-arch/CMakeLists.txt
@@ -6,7 +6,11 @@
 # //
 # //===--===//
 
-find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX} PATHS /opt/rocm)
+if (DEFINED ENV{ROCM_PATH})
+  set(ROCM_PATH $ENV{ROCM_PATH} CACHE PATH "Path to which ROCm has been installed")
+endif()
+
+find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX} PATH ${ROCM_PATH})
 if (NOT ${hsa-runtime64_FOUND})
   message(STATUS "Not building amdgpu-arch: hsa-runtime64 not found")
   return()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm