Skip to content

[rocThrust] Fix google benchmark compile issue on Windows#643

Merged
NguyenNhuDi merged 5 commits into
ROCm:developfrom
NguyenNhuDi:zenguyen-rocthrust/fix-window-build-benchmark-compile-errors
Jul 16, 2025
Merged

[rocThrust] Fix google benchmark compile issue on Windows#643
NguyenNhuDi merged 5 commits into
ROCm:developfrom
NguyenNhuDi:zenguyen-rocthrust/fix-window-build-benchmark-compile-errors

Conversation

@NguyenNhuDi
Copy link
Copy Markdown
Contributor

On some Windows system the compiler used for building google benchmark (cl) is not available. We will switch to using clang++ instead.

@NguyenNhuDi
Copy link
Copy Markdown
Contributor Author

@Naraenda or @NB4444 could you try on Stream to see if this breaks anything on your end?

Comment thread projects/rocthrust/cmake/Dependencies.cmake Outdated
@umfranzw
Copy link
Copy Markdown
Contributor

Just curious, is Visual Studio installed on the machines that this issue is happening on - or is CMake just failing to find the cl executable? It looks like we do list Visual Studio as a requirement in the readme. Though I guess either way, if we can just use clang++ now, that would probably be preferable, since it cuts down on the number of extra requirements on Windows.

@NguyenNhuDi
Copy link
Copy Markdown
Contributor Author

I think CMAKE is failing to find the cl compiler and is not related to vscode. Although I do not know why vscode is a requirement as its possible to compile and run rocthrust unit tests and benchmarks just on powershell.

@NguyenNhuDi NguyenNhuDi requested a review from a team as a code owner July 15, 2025 21:30
@umfranzw umfranzw requested a review from Naraenda July 16, 2025 00:11
umfranzw
umfranzw previously approved these changes Jul 16, 2025
Copy link
Copy Markdown
Contributor

@umfranzw umfranzw left a comment

Choose a reason for hiding this comment

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

Thanks - ok, if this passes CI, then I think that's good enough for me.

Comment thread projects/rocthrust/CHANGELOG.md Outdated
spolifroni-amd
spolifroni-amd previously approved these changes Jul 16, 2025
Copy link
Copy Markdown
Contributor

@spolifroni-amd spolifroni-amd left a comment

Choose a reason for hiding this comment

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

I made a small change to the wording in the changelog. It's not much and isn't super necessary; it just makes it smoother. Approving.

Co-authored-by: spolifroni-amd <Sandra.Polifroni@amd.com>
@NguyenNhuDi NguyenNhuDi dismissed stale reviews from spolifroni-amd and umfranzw via b629146 July 16, 2025 15:22
@Naraenda
Copy link
Copy Markdown
Member

@Naraenda or @NB4444 could you try on Stream to see if this breaks anything on your end?

Our Windows CI is currently offline, but this seems simple enough for us to work around if we need to. We'll let you know if anything needs to change (or we'll create a PR).

Poking @cenxuantian who is also working on this part of the CI on our end.

Comment thread projects/rocthrust/CHANGELOG.md
@NguyenNhuDi NguyenNhuDi merged commit aae95d7 into ROCm:develop Jul 16, 2025
9 of 11 checks passed
assistant-librarian Bot pushed a commit to ROCm/rocThrust that referenced this pull request Jul 16, 2025
[rocThrust] Fix google benchmark compile issue on Windows (#643)

On some Windows system the compiler used for building google benchmark
(`cl`) is not available. We will switch to using `clang++` instead.

---------

Co-authored-by: spolifroni-amd <Sandra.Polifroni@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.

4 participants