Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SWDEV-345870 - Correct HIP path for new directory layout and removed the usage of HSA_PATH in hipcc #3065

Merged
merged 1 commit into from
Nov 24, 2022
Merged

SWDEV-345870 - Correct HIP path for new directory layout and removed the usage of HSA_PATH in hipcc #3065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 0 additions & 10 deletions bin/hipcc.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@
# Other environment variable controls:
# HIP_PATH : Path to HIP directory, default is one dir level above location of this script.
# CUDA_PATH : Path to CUDA SDK (default /usr/local/cuda). Used on NVIDIA platforms only.
# HSA_PATH : Path to HSA dir (defaults to ../../hsa relative to abs_path
# of this script). Used on AMD platforms only.
# HIP_ROCCLR_HOME : Path to HIP/ROCclr directory. Used on AMD platforms only.
# HIP_CLANG_PATH : Path to HIP-Clang (default to ../../llvm/bin relative to this
# script's abs_path). Used on AMD platforms only.
Expand Down Expand Up @@ -122,7 +120,6 @@ BEGIN
$HIP_PATH = $hipvars::HIP_PATH;
$ROCM_PATH = $hipvars::ROCM_PATH;
$HIP_VERSION = $hipvars::HIP_VERSION;
$HSA_PATH = $hipvars::HSA_PATH;
$HIP_ROCCLR_HOME = $hipvars::HIP_ROCCLR_HOME;

if ($HIP_PLATFORM eq "amd") {
Expand Down Expand Up @@ -210,13 +207,6 @@ BEGIN
## Allow __fp16 as function parameter and return type.
$HIPCXXFLAGS .= " -Xclang -fallow-half-arguments-and-returns -D__HIP_HCC_COMPAT_MODE__=1";
}

if (not $isWindows) {
$HSA_PATH=$ENV{'HSA_PATH'} // "$ROCM_PATH/hsa";
$HIPCXXFLAGS .= " -isystem $HSA_PATH/include";
$HIPCFLAGS .= " -isystem $HSA_PATH/include";
}

} elsif ($HIP_PLATFORM eq "nvidia") {
$CUDA_PATH=$ENV{'CUDA_PATH'} // '/usr/local/cuda';
$HIP_INCLUDE_PATH = "$HIP_PATH/include";
Expand Down
7 changes: 3 additions & 4 deletions bin/hipvars.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,14 @@ $isWindows = ($^O eq 'MSWin32' or $^O eq 'msys');
# ROCM_PATH is defined relative to HIP_PATH else it is hardcoded to /opt/rocm.
#
$HIP_PATH=$ENV{'HIP_PATH'} // dirname(Cwd::abs_path("$0/../")); # use parent directory of hipcc
if (-e "$HIP_PATH/../bin/rocm_agent_enumerator") {
$ROCM_PATH=$ENV{'ROCM_PATH'} // dirname("$HIP_PATH"); # use parent directory of HIP_PATH ,FILE_REORG
}elsif (-e "$HIP_PATH/bin/rocm_agent_enumerator") {
if (-e "$HIP_PATH/bin/rocm_agent_enumerator") {
$ROCM_PATH=$ENV{'ROCM_PATH'} // "$HIP_PATH"; # use HIP_PATH
}elsif (-e "$HIP_PATH/../bin/rocm_agent_enumerator") { # case for backward compatibility
$ROCM_PATH=$ENV{'ROCM_PATH'} // dirname("$HIP_PATH"); # use parent directory of HIP_PATH
} else {
$ROCM_PATH=$ENV{'ROCM_PATH'} // "/opt/rocm";
}
$CUDA_PATH=$ENV{'CUDA_PATH'} // '/usr/local/cuda';
$HSA_PATH=$ENV{'HSA_PATH'} // "$ROCM_PATH/hsa";

# Windows has a different structure, all binaries are inside hip/bin
if ($isWindows) {
Expand Down
15 changes: 6 additions & 9 deletions hip-lang-config.cmake.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,15 @@ find_dependency(amd_comgr)

include( "${CMAKE_CURRENT_LIST_DIR}/hip-lang-targets.cmake" )

# From hip-lang config directory, do three level up
get_filename_component(_DIR "${CMAKE_CURRENT_LIST_DIR}" REALPATH)
get_filename_component(_IMPORT_PREFIX "${_DIR}/../../../" REALPATH)
# Find the hip-lang config file path with symlinks resolved
# RealPath: /opt/rocm-ver/lib/cmake/hip-lang/hip-lang-config.cmake
# Go 4 level up to get IMPORT PREFIX
get_filename_component(_DIR "${CMAKE_CURRENT_LIST_FILE}" REALPATH)
get_filename_component(_IMPORT_PREFIX "${_DIR}/../../../../" ABSOLUTE)
Copy link

@ye-luo ye-luo Nov 28, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I found.

CMAKE_CURRENT_LIST_FILE: /opt/rocm-ver/lib/cmake/hip-lang/hip-lang-config.cmake
_DIR: /opt/rocm-ver/lib/cmake/hip-lang/hip-lang-config.cmake
_IMPORT_PREFIX: /opt/rocm-ver

It seems giving the desired _IMPORT_PREFIX value.
_DIR is no more a directory and "${_DIR}/../../../../" seems very strange to me.
Not clear to me why this is better than the old working lines.

Copy link
Contributor

@raramakr raramakr Dec 16, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the hip-runtime-amd package , the hip-lang-config.cmake is there in

/opt/rocm-ver/lib/cmake/hip-lang/hip-lang-config.cmake # actual file
/opt/rocm-ver/hip/lib/cmake/hip-lang/hip-lang-config.cmake # Soft link file for backward compatibility

The directory /opt/rocm-ver/hip/lib/cmake/hip-lang is not a soft link, but file is soft link
With the previous code, the _IMPORT_PREFIX may become /opt/rocm-ver/hip if run from backward compatibility path.

Agree that _DIR is now no more a directory. Will update the variable name



#need _IMPORT_PREFIX to be set #FILE_REORG_BACKWARD_COMPATIBILITY
file(GLOB HIP_CLANG_INCLUDE_SEARCH_PATHS "${_IMPORT_PREFIX}/../llvm/lib/clang/*/include")
file(GLOB HIP_CLANG_INCLUDE_SEARCH_PATHS_REORG "${_IMPORT_PREFIX}/llvm/lib/clang/*/include")
file(GLOB HIP_CLANG_INCLUDE_SEARCH_PATHS "${_IMPORT_PREFIX}/llvm/lib/clang/*/include")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gregrodgers This is what I mentioned about hip-lang never done properly. The hard coded llvm on the path clearly choked aomp. The correct way should locate the clang header files inlcude path based on the compiler not the full rocm root. This is actually done correctly below using HIP_COMPILER_INSTALL_PATH for CLANG_RT.

I gave the fix as https://github.com/ROCm-Developer-Tools/HIP/pull/2776/files
But the upstream keeps doing wrong and even worse.

find_path(HIP_CLANG_INCLUDE_PATH __clang_cuda_math.h
HINTS ${HIP_CLANG_INCLUDE_SEARCH_PATHS}
${HIP_CLANG_INCLUDE_SEARCH_PATHS_REORG}
NO_DEFAULT_PATH)
get_filename_component(HIP_CLANG_INCLUDE_PATH "${HIP_CLANG_INCLUDE_PATH}" DIRECTORY)

Expand All @@ -87,7 +85,6 @@ endif()
#if HSA is not under ROCm then provide CMAKE_PREFIX_PATH=<HSA_PATH>
find_path(HSA_HEADER hsa/hsa.h
PATHS
"${_IMPORT_PREFIX}/../include" #FILE_REORG_BACKWARD_COMPATIBILITY
"${_IMPORT_PREFIX}/include"
"${ROCM_PATH}/include"
)
Expand Down