Skip to content

fix(monte_carlo_pi): migrate off hipcub iterators removed in CUB 3.0 - #493

Merged
lamb-j merged 2 commits into
amd-stagingfrom
lamb-j/monte-carlo-pi-cccl3-iterators
Jul 27, 2026
Merged

fix(monte_carlo_pi): migrate off hipcub iterators removed in CUB 3.0#493
lamb-j merged 2 commits into
amd-stagingfrom
lamb-j/monte-carlo-pi-cccl3-iterators

Conversation

@lamb-j

@lamb-j lamb-j commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Problem

Applications/monte_carlo_pi fails to build against current hipCUB:

main.hip:28:10: fatal error: 'hipcub/iterator/counting_input_iterator.hpp' file not found

hipCUB's CCCL 3.0 update (ROCm/rocm-libraries#9931, merged to develop 2026-07-25) removed the CUB-style iterator headers (counting_input_iterator, constant_input_iterator, transform_input_iterator, discard_output_iterator). CUB 3.0 dropped these in favor of the portable Thrust iterators. monte_carlo_pi was the only example still including them.

Fix

  • main.hip: replace hipcub::CountingInputIterator / hipcub::TransformInputIterator with thrust::counting_iterator / thrust::make_transform_iterator; drop the unused discard_output_iterator include. hipcub::DeviceReduce::Sum is unchanged — it accepts Thrust iterators (the CCCL-recommended pattern).
  • CMakeLists.txt: find + link rocThrust on the AMD path (roc::rocthrust). On NVIDIA the Thrust headers ship with the already-required CUDA Toolkit, so both platforms keep building.

hipCUB's CCCL 3.0 update (ROCm/rocm-libraries#9931) removed the CUB-style
iterator headers (counting/constant/transform/discard) that
Applications/monte_carlo_pi included, breaking its build against current
hipCUB. Switch to the portable Thrust iterators, which is the replacement
CUB 3.0 itself points to.

Changes:
- main.hip: replace hipcub::CountingInputIterator / TransformInputIterator
  with thrust::counting_iterator / thrust::make_transform_iterator; drop the
  unused discard_output_iterator include. hipcub::DeviceReduce::Sum is
  unchanged and accepts Thrust iterators.
- CMakeLists.txt: find + link rocThrust on the AMD path (roc::rocthrust);
  NVIDIA gets the Thrust headers from the already-required CUDA Toolkit.

@idubinov idubinov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread Applications/monte_carlo_pi/main.hip Outdated
Co-authored-by: idubinov <idubinov@amd.com>
lamb-j added a commit to ROCm/llvm-project that referenced this pull request Jul 27, 2026
## Why

`Applications/monte_carlo_pi` was migrated off the CUB-style hipcub
iterators that CCCL 3.0 removed (ROCm/rocm-libraries#9931) to the
portable `thrust::` iterators — see ROCm/rocm-examples#493. That
migration pulls in a rocThrust dependency (`find_package(rocthrust
REQUIRED)`), which the SPIRV CI build job doesn't currently install, so
the `Test rocm-examples` job would fail at configure with `Could not
find rocthrust`.

## What

Add a rocThrust checkout + install into the staging tree so the
rocm-examples build can find it:

- Add `ROCTHRUST_BUILD` env var.
- Checkout `ROCm/rocThrust@develop`, configure + install into staging
after hipCUB. rocThrust is header-only, so it's a fast configure +
install, mirroring the hipCUB step.
- Include the build dir in the strip + tar steps.

## Ordering

The `Test rocm-examples` job stays red until **both** this and
rocm-examples#493 land (either order); then the next `amd-staging` run
goes green. The job is informational (not a required check), so this
doesn't block merges in the meantime.

@zichguan-amd zichguan-amd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM thanks! rocprof failures can be ignored

@lamb-j
lamb-j merged commit f142531 into amd-staging Jul 27, 2026
13 of 16 checks passed
mapatel-amd added a commit that referenced this pull request Aug 7, 2026
* Cleanup arch filtering & Fix LLVM library path (#492)

* Filter rocprof-compute to supported archs instead of skipping

Previously rocprof-compute skipped the entire example if any requested
architecture was unsupported. Use the shared filter_hip_architectures
helper to build for the supported subset and emit a warning listing the
skipped architectures, matching the ComposableKernel/hipTensor examples.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

* Warn instead of silently noting when CK/rocAL/rocCV are missing

ComposableKernel, rocAL, and rocCV emitted a STATUS message when the
library could not be found, so their examples were skipped without a
visible warning in CI. Use WARNING to match every other library and
surface the skip.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

* Fix LLVM lib/bin paths in TheRock CI env setup

The LLVM toolchain (including libomp.so) lives under ${ROCM_PATH}/lib/llvm,
not ${ROCM_PATH}/llvm. Correct PATH and LD_LIBRARY_PATH for both the tarball
and wheel install methods so runtime dependencies like libomp.so resolve.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4 <noreply@anthropic.com>

* fix(monte_carlo_pi): migrate off hipcub iterators removed in CUB 3.0 (#493)

* fix(monte_carlo_pi): migrate off hipcub iterators removed in CUB 3.0

hipCUB's CCCL 3.0 update (ROCm/rocm-libraries#9931) removed the CUB-style
iterator headers (counting/constant/transform/discard) that
Applications/monte_carlo_pi included, breaking its build against current
hipCUB. Switch to the portable Thrust iterators, which is the replacement
CUB 3.0 itself points to.

Changes:
- main.hip: replace hipcub::CountingInputIterator / TransformInputIterator
  with thrust::counting_iterator / thrust::make_transform_iterator; drop the
  unused discard_output_iterator include. hipcub::DeviceReduce::Sum is
  unchanged and accepts Thrust iterators.
- CMakeLists.txt: find + link rocThrust on the AMD path (roc::rocthrust);
  NVIDIA gets the Thrust headers from the already-required CUDA Toolkit.

* Apply suggestion from @idubinov

Co-authored-by: idubinov <idubinov@amd.com>

---------

Co-authored-by: idubinov <idubinov@amd.com>

* Add unified manifest-driven skip mechanism for CI builds and tests (#494)

* Add unified manifest-driven skip mechanism for CI builds and tests

Introduce a single source of truth (.github/build_tools/skip_manifest.py)
feeding all four skip consumers: ctest (skip_tests.txt), CMake build
(Common/SkipExamples.cmake via CMAKE_PROJECT_INCLUDE_BEFORE), make build
(SKIP_FROM_BUILD), and make test (SKIP_FROM_TEST). Migrates the previously
scattered static Makefile skips (hipFFT/rocFFT callback, all rocDecode
examples) into the manifest.

Skips carry repo-relative paths so a shared leaf name like `callback`
(present under hipFFT, rocFFT, and rocProfiler-SDK/counter_collection) only
skips the intended example. A --channel {stable,nightly} flag plus per-entry
`channels` filter scopes a skip to the pinned rocm:7.14 native workflows or
the TheRock multi-arch nightly workflow independently.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

* ci: dedupe CI step summary sections

Drop the redundant "## Skipped tests" block from the Run tests step: the
Generate skip lists step already reports the full skip manifest (scope +
reason) to the step summary, so cat-ing skip_tests.txt only re-listed the
two ctest names.

Drop the duplicate "## ROCm Version" echo from the Wheel sanity check step;
setup_whl_env.py already emits the labeled version line and feeds the
rocm_version output used for artifact naming. The sanity-check step's
rocm_version output was unreferenced.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

* ci: add install-method skip filter to distinguish nightly whl vs tarball

The nightly whl and tarball installs share the "nightly" channel but ship
different payloads, so channel alone can't scope a skip to one of them. Add
an orthogonal install_methods filter (matched against --install-method, passed
from matrix.install_method) and scope the rocDecode make-test skip to the whl
install only -- the stable image and nightly tarball carry the video data, so
their tests run.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4 <noreply@anthropic.com>

* Add nightly GPU build+test on pinned stable ROCm 7.14 image (#495)

## Motivation

Add nightly stable version testing.

## Technical Details

Adds a "preinstalled" install method and a stable_release distro (the self-contained ghcr rocm:7.14 image with ROCm baked in at /opt/rocm) so the nightly matrix exercises the pinned stable ROCm on GPU hardware, not just the TheRock nightlies. CK and OpenMP are enabled for this job.

Disables CK due to version skew.

Add venv and install cmake from pip on the stable docker image.

---------

Co-authored-by: Zichuan Guan <zichuan.guan@amd.com>
Co-authored-by: Claude Opus 4 <noreply@anthropic.com>
Co-authored-by: Jacob Lambert <jacob.lambert@amd.com>
Co-authored-by: idubinov <idubinov@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants