Skip to content

cleanup and clarification of unified/managed memory docs#3819

Closed
briankoco wants to merge 0 commit intodocs/developfrom
bjk-dev
Closed

cleanup and clarification of unified/managed memory docs#3819
briankoco wants to merge 0 commit intodocs/developfrom
bjk-dev

Conversation

@briankoco
Copy link
Copy Markdown
Member

Associated JIRA ticket number/Github issue number

N/A

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Continuous Integration

What were the changes?

  • Explain concept of managed memory in relation to unified memory
  • Clarify that managed memory via system allocators is not officially supported on CDNA1
  • Fix hipHostRegister() managed memory table entries

Why are these changes needed?

Some of the unified memory documentation is incorrect, and performance implications of managed memory were not sufficiently captured

Updated CHANGELOG?

  • Yes
  • No, Does not apply to this PR.

Added/Updated documentation?

  • Yes
  • No, Does not apply to this PR.

Additional Checks

  • I have added tests relevant to the introduced functionality, and the unit tests are passing locally.
  • Any dependent changes have been merged.

@briankoco briankoco added ci:docs-only Only run Read the Docs CI on this PR documentation labels Jul 15, 2025
@briankoco briankoco requested review from adeljo-amd and neon60 July 15, 2025 13:11
@adeljo-amd adeljo-amd requested a review from randyh62 July 15, 2025 13:13
@adeljo-amd
Copy link
Copy Markdown
Contributor

LGTM, I don't have access to approving PRs here. I've invited @randyh62 to also take a look

Comment thread docs/how-to/hip_runtime_api/memory_management/unified_memory.rst
Comment thread docs/how-to/hip_runtime_api/memory_management/unified_memory.rst
Comment thread docs/how-to/hip_runtime_api/memory_management/unified_memory.rst Outdated
Comment thread docs/how-to/hip_runtime_api/memory_management/unified_memory.rst Outdated
Comment thread docs/how-to/hip_runtime_api/memory_management/unified_memory.rst Outdated
Comment thread docs/how-to/hip_runtime_api/memory_management/unified_memory.rst Outdated
Comment thread docs/how-to/hip_runtime_api/memory_management/unified_memory.rst Outdated
@briankoco briankoco closed this Jul 16, 2025
@adeljo-amd
Copy link
Copy Markdown
Contributor

adeljo-amd commented Jul 17, 2025

@briankoco Is there a reason why this was closed instead of it being merged in? Should we prepare a PR from our side with your changes?

@briankoco
Copy link
Copy Markdown
Member Author

I believe the changes have been merged. Github was not allowing me to merge, with an error about the branch being unable to rebase due to conflicts. I followed the command line approach of checking out docs/develop and pulling in the changes from bjk-dev manually, and it seemed to work

@adeljo-amd
Copy link
Copy Markdown
Contributor

Ah okay, I see the commits in docs/develop now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:docs-only Only run Read the Docs CI on this PR documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants