[RPP] Add rocFFT GPU acceleration for audio spectrogram (AICV-283) - #11496
Draft
LakshmiKumar23 wants to merge 17 commits into
Draft
[RPP] Add rocFFT GPU acceleration for audio spectrogram (AICV-283)#11496LakshmiKumar23 wants to merge 17 commits into
LakshmiKumar23 wants to merge 17 commits into
Conversation
…arty ffts library
…_ffts_host_implementation
- Created FindROCFFT.cmake module for rocFFT library detection - Modified CMakeLists.txt to detect rocFFT for HIP backend - Enable audio support with either FFTS (CPU) or rocFFT (HIP) - Link rocFFT only for HIP backend, FFTS only for CPU backend - Set RPP_USE_ROCFFT_LIB=1 when rocFFT is found Co-Authored-By: Claude <noreply@anthropic.com>
Combined changes from users/rrawther/rpp_ffts_host_implementation with our rocFFT GPU implementation: - CPU backend: FFTS library OR in-house FFT (PR #11146) - HIP backend: rocFFT library (our addition) - Both backends now have library-accelerated FFT support - Resolved merge conflicts in CMakeLists.txt Co-Authored-By: Claude <noreply@anthropic.com>
- Fixed FindROCFFT.cmake to search for rocfft.h with PATH_SUFFIXES - Removed 'manual DFT' reference from rocFFT not found message - Verified: librpp.so successfully links librocfft.so.0 Co-Authored-By: Claude <noreply@anthropic.com>
Implements Phase 2 of rocFFT integration: code implementation in spectrogram.cpp Changes: - Add rocFFT header with conditional compilation guard - New kernel: compute_magnitude_from_complex_hip_tensor * Non-vertical: Direct coalesced writes (TF layout) * Vertical: 16x16 shared memory transpose (FT layout) * Supports power=1 (magnitude) and power=2 (squared) - Add rocFFT execution path in hip_exec_spectrogram_tensor * Real-to-complex 1D batch FFT using rocFFT library * Proper resource management and error handling * Specific RPP error codes for different failure scenarios - Preserve manual DFT code as fallback under #else Performance: rocFFT provides O(N log N) vs O(N^2) manual DFT Expected speedup: 3-30x depending on nfft size Build verification: - Clean compile with no warnings - librocfft.so.0 successfully linked - All rocFFT symbols present in librpp.so Co-Authored-By: Claude <noreply@anthropic.com>
Moves rocfft_setup() and rocfft_cleanup() from per-call (in spectrogram) to per-handle lifecycle (Handle constructor/destructor). Before: rocfft_setup/cleanup called on every spectrogram invocation After: Called once when Handle is created/destroyed Performance improvement: ~10% reduction in overhead (143ms → 128ms) Note: rocFFT plan creation/destruction still occurs per-call and is the main performance bottleneck (36x slower than manual DFT for small nfft=512). Future work: implement plan caching for better performance. Co-Authored-By: Claude <noreply@anthropic.com>
Cache rocFFT plans in Handle to avoid expensive plan creation/destruction on every spectrogram call. Plans are keyed by (nfft, batchCount) composite key and destroyed when the Handle is released. Performance improvement: spectrogram now runs 1.6x faster than manual DFT (2.11ms vs 3.37ms avg per batch). - Add rocfft_plan_cache map to HandleImpl (keyed by nfft << 32 | batchCount) - Add get_rocfft_plan() helper to create/retrieve cached plans - Fix namespace mismatch (declaration now in namespace rpp) - Fix AUDIO_SUPPORT preprocessor guard Co-Authored-By: Claude <noreply@anthropic.com>
- Bump version from 3.2.0 to 3.3.0 for rocFFT integration - Enable RPP_AUDIO_SUPPORT by default (no external FFT dependency) - Add changelog entry for rocFFT GPU acceleration and in-house CPU FFT - Update README and build docs to reflect new defaults and FFT backends Co-Authored-By: Claude <noreply@anthropic.com>
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
Pre-commit check failed⛔ pre-commit failed Please run locally:
This repo uses |
- Reformat function signatures to meet line length requirements - Add blank line after include directives - Consolidate single-line if statements Co-Authored-By: Claude <noreply@anthropic.com>
- Break long comment lines - Reformat long assignment statements - Adjust hipMemsetAsync formatting Co-Authored-By: Claude <noreply@anthropic.com>
Apply clang-format to fix all remaining formatting issues: - Line length compliance - Proper indentation alignment - Consistent spacing Co-Authored-By: Claude <noreply@anthropic.com>
|
🎉 All checks passed! This PR is ready for review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds rocFFT library support for GPU-accelerated FFT in the RPP HIP backend audio spectrogram, replacing the O(N²) manual DFT implementation with O(N log N) rocFFT library calls. This is the GPU counterpart to PR #11146's CPU FFT work.
JIRA: AICV-283
Dependencies
Background
CPU Side (HOST Backend) - PR #11146
GPU Side (HIP Backend) - This PR
compute_coefficients_hip_tensor+fourier_transform_hip_tensorKey Note: CPU and GPU FFT implementations were always separate - this PR only touches the HIP backend.
Performance Results
Speedup: 1.6x over manual DFT implementation (with plan caching optimization)
Changes Made
1. CMake Infrastructure
projects/rpp/cmake/FindROCFFT.cmake- CMake finder module for rocFFTrocfft::rocfftprojects/rpp/CMakeLists.txtfind_package(ROCFFT QUIET)-DRPP_USE_ROCFFT2. Code Implementation
projects/rpp/src/modules/tensor/hip/kernel/spectrogram.cpp#ifdef RPP_USE_ROCFFTcompute_magnitude_from_complex_hip_tensor#elsefallback (will be removed after validation period)3. Performance Optimization
projects/rpp/src/include/rpp_handle.hpprocfft_setup()/cleanup()from per-call to Handle ctor/dtorget_rocfft_plan()helper for plan creation/retrieval4. Documentation
projects/rpp/CHANGELOG.mdprojects/rpp/CMakeLists.txtprojects/rpp/README.mdanddocs/install/rpp-build.rstTesting
Test Results
Test Coverage
Build Validation
Works with both rocFFT present and absent:
Future Work
Phase 6: Manual DFT Removal (planned after 1-2 release cycles)
compute_coefficients_hip_tensorandfourier_transform_hip_tensorkernels#ifdef RPP_USE_ROCFFTconditional blocksFiles Changed
New Files:
projects/rpp/cmake/FindROCFFT.cmakeModified Files:
projects/rpp/CMakeLists.txt- FFT detection, linking, version bumpprojects/rpp/src/modules/tensor/hip/kernel/spectrogram.cpp- rocFFT implementationprojects/rpp/src/include/rpp_handle.hpp- Plan cachingprojects/rpp/CHANGELOG.md- Release notesprojects/rpp/README.md- Build documentationprojects/rpp/docs/install/rpp-build.rst- Build documentationChecklist