[Flang-RT] Change alignment for allocatable and pointer arrays to 64 …#206525
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| static constexpr unsigned kDeviceAllocatorPos = 2; | ||
| static constexpr unsigned kManagedAllocatorPos = 3; | ||
| static constexpr unsigned kUnifiedAllocatorPos = 4; | ||
| static constexpr unsigned kDefaultArrayAlignment = 64; |
There was a problem hiding this comment.
Maybe add a blank line of a comment to separate this from the CUF allocator positions.
eugeneepshteyn
left a comment
There was a problem hiding this comment.
LGTM.
Note: for this flang-rt change, please ensure that both static and shared library builds of flang work.
Should the new alignment for global, allocatable, and pointer arrays be documented somewhere? Should definitely be mentioned in release notes, but maybe also in another more permanent doc?
|
@Jason-Van-Beusekom , when building with gcc 9.3.0: I think you need to explicitly include |
Nit: please use |
Updated Pr message and will make sure to have the commit with correct tag. |
Updated in 3c7d7fb |
|
Will hold off on merging till tomorrow for any last minute feedback, also this does not cover all cases as simple allocatable arrays are not handled in the runtime and instead in |
|
This broke recent versions of macOS. I created a PR to fix it: #207248 |
|
|
||
| ## Non-comprehensive list of changes in this release | ||
|
|
||
| - Global, allocatable, and pointer arrays are now aligned to 64 bytes. The |
There was a problem hiding this comment.
Is there a special wording to indicate that binaries compiled with this release are not compatible with binaries built by the previous one? We should probably stress that in the release notes.
There was a problem hiding this comment.
I cannot find notes on it but in the discussion for the RFC for this change: https://discourse.llvm.org/t/rfc-alignment-of-global-arrays/90397/14 if I remember correctly this was mentioned in the meeting, and if I recall correctly flang makes no guarantees that new releases work with binaries compiled with older versions.
I can add a mention of it in the release notes if desired however.
…ys (#208781) In #206525 the runtime was set to default alloctable and pointer arrays to 64byte alignment. In this commit the inlineallocation case is handled. The same approach was taken and an alignment attribute was added, with logic to promote to aligned_alloc or posixMemalign for apple targets, when requested alignment is greater than 16. Of note alignof(std::max_align_t)) was not used as the runtime target may be different than the compile target, so 16 was hardcoded as when to use aligned_alloc Assisted-by: Opus 4.8
…on for arrays (#208781) In llvm/llvm-project#206525 the runtime was set to default alloctable and pointer arrays to 64byte alignment. In this commit the inlineallocation case is handled. The same approach was taken and an alignment attribute was added, with logic to promote to aligned_alloc or posixMemalign for apple targets, when requested alignment is greater than 16. Of note alignof(std::max_align_t)) was not used as the runtime target may be different than the compile target, so 16 was hardcoded as when to use aligned_alloc Assisted-by: Opus 4.8
…on for arrays (#208781) In llvm/llvm-project#206525 the runtime was set to default alloctable and pointer arrays to 64byte alignment. In this commit the inlineallocation case is handled. The same approach was taken and an alignment attribute was added, with logic to promote to aligned_alloc or posixMemalign for apple targets, when requested alignment is greater than 16. Of note alignof(std::max_align_t)) was not used as the runtime target may be different than the compile target, so 16 was hardcoded as when to use aligned_alloc Assisted-by: Opus 4.8
…bytes
in #194969 the default array alignment was changed to 64bytes for Fortran globals. In this PR the malloc wrapper in the flang runtime is modified to accept an alignment argument (0 default) and for cases above 16 byte (std::max_align_t) uses
aligned_allocinstead of malloc.The issue of alignment was discussed in this RFC: https://discourse.llvm.org/t/rfc-alignment-of-global-arrays/90397/13
Assisted-by: Opus 4.6