ROCm / HIP

HIP: C++ Heterogeneous-Compute Interface for Portability
https://rocmdocs.amd.com/projects/HIP/
MIT License
3.71k stars 528 forks source link

HIP 4.1: Problem with Spaces Between -I/-isystem and Include Path #2246

Closed ax3l closed 3 years ago

ax3l commented 3 years ago

Hi,

I realized in our CI (ECP WarpX) that with the latest update from HIP 4.0.20496-4f163c68 to 4.1.21072-c3eb5ccc that ships in public apt repos, the HIP compiler cannot find the C-MPI bindings of OpenMPI anymore when searched for with CMake.

Generic CMake execution for a CMakeLists.txt project that does find_packge(MPI REQUIRED) with C and C++ enabled as languages:

        source /etc/profile.d/rocm.sh
        hipcc --version  # 4.1.21072-c3eb5ccc

        export CXX=$(which hipcc)
        export CC=$(which hipcc)
        cmake -S . -B build -DCMAKE_VERBOSE_MAKEFILE=ON  # fails
        cmake --build build -j 2

Did something change in the HIP defaults with respect to C?

The particular cuplit seems to be this test compile line, that now adds a C++ std flag for a C target:

/usr/bin/hipcc  -isystem /usr/lib/x86_64-linux-gnu/openmpi/include/openmpi -isystem /usr/lib/x86_64-linux-gnu/openmpi/include -pthread -o CMakeFiles/cmTC_08339.dir/test_mpi.c.o -c /usr/local/share/cmake-3.19/Modules/FindMPI/test_mpi.c
error: invalid argument '-std=c++11' not allowed with 'C'

Since HIP is used for the Exascale Computing project, do you think it would be possible add a CI CMake test that uses hipcc to build simple C/C++ projects against MPICH and OpenMPI?

Refs.: CMake report https://gitlab.kitware.com/cmake/cmake/-/issues/21968

ax3l commented 3 years ago

cc @ROCmSupport

ax3l commented 3 years ago

Full reproducer:

docker run -it ubuntu:20.04
apt update
apt install -y wget gnupg2

wget -q -O - http://repo.radeon.com/rocm/rocm.gpg.key | apt-key add -
echo 'deb [arch=amd64] http://repo.radeon.com/rocm/apt/debian/ xenial main'  | tee /etc/apt/sources.list.d/rocm.list
echo 'export PATH=$PATH:/opt/rocm/bin:/opt/rocm/profiler/bin:/opt/rocm/opencl/bin' | tee -a /etc/profile.d/rocm.sh

apt update
apt install -y --no-install-recommends \
    build-essential \
    cmake \
    gfortran        \
    libnuma-dev     \
    libopenmpi-dev  \
    openmpi-bin     \
    rocm-dev

source /etc/profile.d/rocm.sh

cd /opt
cat <<EOF >> main.c
#include <mpi.h>
int main(){}
EOF

cat <<EOF > CMakeLists.txt
cmake_minimum_required(VERSION 3.17)
project(testMPI)
find_package(MPI REQUIRED)

add_executable(test main.c)
target_link_libraries(test MPI::MPI_C)
EOF

export CC=$(which hipcc)
export CXX=$(which hipcc)
cmake -S . -B build

# optional:
#cat /opt/build/CMakeFiles/CMakeOutput.log
cat /opt/build/CMakeFiles/CMakeError.log
ax3l commented 3 years ago

Problem debugged: the issue is that starting with hipcc 4.1.0, spaces anymore between -isystem/-I and the include search path cause this problem.

Since this works with gcc and clang, as well as earlier hipcc releases, this should probably be considered a bug.

Work-around: ignoring hipcc and using clang as C compiler. Another work-around: passing hipcc with -x c also helps.

ax3l commented 3 years ago

It’s a bit curious that the new compiler wrapper in 4.1 unconditionally adds the -std=c++11 flag on the way down when using with a space as above.

The CFE front-end should usually set its own default, which is already at C++14, and this might interfere on the long-term if set otherwise by the hipcc wrapper: https://gist.github.com/ax3l/53db9fa8a4f4c21ecc5c4100c0d93c94

cgmb commented 3 years ago

I've encountered this issue before. If you want to dig deeper into it, hipcc is pretty easy to edit and debug since it's just a perl script.

hipcc tries to detect whether it's being used as a C compiler or a C++ compiler. It has a standard set of flags that it adds when building C++ and a standard set of flags it adds when building C. However, there's a lot of different clang arguments that it doesn't understand, and it sometimes gets confused about what language it's trying to build, resulting in using both the C flags and the C++ flags when building a C source file.

I wasn't courageous enough to submit a patch, but there's definitely a problem here: https://github.com/ROCm-Developer-Tools/HIP/blob/rocm-4.1.0/bin/hipcc#L567-L573

That's executed for every unknown argument that's not preceded by a -o, and may end up setting both needCFLAGS = 1 and needCXXFLAGS = 1. Changing that so that it resets needCXXFLAGS back to zero when it sets needCFLAGS to one and vice versa would probably fix this problem with C flags, but I don't know if it would break something else.

ax3l commented 3 years ago

I've encountered this issue before. If you want to dig deeper into it, hipcc is pretty easy to edit and debug since it's just a perl script.

I have to admit, I don't know perl.

I hope that once #2275 and #2278 are fixed I can transition to using clang++ directly by using the hip::device CMake targets. They are already recommended in the latest HIP docs for CMake users like me :)

For now, I still use hipcc as CXX compiler and worked around the issue reported here by using clang as the C compiler. I would say that was the original culpit, wrongly using a CXX compiler (hipcc) for C targets, too.

ax3l commented 3 years ago

Fixed in ROCm 4.3.0 :tada: