Skip to content

hip-tests: zero-initialize host atomics buffer before __atomic_fetch_add onto it - #8967

Merged
iassiour merged 1 commit into
developfrom
users/iassiour/atomics-host-buffer-zero-init-develop
Oct 14, 2026
Merged

iassiour merged 1 commit into
developfrom
users/iassiour/atomics-host-buffer-zero-init-develop

Conversation

@iassiour

@iassiour iassiour commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

hipHostMalloc does not guarantee zeroed memory. The host competitor buffer
in the *_Host_And_GPU atomics tests was used uninitialized.

Motivation

The atomicAdd/Sub/CAS_system_Positive_Host_And_GPU tests allocate the
host competitor buffer with hipHostMalloc and immediately run host
__atomic_fetch_add on it — without initializing to zero. The device
buffers are hipMemset'd, but the host buffer is not. Since hipHostMalloc
does not guarantee zeroed memory (same as malloc), the test accumulates
onto undefined contents and fails the final comparison.

Technical Details

One-line fix in arithmetic_common.hh TestCore(): add
std::memset(host_ptr, 0, size) after hipHostMalloc, before host
threads run. Uses host-side memset (not hipMemset) since the buffer
is only touched by host threads.

Issue Tracking

JIRA ID : ROCM-28378

Test Plan

Ran all affected tests: atomicAdd_system_Positive_Host_And_GPU (6 type
sections), atomicSub_system_Positive_Host_And_GPU,
atomicCAS_system_Positive_Host_And_GPU (4 type variants) — all pass.
Tests using other common headers (atomicExch, And/Or/Xor, Min/Max) are
not affected.

Test Result

All pass. No regressions.

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Jul 21, 2026 •

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@iassiour
iassiour marked this pull request as ready for review July 21, 2026 19:05
@iassiour
iassiour requested a review from a team as a code owner July 21, 2026 19:05
Copilot AI review requested due to automatic review settings July 21, 2026 19:05
@therock-pr-bot

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Title/Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes nondeterministic failures in HIP atomics “*_Host_And_GPU” tests by ensuring the host-side competitor buffer allocated via hipHostMalloc is explicitly zero-initialized before any host atomic operations run.

Changes:

  • Add <cstring> include to support std::memset.
  • Zero-initialize the hipHostMalloc host competitor buffer in TestCore() before launching host threads.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@iassiour iassiour changed the title hip-tests: zero-initialize host atomics buffer before host threads run hip-tests: zero-initialize host atomics buffer before __atomic_fetch_add onto it Jul 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been inactive for 25 days and will be marked as stale.

If you would like to keep this PR open, please:

  • Add new commits
  • Add a comment explaining why it should remain open

This PR will be automatically closed in 5 days if no further activity occurs.

@github-actions github-actions Bot added the Stale PR has no activity for 25+ days label Aug 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically closed due to inactivity (30 days with no updates).

If you'd like to continue working on this, feel free to reopen the PR or create a new one.

@github-actions github-actions Bot closed this Aug 22, 2026
@iassiour iassiour reopened this Oct 12, 2026
@github-actions github-actions Bot removed the Stale PR has no activity for 25+ days label Oct 13, 2026
@iassiour
iassiour force-pushed the users/iassiour/atomics-host-buffer-zero-init-develop branch from de26228 to a179f34 Compare October 13, 2026 12:09
@mangupta

Copy link
Copy Markdown
Contributor

🤖 AI-assisted pre-review (for @mangupta)

Re-reviewed the updated head (a179f34a9728) — no new findings at or above the posting threshold.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The initialization correctly addresses the uninitialized-memory failure without introducing unresolved issues.

0 open findings

🧠 Review effort: Balanced

@iassiour
iassiour merged commit f674e87 into develop Oct 14, 2026
89 of 90 checks passed
@iassiour
iassiour deleted the users/iassiour/atomics-host-buffer-zero-init-develop branch October 14, 2026 10:05
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants