Skip to content

cleanup and clarification of unified/managed memory docs #3819

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

Closed
wants to merge 0 commits into from

Conversation

briankoco
Copy link
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

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

@briankoco briankoco closed this Jul 16, 2025
@adeljo-amd
Copy link

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
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

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