Skip to content

Replace hipMallocManaged with hipMalloc+hipMemcpy - #326

Open
maarquitos14 wants to merge 1 commit into
ORNL:masterfrom
maarquitos14:maronas/fix-nbnxm-managed-memory
Open

Replace hipMallocManaged with hipMalloc+hipMemcpy#326
maarquitos14 wants to merge 1 commit into
ORNL:masterfrom
maarquitos14:maronas/fix-nbnxm-managed-memory

Conversation

@maarquitos14

Copy link
Copy Markdown
Contributor

hipMallocManaged causes pathological performance on unified memory architectures (e.g. MI300X/gfx942) because atomics on managed memory require system-scope (CPU-GPU) coherency. With repeat = 10000, the benchmark takes ~700s to finish with hipMallocManaged, while it completes in ~7s with hipMalloc + hipMemcpy.

Copilot AI balanced review requested due to automatic review settings August 27, 2026 13:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Replaces HIP managed memory with explicit host/device buffers to avoid costly system-scope atomics on unified-memory GPUs.

Changes:

  • Allocates device buffers with hipMalloc.
  • Initializes host buffers and transfers data with hipMemcpy.
  • Copies results back for debug checks and releases host memory.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@zjin-lcf
zjin-lcf self-requested a review August 27, 2026 16:01
@zjin-lcf

Copy link
Copy Markdown
Collaborator

Thank you for the pull request. A PR (#327) was submitted to revise the programs, and it will supersede this one.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants