Skip to content

fix(rocthrust): Set BUILD_TEST and BUILD_BENCHMARK to prev value not ON - #12391

Merged
NguyenNhuDi merged 2 commits into
developfrom
users/NguyenNhuDi/fix-rocthrust-cmake-bug
Sep 22, 2026
Merged

NguyenNhuDi merged 2 commits into
developfrom
users/NguyenNhuDi/fix-rocthrust-cmake-bug

Conversation

@NguyenNhuDi

@NguyenNhuDi NguyenNhuDi commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

While building for CUDA backend with hip-nv-dev preset, BUILD_TEST was getting set to ON even though preset sets it to OFF. This causes build errors when building benchmarks for rocthrust.

Technical Details

There was an issue where BUILD_TEST and BUILD_BENCHMARK was leaking into rocRAND build. There was a fix in prs #11045 and #10956, however these PRs just set set BUILD_TEST and BUILD_BENCHMARK to ON. In cases where only BUILD_TEST was set to ON or only BUILD_BENCHMARK was set to ON this would turn on the other no matter what. In the case of hip-nv-dev preset this would set BUILD_TEST to ON even though in this case we need it to be set OFF.

By storing the variables into temp variables before forcing them OFF we can restore the original state after rocRAND is finished.

@skip-pr-bot

Test Plan

Local compilation with hip-nv-dev preset.

Test Result

No more cmake errors.

Submission Checklist

@NguyenNhuDi
NguyenNhuDi marked this pull request as ready for review September 21, 2026 18:10
@NguyenNhuDi
NguyenNhuDi requested a review from a team as a code owner September 21, 2026 18:10
@therock-pr-bot

Copy link
Copy Markdown

✅ Author chose to skip pr bot run hence removing label (@skip-pr-bot found in the PR description).

@NguyenNhuDi NguyenNhuDi changed the title fix(rocthrust): Set to BUILD_TEST and BUILD_BENCHMARK to prev value not ON fix(rocthrust): Set BUILD_TEST and BUILD_BENCHMARK to prev value not ON Sep 21, 2026

@amd-jmahovsky amd-jmahovsky 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, but one possible cleanup

@NguyenNhuDi
NguyenNhuDi enabled auto-merge (squash) September 21, 2026 20:04
@codecov

codecov Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #12391      +/-   ##
===========================================
- Coverage    69.67%   69.67%   -0.00%     
===========================================
  Files         2932     2932              
  Lines       481712   481662      -50     
  Branches     69448    69448              
===========================================
- Hits        335630   335580      -50     
  Misses      121892   121892              
  Partials     24190    24190              
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 48.80% <ø> (ø) Carriedforward from 2acd3b0
TensileLite-Unit 76.64% <ø> (ø) Carriedforward from 2acd3b0
hipBLAS 90.62% <ø> (ø) Carriedforward from 2acd3b0
hipBLASLt 35.28% <ø> (ø) Carriedforward from 2acd3b0
hipCUB 82.68% <ø> (ø) Carriedforward from 2acd3b0
hipDNN 86.98% <ø> (ø) Carriedforward from 2acd3b0
hipFFT 54.81% <ø> (ø) Carriedforward from 2acd3b0
hipRAND 76.12% <ø> (ø) Carriedforward from 2acd3b0
hipSOLVER 68.96% <ø> (ø) Carriedforward from 2acd3b0
hipSPARSE 86.99% <ø> (ø) Carriedforward from 2acd3b0
rocBLAS 48.31% <ø> (ø) Carriedforward from 2acd3b0
rocFFT 46.27% <ø> (ø) Carriedforward from 2acd3b0
rocRAND 57.42% <ø> (ø) Carriedforward from 2acd3b0
rocSOLVER 76.83% <ø> (ø) Carriedforward from 2acd3b0
rocSPARSE 74.60% <ø> (-0.02%) ⬇️ Carriedforward from 2acd3b0
rocThrust 78.34% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@NguyenNhuDi
NguyenNhuDi merged commit 45e1aac into develop Sep 22, 2026
89 of 90 checks passed
@NguyenNhuDi
NguyenNhuDi deleted the users/NguyenNhuDi/fix-rocthrust-cmake-bug branch September 22, 2026 15:50
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

stanleytsang-amd pushed a commit that referenced this pull request Oct 2, 2026
…not `ON` (#12391)

## Motivation
While building for `CUDA` backend with `hip-nv-dev` preset, `BUILD_TEST`
was getting set to `ON` even though preset sets it to `OFF`. This causes
build errors when building benchmarks for `rocthrust`.
## Technical Details

There was an issue where `BUILD_TEST` and `BUILD_BENCHMARK` was leaking
into `rocRAND` build. There was a fix in prs #11045 and #10956, however
these PRs just set set `BUILD_TEST` and `BUILD_BENCHMARK` to `ON`. In
cases where only `BUILD_TEST` was set to `ON` or only `BUILD_BENCHMARK`
was set to `ON` this would turn on the other no matter what. In the case
of `hip-nv-dev` preset this would set `BUILD_TEST` to `ON` even though
in this case we need it to be set `OFF`.

By storing the variables into temp variables before forcing them `OFF`
we can restore the original state after `rocRAND` is finished.

@skip-pr-bot
## Test Plan
Local compilation with `hip-nv-dev` preset.

## Test Result

No more cmake errors.

## Submission Checklist

- [ ] Look over the contributing guidelines at
https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests.
Naraenda pushed a commit that referenced this pull request Oct 5, 2026
…not `ON` (#12391)

## Motivation
While building for `CUDA` backend with `hip-nv-dev` preset, `BUILD_TEST`
was getting set to `ON` even though preset sets it to `OFF`. This causes
build errors when building benchmarks for `rocthrust`.
## Technical Details

There was an issue where `BUILD_TEST` and `BUILD_BENCHMARK` was leaking
into `rocRAND` build. There was a fix in prs #11045 and #10956, however
these PRs just set set `BUILD_TEST` and `BUILD_BENCHMARK` to `ON`. In
cases where only `BUILD_TEST` was set to `ON` or only `BUILD_BENCHMARK`
was set to `ON` this would turn on the other no matter what. In the case
of `hip-nv-dev` preset this would set `BUILD_TEST` to `ON` even though
in this case we need it to be set `OFF`.

By storing the variables into temp variables before forcing them `OFF`
we can restore the original state after `rocRAND` is finished.

@skip-pr-bot
## Test Plan
Local compilation with `hip-nv-dev` preset.

## Test Result

No more cmake errors.

## Submission Checklist

- [ ] Look over the contributing guidelines at
https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests.
Naraenda pushed a commit that referenced this pull request Oct 7, 2026
…not `ON` (#12391)

## Motivation
While building for `CUDA` backend with `hip-nv-dev` preset, `BUILD_TEST`
was getting set to `ON` even though preset sets it to `OFF`. This causes
build errors when building benchmarks for `rocthrust`.
## Technical Details

There was an issue where `BUILD_TEST` and `BUILD_BENCHMARK` was leaking
into `rocRAND` build. There was a fix in prs #11045 and #10956, however
these PRs just set set `BUILD_TEST` and `BUILD_BENCHMARK` to `ON`. In
cases where only `BUILD_TEST` was set to `ON` or only `BUILD_BENCHMARK`
was set to `ON` this would turn on the other no matter what. In the case
of `hip-nv-dev` preset this would set `BUILD_TEST` to `ON` even though
in this case we need it to be set `OFF`.

By storing the variables into temp variables before forcing them `OFF`
we can restore the original state after `rocRAND` is finished.

@skip-pr-bot
## Test Plan
Local compilation with `hip-nv-dev` preset.

## Test Result

No more cmake errors.

## Submission Checklist

- [ ] Look over the contributing guidelines at
https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants