ROCm / MIOpen

AMD's Machine Intelligence Library
https://rocm.docs.amd.com/projects/MIOpen/en/latest/
Other
1.09k stars 230 forks source link

[SWDEV-492880] Add ROCm path for hipRTC #3391

Open huanrwan-amd opened 2 weeks ago

huanrwan-amd commented 2 weeks ago

This fix is for https://github.com/ROCm/MIOpen/issues/3314 .

As in the issue: "hipRTC/Comgr automatically added -I /opt/rocm/include to kernel compilations. However, this behavior is being removed, as hipRTC has no guarantees about a system ROCm installation during kernel compilation/execution."

huanrwan-amd commented 1 week ago

I'll approve because this will be correct for the case with the symbolic link present and doesn't present complications otherwise. In the case where rocm is installed somewhere else, this change would not be sufficient. That path might be part of the CMAKE_PREFIX_PATH.

Thanks for the input. For my understanding, the CMAKE_PREFIX_PATH is for building the MIOpendriver binary and *.so. This function BuildHip() is to using hipRTC for an online compilation. One possibility here is to check ROCM_PATH and ROCM_HOME env:

        const char* rocm_path = std::getenv("ROCM_PATH");
        if(rocm_path == nullptr)
        {
            rocm_path = "/opt/rocm";
        }
        const char* rocm_home = std::getenv("ROCM_HOME");
        if(rocm_home == nullptr)
        {
            rocm_home = rocm_path;
        }
        opts.push_back(std::string("-I") + rocm_home + "/include");

I see people using both ROCM_PATH and ROCM_HOME at: https://github.com/cupy/cupy/issues/4493, But ROCM_PATH is recommended. Any suggestions?

lamb-j commented 1 week ago

I've seen ROCM_PATH used as well, not familiar with ROCM_HOME

huanrwan-amd commented 1 week ago

Hi @lamb-j and @cderb updated as we discussed. Using ROCM_PATH for a user specified ROCm path and log the build options for debugging.

BrianHarrisonAMD commented 1 week ago

Greetings @huanrwan-amd

Heads up, there was a formatting error on these changes. I fixed it to allow CI to run, but here is the script I run in case you need it in the future:

#!/bin/bash
find . -iname '*.h' \
    -o -iname '*.hpp' \
    -o -iname '*.cpp' \
    -o -iname '*.h.in' \
    -o -iname '*.hpp.in' \
    -o -iname '*.cpp.in' \
    -o -iname '*.cl' \
| grep -v -E '(build/)|(install/)' \
| xargs -n 1 -P $(nproc) -I{} -t clang-format-12 -style=file {} -i 2>/dev/null
CAHEK7 commented 1 week ago

Greetings @huanrwan-amd

Heads up, there was a formatting error on these changes. I fixed it to allow CI to run, but here is the script I run in case you need it in the future:

#!/bin/bash
find . -iname '*.h' \
    -o -iname '*.hpp' \
    -o -iname '*.cpp' \
    -o -iname '*.h.in' \
    -o -iname '*.hpp.in' \
    -o -iname '*.cpp.in' \
    -o -iname '*.cl' \
| grep -v -E '(build/)|(install/)' \
| xargs -n 1 -P $(nproc) -I{} -t clang-format-12 -style=file {} -i 2>/dev/null

It's also here https://github.com/ROCm/MIOpen/wiki/How-to-format-code