public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc/devel/omp/gcc-11] openmp: Fix up struct gomp_work_share handling [PR102838]
@ 2021-10-20  8:38 Tobias Burnus
  0 siblings, 0 replies; only message in thread
From: Tobias Burnus @ 2021-10-20  8:38 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:5fd5e23b2cb4cfc2a455ab2e55f2a84346a192a8

commit 5fd5e23b2cb4cfc2a455ab2e55f2a84346a192a8
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Wed Oct 20 10:33:40 2021 +0200

    openmp: Fix up struct gomp_work_share handling [PR102838]
    
    If GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC is not defined, the intent was to
    treat the split of the structure between first cacheline (64 bytes)
    as mostly write-once, use afterwards and second cacheline as rw just
    as an optimization.  But as has been reported, with vectorization enabled
    at -O2 it can now result in aligned vector 16-byte or larger stores.
    When not having posix_memalign/aligned_alloc/memalign or other similar API,
    alloc.c emulates it but it needs to allocate extra memory for the dynamic
    realignment.
    So, for the GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC not defined case, this patch
    stops using aligned (64) attribute in the middle of the structure and instead
    inserts padding that puts the second half of the structure at offset 64 bytes.
    
    And when GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC is defined, usually it was allocated
    as aligned, but for the orphaned case it could still be allocated just with
    gomp_malloc without guaranteed proper alignment.
    
    2021-10-20  Jakub Jelinek  <jakub@redhat.com>
    
            PR libgomp/102838
            * libgomp.h (struct gomp_work_share_1st_cacheline): New type.
            (struct gomp_work_share): Only use aligned(64) attribute if
            GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC is defined, otherwise just
            add padding before lock to ensure lock is at offset 64 bytes
            into the structure.
            (gomp_workshare_struct_check1, gomp_workshare_struct_check2):
            New poor man's static assertions.
            * work.c (gomp_work_share_start): Use gomp_aligned_alloc instead of
            gomp_malloc if GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC.
    
    (cherry picked from commit c7abdf46fb7ac9a0c37f120feff3fcc3a752584f)

Diff:
---
 libgomp/ChangeLog.omp | 16 ++++++++++++++++
 libgomp/libgomp.h     | 35 +++++++++++++++++++++++++++++++++++
 libgomp/work.c        |  5 +++++
 3 files changed, 56 insertions(+)

diff --git a/libgomp/ChangeLog.omp b/libgomp/ChangeLog.omp
index 157d89b9196..a37a94eec96 100644
--- a/libgomp/ChangeLog.omp
+++ b/libgomp/ChangeLog.omp
@@ -1,3 +1,19 @@
+2021-10-20  Tobias Burnus  <tobias@codesourcery.com>
+
+	Backported from master:
+	2021-10-20  Jakub Jelinek  <jakub@redhat.com>
+
+	PR libgomp/102838
+	* libgomp.h (struct gomp_work_share_1st_cacheline): New type.
+	(struct gomp_work_share): Only use aligned(64) attribute if
+	GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC is defined, otherwise just
+	add padding before lock to ensure lock is at offset 64 bytes
+	into the structure.
+	(gomp_workshare_struct_check1, gomp_workshare_struct_check2):
+	New poor man's static assertions.
+	* work.c (gomp_work_share_start): Use gomp_aligned_alloc instead of
+	gomp_malloc if GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC.
+
 2021-10-19  Tobias Burnus  <tobias@codesourcery.com>
 
 	Backported from master:
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index f7745a239ec..fae2dbd2e40 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -257,6 +257,30 @@ struct gomp_doacross_work_share
   unsigned int shift_counts[];
 };
 
+/* Like struct gomp_work_share, but only the 1st cacheline of it plus
+   flexible array at the end.
+   Keep in sync with struct gomp_work_share.  */
+struct gomp_work_share_1st_cacheline
+{
+  enum gomp_schedule_type sched;
+  int mode;
+  union {
+    struct {
+      long chunk_size, end, incr;
+    };
+    struct {
+      unsigned long long chunk_size_ull, end_ull, incr_ull;
+    };
+  };
+  union {
+    unsigned *ordered_team_ids;
+    struct gomp_doacross_work_share *doacross;
+  };
+  unsigned ordered_num_used, ordered_owner, ordered_cur;
+  struct gomp_work_share *next_alloc;
+  char pad[];
+};
+
 struct gomp_work_share
 {
   /* This member records the SCHEDULE clause to be used for this construct.
@@ -324,7 +348,12 @@ struct gomp_work_share
      are in a different cache line.  */
 
   /* This lock protects the update of the following members.  */
+#ifdef GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC
   gomp_mutex_t lock __attribute__((aligned (64)));
+#else
+  char pad[64 - offsetof (struct gomp_work_share_1st_cacheline, pad)];
+  gomp_mutex_t lock;
+#endif
 
   /* This is the count of the number of threads that have exited the work
      share construct.  If the construct was marked nowait, they have moved on
@@ -362,6 +391,12 @@ struct gomp_work_share
   unsigned inline_ordered_team_ids[0];
 };
 
+extern char gomp_workshare_struct_check1
+  [offsetof (struct gomp_work_share_1st_cacheline, next_alloc)
+   == offsetof (struct gomp_work_share, next_alloc) ? 1 : -1];
+extern char gomp_workshare_struct_check2
+  [offsetof (struct gomp_work_share, lock) == 64 ? 1 : -1];
+
 /* This structure contains all of the thread-local data associated with 
    a thread team.  This is the data that must be saved when a thread
    encounters a nested PARALLEL construct.  */
diff --git a/libgomp/work.c b/libgomp/work.c
index 9d63eec9832..bf2559155f1 100644
--- a/libgomp/work.c
+++ b/libgomp/work.c
@@ -191,7 +191,12 @@ gomp_work_share_start (size_t ordered)
   /* Work sharing constructs can be orphaned.  */
   if (team == NULL)
     {
+#ifdef GOMP_HAVE_EFFICIENT_ALIGNED_ALLOC
+      ws = gomp_aligned_alloc (__alignof (struct gomp_work_share),
+			       sizeof (*ws));
+#else
       ws = gomp_malloc (sizeof (*ws));
+#endif
       gomp_init_work_share (ws, ordered, 1);
       thr->ts.work_share = ws;
       return true;


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-10-20  8:38 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-20  8:38 [gcc/devel/omp/gcc-11] openmp: Fix up struct gomp_work_share handling [PR102838] Tobias Burnus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).