Skip to content

Set flags for FFT math libs on Windows.#339

Merged
ScottTodd merged 2 commits into
mainfrom
users/scotttodd/windows-math-fft
Apr 7, 2025
Merged

Set flags for FFT math libs on Windows.#339
ScottTodd merged 2 commits into
mainfrom
users/scotttodd/windows-math-fft

Conversation

@ScottTodd
Copy link
Copy Markdown
Member

@ScottTodd ScottTodd commented Apr 2, 2025

Progress on #36.

Now I can build librocfft.lib and hipfft.lib with -DTHEROCK_ENABLE_FFT=ON on Windows.

Comment thread math-libs/CMakeLists.txt Outdated
Comment on lines +155 to +157
# TODO(#36): enable once `rocfft_aot_helper.exe` can access hiprtc0605.dll
# "The code execution cannot proceed because hiprtc0605.dll was not found."
set(_kernel_cache_enable "OFF")
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The hiprtc0605.dll file exists at TheRock\build\core\clr\dist\bin and is later copied to TheRock\build\math-libs\rocFFT\dist\bin, but it is needed by TheRock\build\math-libs\rocFFT\build\library\src\rocfft_aot_helper.exe during the build, here: https://github.com/ROCm/rocFFT/blob/ee8a6666b414b7db75900cabcf269c88bb9176ed/library/src/CMakeLists.txt#L522-L527

if ( ROCFFT_KERNEL_CACHE_ENABLE )
  # ...
  add_custom_command(
    OUTPUT rocfft_kernel_cache.db
    COMMAND ${CMAKE_COMMAND} -E env "LD_LIBRARY_PATH=$ENV{LD_LIBRARY_PATH}:${ROCM_PATH}/${CMAKE_INSTALL_LIBDIR}" "${CMAKE_CURRENT_BINARY_DIR}/rocfft_aot_helper" \"${ROCFFT_BUILD_KERNEL_CACHE_PATH}\" ${ROCFFT_KERNEL_CACHE_PATH} $<TARGET_FILE:rocfft_rtc_helper> ${GPU_TARGETS_AOT}
    DEPENDS rocfft_aot_helper rocfft_rtc_helper
    COMMENT "Compile kernels into shipped cache file"
  )

Maybe we can build off of -E env "LD_LIBRARY_PATH=$ENV{LD_LIBRARY_PATH}:${ROCM_PATH}/${CMAKE_INSTALL_LIBDIR}"

@ScottTodd ScottTodd requested a review from marbre April 2, 2025 22:55
Base automatically changed from users/scotttodd/windows-math-rand-with-clr to main April 3, 2025 15:30
@ScottTodd ScottTodd requested a review from amd-justchen April 3, 2025 23:19
Copy link
Copy Markdown
Member

@marbre marbre left a comment

Choose a reason for hiding this comment

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

No strong feelings about the naming, just something to eventually keep in mind.

Comment thread math-libs/CMakeLists.txt Outdated
Comment thread math-libs/CMakeLists.txt Outdated
@ScottTodd ScottTodd merged commit 802da09 into main Apr 7, 2025
@ScottTodd ScottTodd deleted the users/scotttodd/windows-math-fft branch April 7, 2025 19:06
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Apr 7, 2025
ScottTodd added a commit that referenced this pull request Apr 7, 2025
Redo of #350 now that
#339 is merged.

---

Progress on #36.

Tests for rocPRIM run on my gfx1100 dev machine:
https://gist.github.com/ScottTodd/5c33611469aa1df9d39b816acecbb7f0.

### Warnings

I silenced a bunch of warnings that were getting in my way and added
some notes to #47 about the
remaining warnings since the build logs for rocPRIM were 45MB.

Some warnings will be fixed with newer subproject code. rocPRIM in
particular is 2 months outdated on `mainline` compared to `develop`, and
commits like

ROCm/rocPRIM@678701d
fix warnings by removing code like
```C++
    #ifdef _WIN32
        #define ROCPRIM_KERNEL __global__ static
    #else
        #define ROCPRIM_KERNEL __global__
    #endif
```
that had been causing warnings:
```
16.8	D:/projects/TheRock/math-libs/rocPRIM/rocprim/include\rocprim\device/device_find_first_of.hpp:53:12: warning: duplicate 'static' declaration specifier [-Wduplicate-decl-specifier]
16.8	   53 |     static ROCPRIM_KERNEL
16.8	      |            ^
16.8	D:/projects/TheRock/math-libs/rocPRIM/rocprim/include\rocprim\config.hpp:46:43: note: expanded from macro 'ROCPRIM_KERNEL'
16.8	   46 |         #define ROCPRIM_KERNEL __global__ static
16.8	      |                                           ^
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants