From a3576f6593735b8111e9e3d12259b002d2e52143 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Sat, 17 Oct 2026 16:47:33 -0700 Subject: [PATCH] Focus CI on the latest ROCm and amdclang hipObject will be built with the latest ROCm's amdclang in rocm-systems and TheRock, so CI now builds with that unless a job exists to try something else. Every CI build uses ROCm 10.1.0, apart from the hardware tests, whose guests run ROCm 10.0, and the nightly packages. The Build workflow's jobs are listed one by one rather than made by crossing axes. All of them use HIPOBJ_WERROR: - Core jobs, amdclang on Ubuntu 26.04: RelWithDebInfo; Debug with HIPOBJ_USE_SANITIZERS; RelWithDebInfo with HIPOBJ_USE_THREAD_SANITIZER, as TheRock's TSan builds are; and Debug with code coverage. - Smoke tests with amdclang: a static Release build (TheRock builds only shared libraries, and Release is its default), both NIC backends off, C++23, and C++26. - Upstream Clang 21, and GCC 15 with and without HIPOBJ_USE_SANITIZERS, for the problems amdclang misses. - Ubuntu 24.04's GCC 13 and Clang 18, with its CMake 3.28. Other workflows: - The nightly builds its packages with each ROCm version's amdclang. - clang-tidy, IWYU, and the MinIO integration check use the Ubuntu 26.04 and ROCm 10.1.0 image. clang-tidy stays at LLVM 23, the newest release on apt.llvm.org, since ROCm 10.1's LLVM 24 only has snapshots there. - IWYU, the MinIO integration check, the hardware tests, and ci/ernic/run-local.sh build with amdclang. run-local.sh sets the compiler on the cmake command line, so a build directory left from an earlier run with GCC is reconfigured. IWYU parses with the clang it was built with (Clang 19 on Ubuntu 26.04), which rejects flags newer than itself, such as amdclang's -fno-strict-bool. HIPOBJIWYU.cmake now reads that clang's version, and a Clang build with HIPOBJ_USE_IWYU uses only the flags both compilers have, hardening flags included. Builds without HIPOBJ_USE_IWYU are unchanged. iwyu-wrapper.sh passes a lone --version straight to IWYU, without recording it as a run. applyVendorQpAttrs()'s attr parameter is [[maybe_unused]], since nothing uses it when HIPOBJ_BNXT and HIPOBJ_IONIC are both off, which made HIPOBJ_WERROR fail the build. AGENTS.md, docs/building.rst, and CHANGELOG.md describe the new CI coverage, the nightly's compiler, and what HIPOBJ_USE_IWYU leaves out. Co-authored-by: Claude --- .github/workflows/hipobject-build.yml | 160 +++++++++++++----- .github/workflows/hipobject-clang-tidy.yml | 16 +- .../hipobject-hardware-test-gpu-direct.yml | 2 + .../hipobject-hardware-test-minio-bridge.yml | 2 + .../hipobject-hardware-test-s3-backend.yml | 2 + ...object-hardware-test-two-vm-gpu-direct.yml | 2 + ...ipobject-hardware-test-two-vm-protocol.yml | 2 + .github/workflows/hipobject-iwyu.yml | 13 +- .../hipobject-minio-integration-check.yml | 6 +- .github/workflows/hipobject-nightly.yml | 13 +- AGENTS.md | 55 +++--- CHANGELOG.md | 11 +- ci/ernic/run-local.sh | 7 + ci/iwyu/iwyu-wrapper.sh | 11 +- cmake/HIPOBJCompilerOptions.cmake | 9 +- cmake/HIPOBJIWYU.cmake | 26 +++ docs/building.rst | 14 +- src/rdma/transport.cpp | 3 +- 18 files changed, 268 insertions(+), 86 deletions(-) diff --git a/.github/workflows/hipobject-build.yml b/.github/workflows/hipobject-build.yml index c1402cab..04de2206 100644 --- a/.github/workflows/hipobject-build.yml +++ b/.github/workflows/hipobject-build.yml @@ -52,54 +52,125 @@ defaults: jobs: build: - name: >- - Build (${{ matrix.shared_libs == 'ON' && 'shared' || 'static' }}${{ matrix.sanitizer == 'asan' && ', ASan and UBSan' || matrix.sanitizer == 'tsan' && ', TSan' || '' }}${{ matrix.coverage == 'ON' && ', coverage' || '' }}${{ matrix.build_type && format(', {0}', matrix.build_type) || '' }}) + name: Build (${{ matrix.label }}) runs-on: ubuntu-latest strategy: - # Every other workflow builds the default configuration (shared - # library, no sanitizers), so this is the only place the static, - # sanitizer, and code coverage configurations get compiled and tested. # Let every job finish so a failure in one doesn't hide the state of # the others. fail-fast: false + # The jobs are listed one by one rather than made by crossing axes, + # since most of them change one thing from the first job, and + # crossing would build combinations nobody needs. Every job sets + # label, ubuntu, cc, cxx, and build_type. A job that leaves out one + # of the other keys gets its default: + # + # shared_libs ON. TheRock only builds shared libraries. + # sanitizer None. asan is HIPOBJ_USE_SANITIZERS (ASan, UBSan, + # and the compiler's other checks), and tsan is + # HIPOBJ_USE_THREAD_SANITIZER. + # coverage OFF + # cxx_standard 20 + # backends ON, which turns on both HIPOBJ_BNXT and + # HIPOBJ_IONIC + # packages None. Extra apt packages, such as a compiler. + # + # Every job uses ROCm 10.1.0, the latest ROCm (see the image below). matrix: - shared_libs: [ON, OFF] - # asan is HIPOBJ_USE_SANITIZERS, tsan is HIPOBJ_USE_THREAD_SANITIZER, - # and empty is neither - sanitizer: [''] - coverage: [OFF] - # Empty means the CMakeLists.txt default (RelWithDebInfo) - build_type: [''] include: - # Two sanitizer jobs with the default configuration: one with - # every sanitizer except ThreadSanitizer, and one with - # ThreadSanitizer. They build with ROCm's Clang, which has more - # checks than GCC. They build Debug, so they're also where the - # library's assert()s are checked, and sanitizer reports aren't - # muddied by optimization. - - shared_libs: ON - sanitizer: asan - coverage: OFF + # The core jobs build with ROCm's amdclang on the newest Ubuntu + # release, as TheRock will build hipObject. + - label: amdclang, RelWithDebInfo + ubuntu: '26.04' + cc: /opt/rocm/llvm/bin/amdclang + cxx: /opt/rocm/llvm/bin/amdclang++ + build_type: RelWithDebInfo + # Debug, so the library's assert()s are checked with the + # sanitizers on, and the reports aren't muddied by optimization + - label: amdclang, Debug, ASan and UBSan + ubuntu: '26.04' + cc: /opt/rocm/llvm/bin/amdclang + cxx: /opt/rocm/llvm/bin/amdclang++ build_type: Debug + sanitizer: asan + # RelWithDebInfo, as TheRock's ThreadSanitizer builds are, so the + # threads' timing is closer to a normal build's + - label: amdclang, RelWithDebInfo, TSan + ubuntu: '26.04' cc: /opt/rocm/llvm/bin/amdclang cxx: /opt/rocm/llvm/bin/amdclang++ - - shared_libs: ON + build_type: RelWithDebInfo sanitizer: tsan - coverage: OFF - build_type: Debug + # Debug, so the report's lines match the source. Coverage needs + # Clang. + - label: amdclang, Debug, coverage + ubuntu: '26.04' cc: /opt/rocm/llvm/bin/amdclang cxx: /opt/rocm/llvm/bin/amdclang++ - # One code coverage job with the default configuration. Coverage - # needs Clang, so it builds with ROCm's, too. It builds Debug, so - # the report's lines match the source. - - shared_libs: ON - sanitizer: '' - coverage: ON build_type: Debug + coverage: ON + # Smoke tests of what the core jobs don't build. Release is + # TheRock's default build type. + - label: amdclang, Release, static + ubuntu: '26.04' + cc: /opt/rocm/llvm/bin/amdclang + cxx: /opt/rocm/llvm/bin/amdclang++ + build_type: Release + shared_libs: OFF + - label: amdclang, no NIC backends + ubuntu: '26.04' + cc: /opt/rocm/llvm/bin/amdclang + cxx: /opt/rocm/llvm/bin/amdclang++ + build_type: RelWithDebInfo + backends: OFF + - label: amdclang, C++23 + ubuntu: '26.04' cc: /opt/rocm/llvm/bin/amdclang cxx: /opt/rocm/llvm/bin/amdclang++ + build_type: RelWithDebInfo + cxx_standard: 23 + # C++26 needs CMake 3.30 or later, which Ubuntu 24.04 doesn't have + - label: amdclang, C++26 + ubuntu: '26.04' + cc: /opt/rocm/llvm/bin/amdclang + cxx: /opt/rocm/llvm/bin/amdclang++ + build_type: RelWithDebInfo + cxx_standard: 26 + # Upstream Clang, the version Ubuntu installs as clang + - label: Clang 21 + ubuntu: '26.04' + cc: clang-21 + cxx: clang++-21 + build_type: RelWithDebInfo + packages: clang-21 + # GCC is a second set of eyes: its warnings and sanitizers find + # problems that Clang's don't. The sanitizer job builds + # RelWithDebInfo, since the amdclang one builds Debug. + - label: GCC 15 + ubuntu: '26.04' + cc: gcc-15 + cxx: g++-15 + build_type: RelWithDebInfo + - label: GCC 15, ASan and UBSan + ubuntu: '26.04' + cc: gcc-15 + cxx: g++-15 + build_type: RelWithDebInfo + sanitizer: asan + # The oldest supported Ubuntu release, with older compilers and + # an older CMake (3.28) + - label: Ubuntu 24.04, GCC 13 + ubuntu: '24.04' + cc: gcc-13 + cxx: g++-13 + build_type: RelWithDebInfo + - label: Ubuntu 24.04, Clang 18 + ubuntu: '24.04' + cc: clang-18 + cxx: clang++-18 + build_type: RelWithDebInfo + packages: clang-18 container: - image: rocm/dev-ubuntu-24.04:10.0.0-full + image: rocm/dev-ubuntu-${{ matrix.ubuntu }}:10.1.0-full options: --user root env: ROCM_PATH: /opt/rocm @@ -119,8 +190,14 @@ jobs: # # The MinIO bridge needs nlohmann-json3-dev and libpugixml-dev. # libcurlpp-dev saves downloading and building curlpp. + # + # A job's packages (EXTRA_PACKAGES) are separated by spaces, so read + # splits them into an array with one package in each element. - name: Install dependencies + env: + EXTRA_PACKAGES: ${{ matrix.packages }} run: | + read -ra extra_packages <<< "${EXTRA_PACKAGES}" apt-get update apt-get install -y \ cmake \ @@ -131,18 +208,18 @@ jobs: libpugixml-dev \ libssl-dev \ nlohmann-json3-dev \ - rpm + rpm \ + "${extra_packages[@]}" # HIPOBJ_WERROR makes a new warning fail the build. It only # covers hipObject's own targets, not the code FetchContent pulls in. # # Every job but the coverage job builds the MinIO bridge, so it's - # compiled with GCC's warnings in each configuration and ctest's - # install and package tests include it. The coverage report only - # measures src/, so the bridge would only slow that job down. + # compiled with each compiler's warnings and ctest's install and + # package tests include it. The coverage report only measures src/, + # so the bridge would only slow that job down. # - # CMake picks the compilers up from CC and CXX, and ignores them when - # they're empty, as they are for the jobs that don't set cc and cxx. + # CMake picks the compilers up from CC and CXX. - name: Configure env: CC: ${{ matrix.cc }} @@ -152,13 +229,14 @@ jobs: -DCMAKE_PREFIX_PATH=/opt/rocm \ -DBUILD_TESTING=ON \ -DHIPOBJ_WERROR=ON \ - -DHIPOBJ_BNXT=ON \ - -DHIPOBJ_IONIC=ON \ + -DHIPOBJ_BNXT=${{ matrix.backends || 'ON' }} \ + -DHIPOBJ_IONIC=${{ matrix.backends || 'ON' }} \ -DHIPOBJ_MINIO_CLIENT=${{ matrix.coverage == 'ON' && 'OFF' || 'ON' }} \ - -DBUILD_SHARED_LIBS=${{ matrix.shared_libs }} \ + -DBUILD_SHARED_LIBS=${{ matrix.shared_libs || 'ON' }} \ + -DHIPOBJ_CXX_STANDARD=${{ matrix.cxx_standard || '20' }} \ -DHIPOBJ_USE_SANITIZERS=${{ matrix.sanitizer == 'asan' && 'ON' || 'OFF' }} \ -DHIPOBJ_USE_THREAD_SANITIZER=${{ matrix.sanitizer == 'tsan' && 'ON' || 'OFF' }} \ - -DHIPOBJ_USE_CODE_COVERAGE=${{ matrix.coverage }} \ + -DHIPOBJ_USE_CODE_COVERAGE=${{ matrix.coverage || 'OFF' }} \ -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} - name: Build diff --git a/.github/workflows/hipobject-clang-tidy.yml b/.github/workflows/hipobject-clang-tidy.yml index 0d6da76c..b8d829e8 100644 --- a/.github/workflows/hipobject-clang-tidy.yml +++ b/.github/workflows/hipobject-clang-tidy.yml @@ -8,8 +8,11 @@ name: hipObject - clang-tidy # This uses upstream clang-tidy rather than ROCm's, which is built # without the clang-analyzer-* checks, as hipFile's clang-tidy workflow # does. The compiler is still ROCm's amdclang++. Keep LLVM_VERSION at -# the LLVM major version of the image's ROCm (ROCm 10.0 is LLVM 23), so -# clang-tidy understands the compiler's flags. +# the LLVM major version of the image's ROCm where apt.llvm.org has a +# release of it, so clang-tidy understands the compiler's flags. ROCm +# 10.1 is LLVM 24, which only has snapshot builds, so this uses LLVM 23, +# the newest release. That works while the build uses no warning flag +# newer than Clang 23 (see HIPOBJClangCompilerOptions.cmake). on: # The same triggers and paths as the Build workflow, since anything that @@ -58,13 +61,13 @@ jobs: name: clang-tidy runs-on: ubuntu-latest container: - image: rocm/dev-ubuntu-24.04:10.0.0-full + image: rocm/dev-ubuntu-26.04:10.1.0-full options: --user root env: ROCM_PATH: /opt/rocm LLVM_VERSION: 23 # The Ubuntu release of the image, for apt.llvm.org - UBUNTU_CODENAME: noble + UBUNTU_CODENAME: resolute # The fingerprint of apt.llvm.org's signing key, which is public LLVM_SIGNER_FINGERPRINT: 6084F3CF814B57C1CF12EFD515CF4D18AF4F7421 steps: @@ -125,9 +128,8 @@ jobs: # installed above, rather than whatever HIPOBJClangTidy.cmake's # search finds first. # - # This builds the MinIO bridge, which the Build workflow's Clang - # job (code coverage) doesn't, so HIPOBJ_WERROR is on to make this - # the job that fails on Clang's warnings in the bridge. + # HIPOBJ_WERROR is on, so a compiler warning fails this job too, as + # it does the Build workflow's jobs. - name: Configure env: CC: /opt/rocm/llvm/bin/amdclang diff --git a/.github/workflows/hipobject-hardware-test-gpu-direct.yml b/.github/workflows/hipobject-hardware-test-gpu-direct.yml index e971a1ff..6ae86598 100644 --- a/.github/workflows/hipobject-hardware-test-gpu-direct.yml +++ b/.github/workflows/hipobject-hardware-test-gpu-direct.yml @@ -225,6 +225,8 @@ jobs: -v "${{ github.workspace }}:/hipobject:ro" \ -v "${{ github.workspace }}/ernic-build-gpu:/hipobject-build" \ -e ROCM_PATH=/opt/rocm \ + -e CC=/opt/rocm/llvm/bin/amdclang \ + -e CXX=/opt/rocm/llvm/bin/amdclang++ \ "$ROCM_IMAGE" \ bash -c " apt-get update -qq && \ diff --git a/.github/workflows/hipobject-hardware-test-minio-bridge.yml b/.github/workflows/hipobject-hardware-test-minio-bridge.yml index c98c847d..5ab20563 100644 --- a/.github/workflows/hipobject-hardware-test-minio-bridge.yml +++ b/.github/workflows/hipobject-hardware-test-minio-bridge.yml @@ -169,6 +169,8 @@ jobs: -v "${{ github.workspace }}:/hipobject:ro" \ -v "${{ github.workspace }}/ernic-build:/hipobject-build" \ -e ROCM_PATH=/opt/rocm \ + -e CC=/opt/rocm/llvm/bin/amdclang \ + -e CXX=/opt/rocm/llvm/bin/amdclang++ \ "$ROCM_IMAGE" \ bash -c " apt-get update -qq && \ diff --git a/.github/workflows/hipobject-hardware-test-s3-backend.yml b/.github/workflows/hipobject-hardware-test-s3-backend.yml index 019726e5..6505c2f5 100644 --- a/.github/workflows/hipobject-hardware-test-s3-backend.yml +++ b/.github/workflows/hipobject-hardware-test-s3-backend.yml @@ -145,6 +145,8 @@ jobs: -v "${{ github.workspace }}:/hipobject:ro" \ -v "${{ github.workspace }}/ernic-build-s3:/hipobject-build" \ -e ROCM_PATH=/opt/rocm \ + -e CC=/opt/rocm/llvm/bin/amdclang \ + -e CXX=/opt/rocm/llvm/bin/amdclang++ \ "$ROCM_IMAGE" \ bash -c " apt-get update -qq && \ diff --git a/.github/workflows/hipobject-hardware-test-two-vm-gpu-direct.yml b/.github/workflows/hipobject-hardware-test-two-vm-gpu-direct.yml index 45851b2c..bc0c1975 100644 --- a/.github/workflows/hipobject-hardware-test-two-vm-gpu-direct.yml +++ b/.github/workflows/hipobject-hardware-test-two-vm-gpu-direct.yml @@ -197,6 +197,8 @@ jobs: -v "${{ github.workspace }}:/hipobject:ro" \ -v "${{ github.workspace }}/ernic-build:/hipobject-build" \ -e ROCM_PATH=/opt/rocm \ + -e CC=/opt/rocm/llvm/bin/amdclang \ + -e CXX=/opt/rocm/llvm/bin/amdclang++ \ "$ROCM_IMAGE" \ bash -c " apt-get update -qq && \ diff --git a/.github/workflows/hipobject-hardware-test-two-vm-protocol.yml b/.github/workflows/hipobject-hardware-test-two-vm-protocol.yml index 1b2d83bb..b76c2bfe 100644 --- a/.github/workflows/hipobject-hardware-test-two-vm-protocol.yml +++ b/.github/workflows/hipobject-hardware-test-two-vm-protocol.yml @@ -165,6 +165,8 @@ jobs: -v "${{ github.workspace }}:/hipobject:ro" \ -v "${{ github.workspace }}/ernic-build:/hipobject-build" \ -e ROCM_PATH=/opt/rocm \ + -e CC=/opt/rocm/llvm/bin/amdclang \ + -e CXX=/opt/rocm/llvm/bin/amdclang++ \ "$ROCM_IMAGE" \ bash -c " apt-get update -qq && \ diff --git a/.github/workflows/hipobject-iwyu.yml b/.github/workflows/hipobject-iwyu.yml index 2fc28e95..1709d4d4 100644 --- a/.github/workflows/hipobject-iwyu.yml +++ b/.github/workflows/hipobject-iwyu.yml @@ -57,7 +57,7 @@ jobs: name: include-what-you-use (advisory) runs-on: ubuntu-latest container: - image: rocm/dev-ubuntu-24.04:10.0.0-full + image: rocm/dev-ubuntu-26.04:10.1.0-full options: --user root env: ROCM_PATH: /opt/rocm @@ -89,17 +89,18 @@ jobs: # runs IWYU through a wrapper that records every run for the check # step. # - # The iwyu package installs clang, so the compilers are set to GCC - # rather than left to whichever c++ CMake finds first. GCC also - # checks that IWYU's clang copes with the GCC-only warning flags. + # The compiler is amdclang, as in the other workflows. IWYU's clang + # is older, so the build leaves out the flags it doesn't have (see + # HIPOBJIWYU.cmake). # # HIPOBJ_WERROR is off: the Build workflow checks for warnings, and # this job is only about IWYU. - name: Configure + env: + CC: /opt/rocm/llvm/bin/amdclang + CXX: /opt/rocm/llvm/bin/amdclang++ run: | cmake -B build -G Ninja \ - -DCMAKE_C_COMPILER=gcc \ - -DCMAKE_CXX_COMPILER=g++ \ -DCMAKE_PREFIX_PATH=/opt/rocm \ -DBUILD_TESTING=ON \ -DHIPOBJ_BNXT=ON \ diff --git a/.github/workflows/hipobject-minio-integration-check.yml b/.github/workflows/hipobject-minio-integration-check.yml index eaae8b7c..fce99667 100644 --- a/.github/workflows/hipobject-minio-integration-check.yml +++ b/.github/workflows/hipobject-minio-integration-check.yml @@ -66,7 +66,7 @@ jobs: runs-on: ubuntu-24.04 timeout-minutes: 30 container: - image: rocm/dev-ubuntu-24.04:10.0.0-full + image: rocm/dev-ubuntu-26.04:10.1.0-full options: --user root env: ROCM_PATH: /opt/rocm @@ -137,7 +137,11 @@ jobs: "$MINIO_ROOT_USER" "$MINIO_ROOT_PASSWORD" --insecure mc mb "ci/$TEST_BUCKET" + # CMake picks the compilers up from CC and CXX - name: Build hipObject with curl support + env: + CC: /opt/rocm/llvm/bin/amdclang + CXX: /opt/rocm/llvm/bin/amdclang++ run: | cmake -B build \ -DCMAKE_PREFIX_PATH=/opt/rocm \ diff --git a/.github/workflows/hipobject-nightly.yml b/.github/workflows/hipobject-nightly.yml index 1d71b0d4..38fbbc0d 100644 --- a/.github/workflows/hipobject-nightly.yml +++ b/.github/workflows/hipobject-nightly.yml @@ -127,6 +127,9 @@ jobs: # and install into, the version's own directory instead, as ROCm's # packages do. The test programs find libamdhip64 through # LD_LIBRARY_PATH (see hipobject-build.yml). + # + # The packages are built with that version's amdclang, as TheRock + # builds ROCm. CMake picks the compilers up from CC and CXX. - name: Find ROCm run: | rocm_path="/opt/rocm/core-${ROCM_VERSION%.*}" @@ -134,16 +137,24 @@ jobs: echo "::error::ROCm ${ROCM_VERSION} isn't in ${rocm_path}" exit 1 fi + for compiler in amdclang amdclang++; do + if [[ ! -x "${rocm_path}/llvm/bin/${compiler}" ]]; then + echo "::error::${compiler} isn't in ${rocm_path}/llvm/bin" + exit 1 + fi + done { echo "ROCM_PATH=${rocm_path}" echo "LD_LIBRARY_PATH=${rocm_path}/lib" + echo "CC=${rocm_path}/llvm/bin/amdclang" + echo "CXX=${rocm_path}/llvm/bin/amdclang++" } >> "${GITHUB_ENV}" # rocm-cmake reads the package release from the environment when # hipObject is configured. # # Warnings aren't errors here. The build workflow checks them, but - # only on Ubuntu 24.04, so a newer release's compiler can warn about + # only with ROCm 10.1.0, so another version's compiler can warn about # code that workflow passes. That shouldn't stop the packages. - name: Configure env: diff --git a/AGENTS.md b/AGENTS.md index c617a594..7822114c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -52,7 +52,7 @@ one or edit the other project without asking. 7.11+ layout) and to `/opt/rocm` otherwise; setting `ROCM_VERSION` changes this (see [docs/building.rst](docs/building.rst)). Put `/opt/rocm/bin` on `PATH` and include `/opt/rocm/lib` in `LD_LIBRARY_PATH` when running binaries - you build. The `rocm/dev-ubuntu-24.04` full image that CI uses leaves + you build. The `rocm/dev-ubuntu` full images that CI uses leave `libamdhip64` out of the loader cache, so test binaries won't start there without it. - **Compiler**: hipObject is host-only C/C++ (no GPU kernels), so any C++20 @@ -77,17 +77,27 @@ Notes: - `HIPOBJ_BNXT` and `HIPOBJ_IONIC` are on by default, and the build steps in [docs/building.rst](docs/building.rst) leave them that way, so they compile - both RDMA backends, as CI's build workflow does. The emulated-hardware - workflows turn `HIPOBJ_BNXT` off, since their NIC is an ionic one. -- CI ([hipobject-build.yml](.github/workflows/hipobject-build.yml)) builds this - configuration in the `rocm/dev-ubuntu-24.04:10.0.0-full` image, along with - variants that add `-DBUILD_SHARED_LIBS=OFF`, `-DHIPOBJ_USE_SANITIZERS=ON` - (every sanitizer except ThreadSanitizer), `-DHIPOBJ_USE_THREAD_SANITIZER=ON`, - or `-DHIPOBJ_USE_CODE_COVERAGE=ON`. The sanitizer and coverage variants are - built with ROCm's amdclang. Coverage is reported on the run's summary page, - and the job fails below the [coverage target](#tests); on `develop`, a job - then publishes the result to the `badges` branch for the README's Coverage - badge. It is the only workflow that builds these variants. + both RDMA backends, as all but one of CI's build jobs do. The + emulated-hardware workflows turn `HIPOBJ_BNXT` off, since their NIC is an + ionic one. +- CI's build workflow + ([hipobject-build.yml](.github/workflows/hipobject-build.yml)) focuses on + the latest ROCm and its amdclang, which is how TheRock will build hipObject. + Every job uses ROCm 10.1.0, with `HIPOBJ_WERROR` on. Its core jobs build + this configuration with amdclang on Ubuntu 26.04 (the + `rocm/dev-ubuntu-26.04:10.1.0-full` image): `RelWithDebInfo`, then Debug + with `-DHIPOBJ_USE_SANITIZERS=ON` (every sanitizer except ThreadSanitizer), + `RelWithDebInfo` with `-DHIPOBJ_USE_THREAD_SANITIZER=ON`, and Debug with + `-DHIPOBJ_USE_CODE_COVERAGE=ON`. Single jobs smoke-test the rest: with + amdclang, a static `Release` build (`-DBUILD_SHARED_LIBS=OFF`), one with + both backends off, and C++23 and C++26 (`HIPOBJ_CXX_STANDARD`); upstream + Clang 21; GCC 15, with and without `-DHIPOBJ_USE_SANITIZERS=ON`; and GCC 13 + and Clang 18 on Ubuntu 24.04, which also has an older CMake (3.28). + Coverage is reported on the run's summary page, and the job fails below the + [coverage target](#tests); on `develop`, a job then publishes the result to + the `badges` branch for the README's Coverage badge. The nightly packages + ([hipobject-nightly.yml](.github/workflows/hipobject-nightly.yml)) are + built with amdclang for ROCm 10.1.0 and 7.14.1, on Ubuntu 24.04 and 26.04. - Optional MinIO C++ bridge: `-DHIPOBJ_MINIO_CLIENT=ON`, plus the system packages listed in [docs/building.rst](docs/building.rst); see [integrations/minio-cpp/TESTING.md](integrations/minio-cpp/TESTING.md). @@ -502,8 +512,8 @@ configuration files. - **No compiler warnings, with both GCC and Clang.** They warn about different things, so build with each, in separate build directories (`-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++`, and `clang`/`clang++`). - CI builds with GCC, apart from its one code coverage job, so it doesn't check - Clang's warnings across configurations. The build turns on a long list of + CI builds with both, but mostly with amdclang, and only a few + configurations with GCC and upstream Clang. The build turns on a long list of warnings ([`HIPOBJGNUCompilerOptions.cmake`](cmake/HIPOBJGNUCompilerOptions.cmake), [`HIPOBJClangCompilerOptions.cmake`](cmake/HIPOBJClangCompilerOptions.cmake)) @@ -545,11 +555,15 @@ configuration files. with upstream clang-tidy 23. - **include-what-you-use (IWYU)**: configure with `-DHIPOBJ_USE_IWYU=ON` and build; IWYU prints the `#include` lines it suggests adding or removing for - each file. Unlike the other checks, it doesn't have to report nothing, since - some of its suggestions are wrong. Make the suggestions that are right for - the files you touch, such as adding a header for a symbol the file uses but - only gets indirectly. Ignore the ones that are wrong, and leave the code as - it is. For example, IWYU asks for `` in files that compare + each file. Use a separate build directory, only for IWYU: when the compiler + is a newer Clang than IWYU's, such as amdclang, the build leaves out the + flags IWYU's clang doesn't have, hardening flags included + ([`HIPOBJIWYU.cmake`](cmake/HIPOBJIWYU.cmake)). Use Clang; IWYU rejects + some of GCC's flags. Unlike the other checks, it doesn't have to report + nothing, since some of its suggestions are wrong. Make the suggestions that + are right for the files you touch, such as adding a header for a symbol the + file uses but only gets indirectly. Ignore the ones that are wrong, and + leave the code as it is. For example, IWYU asks for `` in files that compare `std::chrono` time points (`steady_clock::now() < deadline`), because libstdc++ implements those comparisons with `operator<=>`, but `` already includes ``, as the standard requires. Other wrong @@ -589,7 +603,8 @@ configuration files. fetches and instruments GoogleTest, so it needs network access the first time it's configured. Don't add sanitizer flags or conditions anywhere but `HIPOBJSanitizers.cmake`: it instruments every target itself, at the end - of the configuration. CI runs both builds with amdclang. + of the configuration. CI runs both builds with amdclang, and + `HIPOBJ_USE_SANITIZERS` with GCC too. - **Lifetime safety, when a change could leave something dangling.** Clang 23's `-Wlifetime-safety` looks for use-after-free, use-after-scope, and dangling references, but it reports too many false positives to leave diff --git a/CHANGELOG.md b/CHANGELOG.md index 663ef69a..d3480036 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,8 +29,9 @@ doesn't. - Nightly DEB packages of the `develop` branch, for Ubuntu 24.04 and 26.04 with ROCm 10.1.0 and 7.14.1, are published as the `nightly` - prerelease on GitHub, which each new build replaces. They're for - testing, and aren't a supported release. + prerelease on GitHub, which each new build replaces. They're built + with the ROCm version's amdclang. They're for testing, and aren't a + supported release. - The `ROCM_VERSION` CMake and environment variable sets the ROCm version, which selects the package dependencies and the default `ROCM_PATH`. It defaults to the version of the ROCm in `ROCM_PATH`. @@ -43,7 +44,11 @@ - The `HIPOBJ_USE_IWYU` CMake option runs include-what-you-use on hipObject's own code while it compiles, which prints suggestions for `#include` lines to add or remove. The suggestions don't fail the - build. It defaults to `OFF`. + build. It defaults to `OFF`. When the compiler is a newer Clang than + the one include-what-you-use was built with, such as amdclang, the + build leaves out the compiler flags that include-what-you-use's clang + doesn't have, including some hardening flags, so use a separate build + directory for it. - The `HIPOBJ_USE_CODE_COVERAGE` CMake option builds hipObject with LLVM code coverage instrumentation, and adds a `hipobj-coverage` target that reports the library's coverage after the tests run. It diff --git a/ci/ernic/run-local.sh b/ci/ernic/run-local.sh index b0538861..16a22ba3 100755 --- a/ci/ernic/run-local.sh +++ b/ci/ernic/run-local.sh @@ -185,6 +185,11 @@ _rocm_build() { ca_args="-v ${AMD_CA_CERT}:/tmp/amd-root-ca.crt:ro" fi + # Builds with amdclang, as CI does. The compilers are set on the cmake + # command line, not with CC and CXX, so a build directory left from an + # earlier run with another compiler is reconfigured: CMake ignores CC + # and CXX once it has cached a compiler. + # # shellcheck disable=SC2086 docker run --rm \ --user root \ @@ -204,6 +209,8 @@ _rocm_build() { libcurl4-openssl-dev libssl-dev \ nlohmann-json3-dev libpugixml-dev && \ cmake -B /hipobject-build -G Ninja -S /hipobject \ + -DCMAKE_C_COMPILER=/opt/rocm/llvm/bin/amdclang \ + -DCMAKE_CXX_COMPILER=/opt/rocm/llvm/bin/amdclang++ \ -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_PREFIX_PATH=/opt/rocm \ -DBUILD_TESTING=ON \ diff --git a/ci/iwyu/iwyu-wrapper.sh b/ci/iwyu/iwyu-wrapper.sh index bc727f5c..5bfa4ade 100755 --- a/ci/iwyu/iwyu-wrapper.sh +++ b/ci/iwyu/iwyu-wrapper.sh @@ -24,12 +24,21 @@ # # HIPOBJ_IWYU_WRAPPED_EXE selects the IWYU to run. It defaults to the # include-what-you-use on PATH. +# +# "--version" alone isn't a run, so it goes straight to IWYU, without +# being recorded or needing HIPOBJ_IWYU_LOG_DIR. HIPOBJIWYU.cmake runs it +# when hipObject is configured, to get the version of IWYU's clang. set -u -log_dir=${HIPOBJ_IWYU_LOG_DIR:?HIPOBJ_IWYU_LOG_DIR must be set} iwyu=${HIPOBJ_IWYU_WRAPPED_EXE:-include-what-you-use} +if [[ $# -eq 1 && $1 == --version ]]; then + exec "${iwyu}" --version +fi + +log_dir=${HIPOBJ_IWYU_LOG_DIR:?HIPOBJ_IWYU_LOG_DIR must be set} + # The run's exit status goes in this file. Its name is the base for the # run's other files. run=$(mktemp "${log_dir}/run.XXXXXX") || exit 1 diff --git a/cmake/HIPOBJCompilerOptions.cmake b/cmake/HIPOBJCompilerOptions.cmake index f0882d84..dde14388 100644 --- a/cmake/HIPOBJCompilerOptions.cmake +++ b/cmake/HIPOBJCompilerOptions.cmake @@ -63,7 +63,14 @@ function(hipobj_set_compiler_flags target) if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") hipobj_get_gnu_warning_flags(flags ${CMAKE_CXX_COMPILER_VERSION}) elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") - hipobj_get_clang_warning_flags(flags ${CMAKE_CXX_COMPILER_VERSION}) + # IWYU gets the same flags as the compiler, so in an IWYU build, use + # only the flags that IWYU's clang has too (see HIPOBJIWYU.cmake) + set(clang_version ${CMAKE_CXX_COMPILER_VERSION}) + if(HIPOBJ_USE_IWYU AND HIPOBJ_IWYU_CLANG_VERSION + AND HIPOBJ_IWYU_CLANG_VERSION VERSION_LESS clang_version) + set(clang_version ${HIPOBJ_IWYU_CLANG_VERSION}) + endif() + hipobj_get_clang_warning_flags(flags ${clang_version}) endif() target_compile_options(${target} PRIVATE "$<$:${flags}>") diff --git a/cmake/HIPOBJIWYU.cmake b/cmake/HIPOBJIWYU.cmake index 994c044a..8cad1cff 100644 --- a/cmake/HIPOBJIWYU.cmake +++ b/cmake/HIPOBJIWYU.cmake @@ -6,6 +6,13 @@ # # When this option is enabled, compilation will emit IWYU suggestions. # They are advisory: IWYU doesn't fail the build. +# +# A build with this option is for checking includes, not for building +# hipObject to use. IWYU gets the same flags as the compiler, and it +# parses with the clang it was built with, which rejects flags that are +# newer than that clang. So when the compiler is a newer Clang (such as +# amdclang), the build leaves out the flags that IWYU's clang doesn't +# have, including some hardening flags (see hipobj_set_compiler_flags()). include_guard(GLOBAL) @@ -16,6 +23,25 @@ if(HIPOBJ_USE_IWYU) find_program(HIPOBJ_IWYU_EXE NAMES include-what-you-use REQUIRED) message(STATUS "include-what-you-use enabled: ${HIPOBJ_IWYU_EXE}") + # The version of IWYU's clang, from a line like "include-what-you-use + # 0.23 based on Ubuntu clang version 19.1.7 (20ubuntu4)". Without it, + # the build keeps every flag, and IWYU may reject some of them. + set(HIPOBJ_IWYU_CLANG_VERSION "") + execute_process( + COMMAND "${HIPOBJ_IWYU_EXE}" --version + RESULT_VARIABLE iwyu_version_result + OUTPUT_VARIABLE iwyu_version_output + ERROR_VARIABLE iwyu_version_output) + if(iwyu_version_result EQUAL 0 + AND iwyu_version_output MATCHES "clang version ([0-9]+(\\.[0-9]+)*)") + set(HIPOBJ_IWYU_CLANG_VERSION "${CMAKE_MATCH_1}") + message(STATUS "include-what-you-use's clang: ${HIPOBJ_IWYU_CLANG_VERSION}") + else() + message(WARNING + "Can't get the version of include-what-you-use's clang, so the " + "build may use flags it rejects:\n${iwyu_version_output}") + endif() + # GoogleTest exposes its public API through an umbrella header # (gtest/gtest.h) but defines symbols in private sub-headers. Without a # mapping, IWYU suggests including those private headers and confuses diff --git a/docs/building.rst b/docs/building.rst index f48008ed..e971a6cd 100644 --- a/docs/building.rst +++ b/docs/building.rst @@ -268,7 +268,12 @@ versioned names such as ``clang-tidy-23``, newest first, and then in clang-tidy comes last because it's built without the ``clang-analyzer-*`` checks. Set ``HIPOBJ_CLANG_TIDY_EXE`` to the path of the one to use. ``HIPOBJ_USE_IWYU`` runs include-what-you-use -the same way; its suggestions don't fail the build. +the same way; its suggestions don't fail the build. Use a separate +build directory for it, with Clang as the compiler: when the +compiler is a newer Clang than the one include-what-you-use was +built with (such as ``amdclang``), the build leaves out the flags +that include-what-you-use's clang doesn't have, including some +hardening flags. ``ROCM_PATH`` and ``ROCM_VERSION`` can also be set in the environment. If ``ROCM_PATH`` isn't set but ``ROCM_VERSION`` is, @@ -434,9 +439,10 @@ DEB packages of the ``develop`` branch are built every night that it changes, and published as the `nightly prerelease `_, which each new build replaces. There are packages for Ubuntu -24.04 and 26.04, built for ROCm 10.1.0 and 7.14.1. They -install into ``/opt/rocm/core-.``, so the -packages for both ROCm versions can be installed together, and +24.04 and 26.04, built for ROCm 10.1.0 and 7.14.1 with that +version's ``amdclang``. They install into +``/opt/rocm/core-.``, so the packages for both +ROCm versions can be installed together, and their release is ``0..g~``, so a newer nightly upgrades an older one. The nightly packages are for diff --git a/src/rdma/transport.cpp b/src/rdma/transport.cpp index 3df5be8d..5a757503 100644 --- a/src/rdma/transport.cpp +++ b/src/rdma/transport.cpp @@ -120,7 +120,8 @@ namespace { return conn.device().verbs().modify_qp(conn.qp(), &attr, mask) == 0; } - void applyVendorQpAttrs(const RdmaDevice &device, ibv_qp_attr &attr) + // attr is unused when neither HIPOBJ_BNXT nor HIPOBJ_IONIC is on + void applyVendorQpAttrs(const RdmaDevice &device, [[maybe_unused]] ibv_qp_attr &attr) { const std::optional vendorId = device.vendorId(); if (!vendorId) {