Skip to content

Add C11 atomic wrapper structs to prevent direct access and control memory ordering #13373

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Copilot
Copy link

@Copilot Copilot AI commented Aug 21, 2025

This PR implements a C11 atomic wrapper system that prevents direct access to atomic variables and provides explicit memory ordering control, addressing the overhead of sequential consistency.

Problem

OpenMPI currently uses C11 _Atomic types directly via typedefs, which allows users to accidentally perform direct assignments that default to sequential consistency:

// Current (problematic) - defaults to seq_cst
opal_atomic_int32_t counter = 0;  
counter = 42;  // Sequential consistency overhead

Solution

The implementation wraps all _Atomic types in structs to prevent direct access and provides accessor functions with explicit memory ordering control:

// New wrapper struct (prevents direct access)
typedef struct { _Atomic int32_t value; } opal_atomic_int32_t;

// Explicit memory ordering (defaults to relaxed)
opal_atomic_store_32_relaxed(&counter, 42);       // Relaxed ordering
opal_atomic_store_32(&counter, 42, memory_order_acquire);  // Explicit ordering

Key Changes

  1. Wrapper Structs: All _Atomic types are now wrapped in structs with .value members
  2. Accessor Functions: Provided load/store functions that require explicit memory ordering
  3. Relaxed Defaults: Convenience functions default to memory_order_relaxed to avoid seq_cst overhead
  4. Volatile Support: Added variants for volatile atomic variables used in lock-free data structures
  5. API Migration: Updated core atomic operations and data structures to use the new API

Affected Components

  • Core atomic infrastructure (opal_stdatomic.h, atomic_stdc.h)
  • Object reference counting and memory management
  • Lock-free data structures (LIFO, FIFO, lists)
  • Threading and synchronization primitives
  • BTL shared memory communication
  • Memory pool and cache management

Verification

The wrapper successfully prevents compilation of direct atomic access:

opal_atomic_int32_t x = {.value = 0};
x = 5;  // ❌ Compilation error - direct assignment prevented
opal_atomic_store_32_relaxed(&x, 5);  // ✅ Required explicit API

This ensures all atomic operations use explicit memory ordering, eliminating accidental sequential consistency overhead while maintaining correctness.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 3 commits August 21, 2025 20:19
Co-authored-by: devreal <10974502+devreal@users.noreply.github.com>
Co-authored-by: devreal <10974502+devreal@users.noreply.github.com>
…irect access

Co-authored-by: devreal <10974502+devreal@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Before you begin, please make sure you have the following tools available: GNU Autoconf: 2.69.0 GNU Automake: 1.13.4 GNU Libtool: 2.4.2 To test the changes, you can create the build system as follows: git submodule update --init --recursive ./auto... Add C11 atomic wrapper structs to prevent direct access and control memory ordering Aug 21, 2025
@Copilot Copilot AI requested a review from devreal August 21, 2025 20:34
Copilot finished work on behalf of devreal August 21, 2025 20:34
Copy link

Hello! The Git Commit Checker CI bot found a few problems with this PR:

d49a9f8: Complete C11 atomic wrapper implementation - worki...

  • check_signed_off: does not contain a valid Signed-off-by line

f81e3ed: Fix additional atomic access issues and add volati...

  • check_signed_off: does not contain a valid Signed-off-by line

3759195: Implement C11 atomic wrapper structs and fix direc...

  • check_signed_off: does not contain a valid Signed-off-by line

0c0f801: Initial plan

  • check_signed_off: does not contain a valid Signed-off-by line

Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!

2 similar comments
Copy link

Hello! The Git Commit Checker CI bot found a few problems with this PR:

d49a9f8: Complete C11 atomic wrapper implementation - worki...

  • check_signed_off: does not contain a valid Signed-off-by line

f81e3ed: Fix additional atomic access issues and add volati...

  • check_signed_off: does not contain a valid Signed-off-by line

3759195: Implement C11 atomic wrapper structs and fix direc...

  • check_signed_off: does not contain a valid Signed-off-by line

0c0f801: Initial plan

  • check_signed_off: does not contain a valid Signed-off-by line

Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!

Copy link

Hello! The Git Commit Checker CI bot found a few problems with this PR:

d49a9f8: Complete C11 atomic wrapper implementation - worki...

  • check_signed_off: does not contain a valid Signed-off-by line

f81e3ed: Fix additional atomic access issues and add volati...

  • check_signed_off: does not contain a valid Signed-off-by line

3759195: Implement C11 atomic wrapper structs and fix direc...

  • check_signed_off: does not contain a valid Signed-off-by line

0c0f801: Initial plan

  • check_signed_off: does not contain a valid Signed-off-by line

Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!

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.

2 participants