-
Notifications
You must be signed in to change notification settings - Fork 201
Users/todavis/aocl combined testing #4439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
tony-davis
wants to merge
22
commits into
develop
Choose a base branch
from
users/todavis/aocl-combined-testing
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
+86
−4
Conversation
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
Add find_package(aocl) support to rocBLAS client BLAS detection logic. This allows CMake-based build systems (e.g., TheRock) to provide AOCL via standard CMake package configuration, while maintaining backward compatibility with install.sh and system installations. Search order: 1. BUILD_DIR/deps/aocl (install.sh builds) 2. find_package(aocl) (CMake package builds) 3. /opt/AMD/aocl system paths (fallback) 4. AOCL 4.x BLIS libraries (legacy fallback)
Reorder AOCL detection logic to check find_package(aocl) first, before falling back to BUILD_DIR and system paths. This prioritizes CMake-based build systems (e.g., TheRock) which explicitly provide AOCL via package configuration, while maintaining backward compatibility with legacy builds.
Improve robustness of find_package(aocl) implementation based on PR review: 1. Package name case sensitivity: Try AOCL (uppercase) first, then fall back to aocl (lowercase) for compatibility with different CMake configs 2. IMPORTED_LOCATION fallback: Many CMake packages only set config-specific properties (IMPORTED_LOCATION_RELEASE/DEBUG). Add fallback chain to handle this common pattern and prevent silent failures 3. Include directory normalization: INTERFACE_INCLUDE_DIRECTORIES may return a semicolon-separated list. Normalize to first entry for compatibility with downstream single-path usage 4. Version-agnostic logging: Remove hardcoded "5.x" from status message since package detection doesn't verify AOCL version These changes prevent silent fallback to legacy detection paths when AOCL is provided via CMake package but has non-standard property configurations.
Extract find_package(aocl) logic into a find_aocl_cmake_package() helper function to improve code organization and maintainability. This follows the existing pattern established by find_libblis() in this file. Changes: - Created find_aocl_cmake_package() function (lines 66-102) - Encapsulates all CMake package detection and property extraction logic - Main detection flow simplified to single function call - Matches code style and documentation patterns of find_libblis() Benefits: - Improved readability: main detection flow is now 3 lines vs 32 lines - Consistent with project conventions: mirrors find_libblis pattern - Better maintainability: logic is isolated and reusable - Clearer separation of concerns: helper functions vs detection flow Addresses PR feedback from @TorreZuk requesting extraction into helper function similar to find_libblis.
…vention Follow CMake best practices for package naming (per Kitware PSA): - Prioritize find_package(AOCL) with AOCL::AOCL target (standard for acronyms) - Maintain backward compatibility with AOCL::aocl and find_package(aocl) - Use variable to handle different target names cleanly This matches standard CMake patterns like BLAS::BLAS, LAPACK::LAPACK, ZLIB::ZLIB and aligns with CMake 3.31+ Common Package Specification requirements. Reference: https://www.kitware.com/psa-your-package-name-and-target-namespace-should-match/
…thub.com/ROCm/rocm-libraries into users/todavis/rocblas-aocl-cmake-package
Add find_package(aocl) support to rocBLAS client BLAS detection logic. This allows CMake-based build systems (e.g., TheRock) to provide AOCL via standard CMake package configuration, while maintaining backward compatibility with install.sh and system installations. Search order: 1. BUILD_DIR/deps/aocl (install.sh builds) 2. find_package(aocl) (CMake package builds) 3. /opt/AMD/aocl system paths (fallback) 4. AOCL 4.x BLIS libraries (legacy fallback)
Reorder AOCL detection logic to check find_package(aocl) first, before falling back to BUILD_DIR and system paths. This prioritizes CMake-based build systems (e.g., TheRock) which explicitly provide AOCL via package configuration, while maintaining backward compatibility with legacy builds.
Improve robustness of find_package(aocl) implementation based on PR review: 1. Package name case sensitivity: Try AOCL (uppercase) first, then fall back to aocl (lowercase) for compatibility with different CMake configs 2. IMPORTED_LOCATION fallback: Many CMake packages only set config-specific properties (IMPORTED_LOCATION_RELEASE/DEBUG). Add fallback chain to handle this common pattern and prevent silent failures 3. Include directory normalization: INTERFACE_INCLUDE_DIRECTORIES may return a semicolon-separated list. Normalize to first entry for compatibility with downstream single-path usage 4. Version-agnostic logging: Remove hardcoded "5.x" from status message since package detection doesn't verify AOCL version These changes prevent silent fallback to legacy detection paths when AOCL is provided via CMake package but has non-standard property configurations.
Extract find_package(aocl) logic into a find_aocl_cmake_package() helper function to improve code organization and maintainability. This follows the existing pattern established by find_libblis() in this file. Changes: - Created find_aocl_cmake_package() function (lines 66-102) - Encapsulates all CMake package detection and property extraction logic - Main detection flow simplified to single function call - Matches code style and documentation patterns of find_libblis() Benefits: - Improved readability: main detection flow is now 3 lines vs 32 lines - Consistent with project conventions: mirrors find_libblis pattern - Better maintainability: logic is isolated and reusable - Clearer separation of concerns: helper functions vs detection flow Addresses PR feedback from @TorreZuk requesting extraction into helper function similar to find_libblis.
…vention Follow CMake best practices for package naming (per Kitware PSA): - Prioritize find_package(AOCL) with AOCL::AOCL target (standard for acronyms) - Maintain backward compatibility with AOCL::aocl and find_package(aocl) - Use variable to handle different target names cleanly This matches standard CMake patterns like BLAS::BLAS, LAPACK::LAPACK, ZLIB::ZLIB and aligns with CMake 3.31+ Common Package Specification requirements. Reference: https://www.kitware.com/psa-your-package-name-and-target-namespace-should-match/
This change enables build systems to inject HIP include paths through the CXXFLAGS environment variable, fixing compilation issues when HIP headers are not in default locations. Changes: - TensileConfig.cmake: Set CXXFLAGS with HIP include path from hip_DIR - SourceCommands.py: Respect CXXFLAGS in _compileSourceObjectFile This allows TheRock and other build systems to provide custom include paths for HIP headers during Tensile kernel compilation. Co-authored-by: Cursor <cursoragent@cursor.com>
Add find_package(aocl) support to rocBLAS client BLAS detection logic. This allows CMake-based build systems (e.g., TheRock) to provide AOCL via standard CMake package configuration, while maintaining backward compatibility with install.sh and system installations. Search order: 1. BUILD_DIR/deps/aocl (install.sh builds) 2. find_package(aocl) (CMake package builds) 3. /opt/AMD/aocl system paths (fallback) 4. AOCL 4.x BLIS libraries (legacy fallback)
Reorder AOCL detection logic to check find_package(aocl) first, before falling back to BUILD_DIR and system paths. This prioritizes CMake-based build systems (e.g., TheRock) which explicitly provide AOCL via package configuration, while maintaining backward compatibility with legacy builds.
Improve robustness of find_package(aocl) implementation based on PR review: 1. Package name case sensitivity: Try AOCL (uppercase) first, then fall back to aocl (lowercase) for compatibility with different CMake configs 2. IMPORTED_LOCATION fallback: Many CMake packages only set config-specific properties (IMPORTED_LOCATION_RELEASE/DEBUG). Add fallback chain to handle this common pattern and prevent silent failures 3. Include directory normalization: INTERFACE_INCLUDE_DIRECTORIES may return a semicolon-separated list. Normalize to first entry for compatibility with downstream single-path usage 4. Version-agnostic logging: Remove hardcoded "5.x" from status message since package detection doesn't verify AOCL version These changes prevent silent fallback to legacy detection paths when AOCL is provided via CMake package but has non-standard property configurations.
Extract find_package(aocl) logic into a find_aocl_cmake_package() helper function to improve code organization and maintainability. This follows the existing pattern established by find_libblis() in this file. Changes: - Created find_aocl_cmake_package() function (lines 66-102) - Encapsulates all CMake package detection and property extraction logic - Main detection flow simplified to single function call - Matches code style and documentation patterns of find_libblis() Benefits: - Improved readability: main detection flow is now 3 lines vs 32 lines - Consistent with project conventions: mirrors find_libblis pattern - Better maintainability: logic is isolated and reusable - Clearer separation of concerns: helper functions vs detection flow Addresses PR feedback from @TorreZuk requesting extraction into helper function similar to find_libblis.
…vention Follow CMake best practices for package naming (per Kitware PSA): - Prioritize find_package(AOCL) with AOCL::AOCL target (standard for acronyms) - Maintain backward compatibility with AOCL::aocl and find_package(aocl) - Use variable to handle different target names cleanly This matches standard CMake patterns like BLAS::BLAS, LAPACK::LAPACK, ZLIB::ZLIB and aligns with CMake 3.31+ Common Package Specification requirements. Reference: https://www.kitware.com/psa-your-package-name-and-target-namespace-should-match/
Fix compilation errors when HIP headers are not in default system paths by explicitly adding HIP include directories to compilation commands. Changes: - device-library/CMakeLists.txt: Pass CXXFLAGS with HIP includes to Tensile - matrix-transform/CMakeLists.txt: Add HIP include dirs using generator expression - tensilelite Component.py: Respect CXXFLAGS environment variable in Compiler class This enables TheRock and other build systems to successfully compile hipBLASlt device libraries when using custom HIP installations. Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (47.97%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #4439 +/- ##
============================================
- Coverage 65.33% 47.97% -17.36%
============================================
Files 1577 366 -1211
Lines 242154 51948 -190206
Branches 33912 6025 -27887
============================================
- Hits 158201 24919 -133282
+ Misses 69945 24418 -45527
+ Partials 14008 2611 -11397
🚀 New features to boost your workflow:
|
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.
Motivation
Technical Details
Test Plan
Test Result
Submission Checklist