Skip to content

Conversation

@grospelliergilles
Copy link
Member

Before this PR, in the case of multiple synchronization, data was stored consecutively in memory. This could lead to the use of a memory address that did not comply with certain constraints.

For example, if a variable of type Byte and an variable of type Int32 were synchronized at the same time and there are 17 elements, the buffer for the Int32 type will have an offset of 17, which can cause problems on certain architectures (for example GPUs).

With this PR, we guarantee an alignment of at least that required for std::max_align_t.

@grospelliergilles grospelliergilles self-assigned this Aug 30, 2025
@grospelliergilles grospelliergilles added bug Something isn't working arcane Arcane Component refactoring Code refactoring and cleanup arcane:accelerator Arcane Accelerator API/Runtime labels Aug 30, 2025
…fers de synchronisation.

Avant cette PR, en cas de synchronisation multiple, les données étaient
rangées consécutivement en mémoire. Cela peut conduire à utiliser une
adresse mémoire ne respectant pas certaines constraintes.
Par exemple, si on synchronise une variable de type octet et de type Int32
en même temps et qu'on a 17 éléments, le buffer pour le type 'Int32' aura
comme offset 17 ce qui peut poser problème sur certaines architectures (
par exemple les GPUs).

Avec cette PR, on garantit un alignement d'au moins celui nécessaire pour
'std::max_align_t'.
@grospelliergilles grospelliergilles force-pushed the dev/gg-add-alignment-requirement-for-sync-buffer branch from 4bc9ad2 to b2a7192 Compare September 1, 2025 06:13
@codecov
Copy link

codecov bot commented Sep 1, 2025

Codecov Report

❌ Patch coverage is 92.59259% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.82%. Comparing base (2144277) to head (b2a7192).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
arcane/src/arcane/impl/DataSynchronizeBuffer.cc 91.30% 5 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2193      +/-   ##
==========================================
+ Coverage   70.79%   70.82%   +0.02%     
==========================================
  Files        2331     2331              
  Lines      170404   170440      +36     
  Branches    19802    19808       +6     
==========================================
+ Hits       120640   120712      +72     
+ Misses      42488    42451      -37     
- Partials     7276     7277       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@grospelliergilles grospelliergilles merged commit 40cea8a into main Sep 1, 2025
36 of 37 checks passed
@grospelliergilles grospelliergilles deleted the dev/gg-add-alignment-requirement-for-sync-buffer branch September 8, 2025 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arcane:accelerator Arcane Accelerator API/Runtime arcane Arcane Component bug Something isn't working refactoring Code refactoring and cleanup

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants