* Re: [committed] openmp: Improve expand_omp_atomic_pipeline
2021-09-03 8:02 [committed] openmp: Improve expand_omp_atomic_pipeline Jakub Jelinek
@ 2021-09-06 9:56 ` Thomas Schwinge
0 siblings, 0 replies; 2+ messages in thread
From: Thomas Schwinge @ 2021-09-06 9:56 UTC (permalink / raw)
To: Jakub Jelinek, gcc-patches; +Cc: Tom de Vries, Andrew Stubbs, Julian Brown
[-- Attachment #1: Type: text/plain, Size: 3990 bytes --]
Hi!
On 2021-09-03T10:02:31+0200, Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> When __atomic_* builtins were introduced, omp-expand.c (omp-low.c
> at that point) has been adjusted in several spots so that it uses
> the atomic builtins instead of sync builtins, but
> expand_omp_atomic_pipeline has not because the __atomic_compare_exchange_*
> APIs take address of the argument, so it kept using __sync_val_compare_swap_*.
> That means it always uses seq_cst though.
> This patch changes it to use the ATOMIC_COMPARE_EXCHANGE ifn which gimple-fold
> folds __atomic_compare_exchange_* into - that ifn also passes expected
> directly.
>
> Bootstrapped/regtested on x86_64-linux, committed to trunk.
I've pushed "'libgomp.c/target-43.c': '-latomic' for nvptx offloading" to
master branch in commit 086bb917d6efa32d9841c34a3b762f4278c762cd, see
attached.
And, I've filed <https://gcc.gnu.org/PR102215>
"[GCN offloading] Missing '__atomic_compare_exchange_1' etc.".
Grüße
Thomas
> 2021-09-03 Jakub Jelinek <jakub@redhat.com>
>
> * omp-expand.c (expand_omp_atomic_pipeline): Use
> IFN_ATOMIC_COMPARE_EXCHANGE instead of
> BUILT_IN_SYNC_VAL_COMPARE_AND_SWAP_? so that memory order
> can be provided.
>
> --- gcc/omp-expand.c.jj 2021-09-01 11:37:41.966556334 +0200
> +++ gcc/omp-expand.c 2021-09-02 15:50:58.276341091 +0200
> @@ -8807,8 +8807,6 @@ expand_omp_atomic_pipeline (basic_block
> edge e;
> enum built_in_function fncode;
>
> - /* ??? We need a non-pointer interface to __atomic_compare_exchange in
> - order to use the RELAXED memory model effectively. */
> fncode = (enum built_in_function)((int)BUILT_IN_SYNC_VAL_COMPARE_AND_SWAP_N
> + index + 1);
> cmpxchg = builtin_decl_explicit (fncode);
> @@ -8825,6 +8823,15 @@ expand_omp_atomic_pipeline (basic_block
> /* Load the initial value, replacing the GIMPLE_OMP_ATOMIC_LOAD. */
> si = gsi_last_nondebug_bb (load_bb);
> gcc_assert (gimple_code (gsi_stmt (si)) == GIMPLE_OMP_ATOMIC_LOAD);
> + location_t loc = gimple_location (gsi_stmt (si));
> + enum omp_memory_order omo = gimple_omp_atomic_memory_order (gsi_stmt (si));
> + enum memmodel imo = omp_memory_order_to_memmodel (omo);
> + tree mo = build_int_cst (NULL, imo);
> + if (imo == MEMMODEL_RELEASE)
> + imo = MEMMODEL_RELAXED;
> + else if (imo == MEMMODEL_ACQ_REL)
> + imo = MEMMODEL_ACQUIRE;
> + tree fmo = build_int_cst (NULL, imo);
>
> /* For floating-point values, we'll need to view-convert them to integers
> so that we can perform the atomic compare and swap. Simplify the
> @@ -8921,7 +8928,15 @@ expand_omp_atomic_pipeline (basic_block
> GSI_SAME_STMT);
>
> /* Build the compare&swap statement. */
> - new_storedi = build_call_expr (cmpxchg, 3, iaddr, loadedi, storedi);
> + tree ctype = build_complex_type (itype);
> + int flag = int_size_in_bytes (itype);
> + new_storedi = build_call_expr_internal_loc (loc, IFN_ATOMIC_COMPARE_EXCHANGE,
> + ctype, 6, iaddr, loadedi,
> + storedi,
> + build_int_cst (integer_type_node,
> + flag),
> + mo, fmo);
> + new_storedi = build1 (REALPART_EXPR, itype, new_storedi);
> new_storedi = force_gimple_operand_gsi (&si,
> fold_convert (TREE_TYPE (loadedi),
> new_storedi),
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-libgomp.c-target-43.c-latomic-for-nvptx-offloading.patch --]
[-- Type: text/x-diff, Size: 1151 bytes --]
From 086bb917d6efa32d9841c34a3b762f4278c762cd Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Mon, 6 Sep 2021 11:42:03 +0200
Subject: [PATCH] 'libgomp.c/target-43.c': '-latomic' for nvptx offloading
... to avoid a regression with recent
commit 090f0d78f194e3cda23fe904016db77ea36c38fa
"openmp: Improve expand_omp_atomic_pipeline":
unresolved symbol __atomic_compare_exchange_1
collect2: error: ld returned 1 exit status
mkoffload: fatal error: [...]/gcc/x86_64-pc-linux-gnu-accel-nvptx-none-gcc returned 1 exit status
libgomp/
* testsuite/libgomp.c/target-43.c: '-latomic' for nvptx offloading.
---
libgomp/testsuite/libgomp.c/target-43.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libgomp/testsuite/libgomp.c/target-43.c b/libgomp/testsuite/libgomp.c/target-43.c
index 46b1cfc5b20..028e9120f2a 100644
--- a/libgomp/testsuite/libgomp.c/target-43.c
+++ b/libgomp/testsuite/libgomp.c/target-43.c
@@ -1,4 +1,6 @@
/* { dg-do run } */
+/* { dg-additional-options "-foffload-options=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+
#include <stdlib.h>
#define N 32
--
2.33.0
^ permalink raw reply [flat|nested] 2+ messages in thread