Repository navigation
fix(hip): stop HIP_ADD_LIBRARY from propagating -x hip to consumers - #15126
Open
david-salinas wants to merge 1 commit into
Open
david-salinas wants to merge 1 commit into
david-salinas wants to merge 1 commit into
Conversation
HIP_ADD_LIBRARY linked hip::device as PUBLIC, which propagates -x hip and --offload-arch to every target that links the library. This forces plain C++ sources in consumer targets to be compiled as HIP device code, changing semantics (e.g. clang's HIP-mode <complex> wrapper disables C99 complex functions, altering std::abs results). Link hip::device PRIVATE so only the library's own sources get -x hip, and hip::host PUBLIC so consumers inherit the HIP runtime (headers + libamdhip64) without the device compilation flags. Ref: https://amd-hub.atlassian.net/browse/ROCM-33939 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
This branch has not been deployed
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.
HIP_ADD_LIBRARY linked hip::device as PUBLIC, which propagates -x hip and --offload-arch to every target that links the library. This forces plain C++ sources in consumer targets to be compiled as HIP device code, changing semantics (e.g. clang's HIP-mode wrapper disables C99 complex functions, altering std::abs results).
Link hip::device PRIVATE so only the library's own sources get -x hip, and hip::host PUBLIC so consumers inherit the HIP runtime (headers + libamdhip64) without the device compilation flags.
Ref: https://amd-hub.atlassian.net/browse/ROCM-33939
Motivation
PR #5708 replaced the legacy FindHIP.cmake with a compatibility shim. The shim's HIP_ADD_LIBRARY macro linked hip::device as PUBLIC so consumers would inherit the HIP runtime. However, hip::device carries -x hip and --offload-arch on its interface, which forces all downstream consumer sources to be compiled as HIP device code. The old FindHIP.cmake never propagated these flags to consumers. This causes subtle correctness issues — for example, in HIP mode clang's wrapper disables C99 complex functions, so std::abs of a complex value uses libstdc++'s generic formula instead of cabs, producing results that differ in the last bit.
Technical Details
The hip::device and hip::host imported targets differ in one key way:
┌─────────────┬───────────┬────────┬─────────┐
│ Target │ -x hip │ --offload-arch │ HIP headers │ libamdhip64 │
├─────────────┼───────────┼────────┼─────────┼
│ hip::host │ No │ No │ Yes │ Yes │
├─────────────┼───────────┼────────┼─────────┼
│ hip::device │ Yes │ Yes │ Yes │ Yes │
└─────────────┴───────────┴────────┴─────────┴
The fix changes HIP_ADD_LIBRARY in projects/hip/cmake/FindHIP.cmake from:
target_link_libraries(${hip_target} PUBLIC hip::device)
to:
target_link_libraries(${hip_target} PRIVATE hip::device PUBLIC hip::host)
HIP_ADD_EXECUTABLE is unchanged — it already uses PRIVATE hip::device, which is correct since nothing consumes an executable's link interface.
Issue Tracking
JIRA ID: ROCM-33939
Test Plan
Test Result
Submission Checklist