Repository navigation
Enable flang and openmp use throughout ROCm. - #3928
stellaraccident wants to merge 26 commits into
Conversation
|
Builds are failing: |
ScottTodd
left a comment
There was a problem hiding this comment.
Overall looks good once the CI passes
|
Try 2. |
😢 |
|
what are the valid combinations of the 3 amd-llvm* can amd-llvm + amd-llvm-offload be built ? same for amd-llvm + amd-llvm-flang ? if i specify amd-llvm-flang does it imply amd-llvm ? or do i need to explicitly as for amd-llvm? |
LLVM's openmp-config.cmake exports OpenMP::omp, not OpenMP::OpenMP_CXX. When provide_package redirected find_package(OpenMP) to the LLVM config, downstream consumers (rocRoller, mxdatagenerator) that link against OpenMP::OpenMP_CXX failed at configure time. Removing the provide_package lets CMake's built-in FindOpenMP module handle detection, which correctly creates the OpenMP::OpenMP_CXX target. Fixes: #3928 (marbre report on "Try 2") Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
My agent organized my answer a bit: The three sub-projects form a dependency chain:
Valid combinations:
Dependencies are enforced via artifact_deps in the topology, so enabling amd-llvm-flang automatically pulls in amd-llvm — you don't need to request it explicitly. |
In the latest run rocprofiler-systems is unhappy about the Fortran related changes. |
5c0a394 to
9879624
Compare
|
A month later - needs more cleanup before landing but pushing a WIP branch that passes my local build to see what CI says. |
Changes: - Add amd-llvm-base as a foundation compiler artifact without Flang/OpenMP/offload. - Add optional amd-llvm-flang and amd-llvm-offload layers with flang-rt and OpenMP runtime packaging. - Wire Fortran optional subproject behavior and initial ROCm consumers. - Add tactical LLVM patches for split OpenMP/offload builds. Generated with Claude Code. Co-Authored-By: Claude <noreply@anthropic.com>
Changes: - Stop advertising LLVM OpenMP config as the global OpenMP provider. - Build hip-clr against amd-llvm-offload when that layer is enabled so the normal amd-hip toolchain carries HIP, Flang, and OpenMP together. - Keep rocprofiler-systems' LLVM OpenMP config shim local and explicit. Generated with Claude Code Co-authored-by: Claude <noreply@anthropic.com>
Changes: - Remove the foundation stage and fold sysdeps/base into compiler-runtime. - Add a runtime-tests stage for hip-tests and rocrtst in multi-stage CI. - Keep rocprofiler-systems on module-mode OpenMP hints instead of LLVM OpenMP config mode. Generated with Claude Code
baff225 to
c83335c
Compare
Changes: - keep kpacked test host binaries paired with per-ISA artifacts instead of publishing them through shared generic test artifacts - keep setuptools and wheel in setup_venv-created venvs and disable build isolation for explicit artifact-only installs - cover the no-index setup_venv command shape in unit tests Claude Code
Changes: - normalize hipblas-test's Windows process status to 0 when GTest reports success - keep quick_exit on Windows so post-main DLL cleanup still does not run Claude Code
Partial landing split from #3928. Changes: - Replace install(FILES) for the root llvm and amdgcn directory symlinks with install(CODE) that creates the symlinks at install time. - Create the install prefix inside the install code before creating root-level symlinks; local validation showed the closed #4301 form failed when the prefix did not exist yet. - Leave the existing amdflang symlink behavior unchanged; conditional Fortran symlink handling remains with the split compiler work. Rationale: - CMake install(FILES) rejects directory symlinks, which breaks aux-overlay installation for the SDK layout symlinks tracked by #3714. Validation: - cmake -S base/aux-overlay -B /tmp/therock-aux-overlay-test-build -GNinja -DTHEROCK_SOURCE_DIR=/srv/vm-shared/projects/therock-workspace1/worktrees/TheRock -DTHEROCK_PACKAGE_VERSION=0.0.0 -DGPU_TARGETS=gfx1100 - cmake --build /tmp/therock-aux-overlay-test-build - cmake --install /tmp/therock-aux-overlay-test-build --prefix /tmp/therock-aux-overlay-test-install - verified llvm, amdgcn, and bin/amdclang are symlinks in /tmp/therock-aux-overlay-test-install - git diff --cached --check Generated with Codex
Partial landing split from #3928. Changes: - When a stage depends on an artifact group, include not only the artifacts produced by that group but also each artifact's transitive artifact dependencies. - Add topology regression coverage for a group dependency where a pulled-in artifact depends on additional toolchain artifacts from the same producer group. Rationale: - Multi-stage CI fetches stage inputs from get_inbound_artifacts(). Without this closure, a downstream stage can fetch a higher-level artifact while missing lower-level artifacts that it needs at configure/build time. - This is a standalone topology correctness fix that derisks the split compiler artifacts from #3928. Validation: - python -m unittest build_tools.tests.build_topology_test - python build_tools/topology_to_cmake.py --validate-only - git diff --cached --check Generated with Codex
|
|
||
| When OpenMP is built standalone against an installed LLVM tree, | ||
| LIBOMP_HEADERS_INTDIR can point at the installed LLVM resource include | ||
| directory instead of the local runtime build directory. That makes GCC |
There was a problem hiding this comment.
shouldnt this build of openmp files always use clang rather than gcc ?
|
I wonder if we need to wire something in the cmake/therock_sanitizers.cmake to handle passing sanitizer options to flang. |
| if(TARGET FortranSupport AND TARGET MLIRBuiltinTypeInterfacesIncGen) | ||
| add_dependencies(FortranSupport MLIRBuiltinTypeInterfacesIncGen) | ||
| endif() | ||
| if(TARGET FIRSupport AND TARGET MLIRLinalgOpsIncGen) | ||
| add_dependencies(FIRSupport MLIRLinalgOpsIncGen) | ||
| endif() | ||
| if(TARGET FortranSupport) | ||
| target_include_directories(FortranSupport PRIVATE | ||
| ${CMAKE_BINARY_DIR}/flang/lib/Support) | ||
| endif() |
There was a problem hiding this comment.
Followup from today's conversation:
I think duplicating the CMakeLists.txt from upstream is highly undesirable. If changes upstream, it is hard to debug what changes and replicated it in this copy of CMakeLists.txt. FLANG_INCLUDE_QUADMATH_H is a great example for this: Build will not fail initially, but REAL(16) will be silently unavailable and some poor soul has to find out why. I am planning larger changes to this file upstream (llvm/llvm-project#171515) and would rather not have to do the same a second time for TheRock.
Upstream LLVM already has a means to build components separately in standalone builds. That is: LLVM+Flang -> MLIR -> Flang. You mentioned the disadvantage is that the MLIR build includes a lot of other stuff that is actually not needed for Flang, and a common build dir just handles dependency resolution.
This is an argument for MLIR, Flang which are build through LLVM_ENABLE_PROJECTS, but not for LLVM_ENABLE_RUNTIMES (i.e. amd-llvm-offload). A runtimes build can build multiple runtimes via passing them all to LLVM_ENABLE_RUNTIMES, e.g -DLLVM_ENABLE_RUNTIMES=openmp;offload.
For MLIR, it would be less work to only build the components actually needed, either by ninja MLIRLinalgOpsIncGen ..., creating a new target in our downstream llvm-project (such as
add_custom_target(mlir-flang-deps)
add_dependencies(mlir-flang-deps MLIRLinalgOpsIncGen ...)) that depend only on needed targets, or straight up commenting-out upstream components that we do not need. Dependencies are already managed in this CMakeLists copy and a missing dependency due to an upstream change is much more obvious than hunting down changes to configuration parameters.
If we have to manually manage out top-level dependencies anyway, why not keep everything in a single build dir and create targets that only build/install components that are needed (replacing ninja all and ninja install)? From AI:
# Install base_lib into the "base" component
install(TARGETS base_lib
DESTINATION lib
COMPONENT base)
# Create the custom target for 'ninja install-base'
add_custom_target(install-base
# 1. The command to run the component-specific install
COMMAND ${CMAKE_COMMAND} --install ${CMAKE_BINARY_DIR} --component base
# 2. The targets that MUST be built before installing
DEPENDS base_lib
COMMENT "Building and installing the 'base' component..."
)(upstream LLVM already uses COMPONENT in install commands)
or reconfigure the same build dir with LLVM_ENABLE_PROJECTS=${LLVM_ENABLE_PROJECTS};flang a second build to incrementally build flang on top of the previous build.
Partial landing split from #3928. Changes: - Remove the foundation stage and fold sysdeps/base into compiler-runtime. - Add a runtime-tests stage for hip-tests and rocrtst so runtime test builds no longer sit on the compiler-runtime critical path. - Update multi-stage workflow dependencies, helper text, docs, and CI unit-test expectations for the new stage layout. - Keep rocprofiler-systems OpenMP/example changes out of this split; those remain separate from the topology refactor. Evidence: - python build_tools/topology_to_cmake.py --validate-only - python -m pytest build_tools/tests/build_topology_test.py build_tools/github_actions/tests/configure_multi_arch_ci_test.py - python build_tools/configure_stage.py --stage compiler-runtime --print - python build_tools/configure_stage.py --stage runtime-tests --print Generated with Claude Code
Partial landing split from #3928. Changes: - Remove the foundation stage and fold sysdeps/base into compiler-runtime. - Add a runtime-tests stage for hip-tests and rocrtst so runtime test builds no longer sit on the compiler-runtime critical path. - Update multi-stage workflow dependencies, helper text, docs, and CI unit-test expectations for the new stage layout.
Partial landing split from #3928. Changes: - Keep setuptools and wheel available when setup_venv updates a venv so ROCm sdists can build without resolving build dependencies from the package index under test. - Treat an explicit empty --index-url as an artifact-only install request and pass pip --no-index --no-build-isolation while preserving --find-links. - Add unit coverage for the empty-index command shape. Validation: - python -m unittest build_tools.tests.setup_venv_test - git diff --cached --check Generated with Codex Co-authored-by: Scott Todd <Scott.Todd@amd.com>
Partial upstream split from ROCm/TheRock#3928. Changes: - Rename the local HIP C binding function result from c_int to hip_status in the rocBLAS Fortran samples. - Declare the renamed result explicitly as integer(c_int) under implicit none. Rationale: - The samples imported iso_c_binding::c_int and also used result(c_int), which makes the function result name collide with the imported kind name. - Flang diagnoses the collision and missing explicit result type; gfortran also accepts the explicit non-conflicting result name. Validation: - Applied on current rocm-libraries develop after git pull --ff-only origin develop. - gfortran -fsyntax-only on example_fortran_{axpy,gemv,scal}.f90 with temporary stub rocblas modules. - git diff --check. Generated with Codex --------- Co-authored-by: Michael Klemm <michael.klemm@amd.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Torre Zuk <Torre.Zuk@amd.com>
rocBLAS: Fix Fortran sample HIP result names Partial upstream split from ROCm/TheRock#3928. Changes: - Rename the local HIP C binding function result from c_int to hip_status in the rocBLAS Fortran samples. - Declare the renamed result explicitly as integer(c_int) under implicit none. Rationale: - The samples imported iso_c_binding::c_int and also used result(c_int), which makes the function result name collide with the imported kind name. - Flang diagnoses the collision and missing explicit result type; gfortran also accepts the explicit non-conflicting result name. Validation: - Applied on current rocm-libraries develop after git pull --ff-only origin develop. - gfortran -fsyntax-only on example_fortran_{axpy,gemv,scal}.f90 with temporary stub rocblas modules. - git diff --check. Generated with Codex
Partial upstream split from ROCm/TheRock#3928. Changes: - Rename the local HIP C binding function result from c_int to hip_status in the rocBLAS Fortran samples. - Declare the renamed result explicitly as integer(c_int) under implicit none. Rationale: - The samples imported iso_c_binding::c_int and also used result(c_int), which makes the function result name collide with the imported kind name. - Flang diagnoses the collision and missing explicit result type; gfortran also accepts the explicit non-conflicting result name. Validation: - Applied on current rocm-libraries develop after git pull --ff-only origin develop. - gfortran -fsyntax-only on example_fortran_{axpy,gemv,scal}.f90 with temporary stub rocblas modules. - git diff --check. Generated with Codex --------- Co-authored-by: Michael Klemm <michael.klemm@amd.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Torre Zuk <Torre.Zuk@amd.com>
Summary
Splits the ROCm LLVM build into a small bootstrap compiler plus optional Fortran and OpenMP/offload layers, then adjusts TheRock's build topology so downstream projects can consume the right compiler view without forcing every build to pay for Flang/offload.
The main goals are:
ROCM_BUILD_FORTRAN_LIBSflag.find_package(OpenMP).foundationstage intocompiler-runtimeso CI does not add a lightweight root stage or require a second LLVM checkout.hip-testsandrocrtstto a separateruntime-testsstage so runtime tests do not rebuild on downstream stages that only need HIP/core runtime artifacts.LLVM Layering
The LLVM-related artifacts are now layered as follows:
amd-llvm-base: bootstrap compiler layer. Builds the LLVM project withclang,lld,clang-tools-extra, X86/AMDGPU targets, compiler-rt/device-libs, and no Flang/MLIR/OpenMP/offload. This is the base compiler used by later layers.amd-llvm: compiler infrastructure layer. Buildsamd-comgrandhipccon top ofamd-llvm-base.amd-llvm-flang: optional Linux-only Flang frontend layer. Builds Flang/MLIR as implementation details needed by the frontend, but does not install MLIR into the final ROCm dist.amd-llvm-offload: optional Linux-only runtime layer. Builds OpenMP/offload runtimes and, when Flang is enabled, flang runtime pieces. Depends onamd-llvm,amd-llvm-flangwhen enabled, andcore-runtime.hip-clruses theamd-llvm-offloadcompiler view when that target exists. That keeps HIP, Flang, OpenMP headers/libraries, and offload runtime visibility together in theamd-hiptoolchain view without making the base compiler advertise OpenMP globally.Build Topology
The old
foundationstage has been removed. Its former artifact groups are now part ofcompiler-runtime:third-party-sysdepsbasecompilerThis avoids a root stage that mostly adds critical path and avoids splitting LLVM-related source checkout/build behavior across stages.
A new
runtime-testsstage contains:core-hiptestsrocrtstruntime-testsdepends on the runtime/compiler artifacts it needs and runs parallel to math-libs. Linux builds bothhip-testsandrocrtst; Windows has the same stage socore-hiptestsartifacts are still produced, whilerocrtstremains disabled by platform metadata.Fortran Controls
This adds or wires:
THEROCK_ENABLE_FLANGTHEROCK_ENABLE_AMD_LLVM_OFFLOADTHEROCK_FLAG_BUILD_FORTRAN_LIBSFORTRAN_OPTIONAL/FORTRAN_REQUIREDsubproject declaration keywordsROCM_BUILD_FORTRAN_LIBS=ON/OFFpropagation to downstream subprojectsKnown Fortran consumers can now declare whether Fortran is optional or required. When Fortran libs are disabled, optional users can configure without forcing Flang/offload into the build.
OpenMP Handling
amd-hiptoolchain view whenhip-clris built on top ofamd-llvm-offload.rocprofiler-systemsgets a targeted pre-hook that passeslibomptargetexplicitly and seeds module-modeFindOpenMPcache entries where needed.libgomppath, avoiding accidental selection of the offload package'slibgomp.soshim.Downstream Wiring
The change updates downstream projects that need Fortran/OpenMP awareness, including RAND/BLAS-family projects and
rocprofiler-systems.Notable fixes:
ROCM_BUILD_FORTRAN_LIBS.rocprofiler-systemsreceives the offload library directory explicitly forlibomptarget.rocprofiler-systemsexamples are disabled in TheRock integration builds to avoid using deep example builds as the first test of the Fortran/offload toolchain.Verification
Local verification was done on a
gfx1100machine.Static checks:
No-Fortran HIP/RAND build:
THEROCK_ENABLE_ALL=OFF,THEROCK_ENABLE_HIP_RUNTIME=ON,THEROCK_ENABLE_RAND=ON,THEROCK_ENABLE_FLANG=OFF,THEROCK_ENABLE_AMD_LLVM_OFFLOAD=OFF, andTHEROCK_FLAG_BUILD_FORTRAN_LIBS=OFF.dist-rocm.amdflang,flang,libomp.so,libomptarget.so,omp_lib.mod, orlibflang_rt*.hipcc --offload-arch=gfx1100.Fortran/offload toolchain build:
THEROCK_ENABLE_FLANG=ON,THEROCK_ENABLE_AMD_LLVM_OFFLOAD=ON, andTHEROCK_FLAG_BUILD_FORTRAN_LIBS=ON.amd-llvm-base,amd-llvm,amd-llvm-flang,amd-llvm-offload, HIP runtime, RAND, and BLAS-family artifacts.dist/rocmcontainsamdflang,flang,omp_lib.mod, flang runtime libraries,libomp.so,libomptarget.so, AMDGCN device runtime pieces, BLAS/RAND libraries, and no installed MLIR CMake package in the final ROCm dist.BLAS/RAND integration:
mxDataGenerator,hipBLASLt,rocBLAS,hipBLAS,rocRAND, andhipRAND.