Skip to content

Commit 3b8ac7a

Browse files
authored
[OpenMP] Fix various alignment issues (#142376)
When running the `openmp` testsuite on 32-bit SPARC, several tests `FAIL` apparently randomly, but always with the same kind of error: ``` # error: command failed with exit status: -11 ``` The tests die with `SIGBUS`, as can be seen in `truss` output: ``` 26461/1: Incurred fault #5, FLTACCESS %pc = 0x00010EAC 26461/1: siginfo: SIGBUS BUS_ADRALN addr=0x0013D12C 26461/1: Received signal #10, SIGBUS [default] 26461/1: siginfo: SIGBUS BUS_ADRALN addr=0x0013D12C ``` i.e. the code is trying an unaligned access which cannot work on SPARC, a strict-alignment target which enforces natural alignment on access. This explains the apparent randomness of the failures: if the memory happens to be aligned appropriately, the tests work, but fail if not. A `Debug` build reveals much more: - `__kmp_alloc` currently aligns to `sizeof(void *)`, which isn't enough on strict-alignment targets when the data are accessed as types requiring larger alignment. Therefore, this patch increases `alignment` to `SizeQuant`. - 32-bit Solaris/sparc `libc` guarantees 8-byte alignment from `malloc`, so this patch adjusts `SizeQuant` to match. - There's a `SIGBUS` in ``` __kmpc_fork_teams (loc=0x112f8, argc=0, microtask=0x16cc8 <__omp_offloading_ffbc020a_4b1abe_main_l9_debug__.omp_outlined>) at openmp/runtime/src/kmp_csupport.cpp:573 573 *(kmp_int64 *)(&this_thr->th.th_teams_size) = 0L; ``` Casting to a pointer to a type requiring 64-bit alignment when that isn't guaranteed is wrong. Instead, this patch uses `memset` instead. - There's another `SIGBUS` in ``` 0xfef8cb9c in __kmp_taskloop_recur (loc=0x10cb8, gtid=0, task=0x23cd00, lb=0x23cd18, ub=0x23cd20, st=1, ub_glob=499, num_tasks=100, grainsize=5, extras=0, last_chunk=0, tc=500, num_t_min=20, codeptr_ra=0xfef8dbc8 <__kmpc_taskloop(ident_t*, int, kmp_task_t*, int, kmp_uint64*, kmp_uint64*, kmp_int64, int, int, kmp_uint64, void*)+240>, task_dup=0x0) at openmp/runtime/src/kmp_tasking.cpp:5147 5147 p->st = st; ``` `p->st` doesn't currently guarantee the 8-byte alignment required by `kmp_int64 st`. `p` is set in ``` __taskloop_params_t *p = (__taskloop_params_t *)new_task->shareds; ``` but `shareds_offset` is currently aligned to `sizeof(void *)` only. Increasing it to `sizeof(kmp_uint64)` to match its use fixes the `SIGBUS`. With these fixes I get clean `openmp` test results on 32-bit SPARC (both Solaris and Linux), with one unrelated exception. Tested on `sparc-sun-solaris2.11`, `sparcv9-sun-solaris2.11`, `sparc-unknown-linux-gnu`, `sparc64-unknown-linux-gnu`, `i386-pc-solaris2.11`, `amd64-pc-solaris2.11`, `i686-pc-linux-gnu`, and `x86_64-pc-linux-gnu`.
1 parent 5781d52 commit 3b8ac7a

File tree

3 files changed

+6
-6
lines changed

3 files changed

+6
-6
lines changed

openmp/runtime/src/kmp_alloc.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,10 @@ static void bectl(kmp_info_t *th, bget_compact_t compact,
7070
/* Buffer allocation size quantum: all buffers allocated are a
7171
multiple of this size. This MUST be a power of two. */
7272

73-
/* On IA-32 architecture with Linux* OS, malloc() does not
74-
ensure 16 byte alignment */
73+
/* On some architectures, malloc() does not ensure 16 byte alignment,
74+
Solaris/sparc and x86 among them. */
7575

76-
#if KMP_ARCH_X86 || !KMP_HAVE_QUAD
76+
#if KMP_ARCH_X86 || KMP_ARCH_SPARC || !KMP_HAVE_QUAD
7777

7878
#define SizeQuant 8
7979
#define AlignType double
@@ -1861,7 +1861,7 @@ typedef struct kmp_mem_desc { // Memory block descriptor
18611861
void *ptr_align; // Pointer to aligned memory, returned
18621862
kmp_allocator_t *allocator; // allocator
18631863
} kmp_mem_desc_t;
1864-
static int alignment = sizeof(void *); // align to pointer size by default
1864+
constexpr size_t alignment = SizeQuant;
18651865

18661866
// external interfaces are wrappers over internal implementation
18671867
void *__kmpc_alloc(int gtid, size_t size, omp_allocator_handle_t allocator) {

openmp/runtime/src/kmp_csupport.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ void __kmpc_fork_teams(ident_t *loc, kmp_int32 argc, kmpc_micro microtask,
570570

571571
this_thr->th.th_teams_microtask = NULL;
572572
this_thr->th.th_teams_level = 0;
573-
*(kmp_int64 *)(&this_thr->th.th_teams_size) = 0L;
573+
memset(&this_thr->th.th_teams_size, 0, sizeof(kmp_teams_size_t));
574574
va_end(ap);
575575
#if KMP_STATS_ENABLED
576576
if (previous_state == stats_state_e::SERIAL_REGION) {

openmp/runtime/src/kmp_tasking.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1528,7 +1528,7 @@ kmp_task_t *__kmp_task_alloc(ident_t *loc_ref, kmp_int32 gtid,
15281528
// Calculate shared structure offset including padding after kmp_task_t struct
15291529
// to align pointers in shared struct
15301530
shareds_offset = sizeof(kmp_taskdata_t) + sizeof_kmp_task_t;
1531-
shareds_offset = __kmp_round_up_to_val(shareds_offset, sizeof(void *));
1531+
shareds_offset = __kmp_round_up_to_val(shareds_offset, sizeof(kmp_uint64));
15321532

15331533
// Allocate a kmp_taskdata_t block and a kmp_task_t block.
15341534
KA_TRACE(30, ("__kmp_task_alloc: T#%d First malloc size: %ld\n", gtid,

0 commit comments

Comments
 (0)