This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[gomp] Fix #pragma omp atomic


Hi!

goa_lhs_expr_p does only simple tree pointer comparisons to find if there is
a LHS expression copy somewhere on the RHS.  Unfortunately, there has been
unshare_body in between, so whenever we have something tiny bit complicated
on the LHS, the test fails and we get both incorrect and inefficient code.
Incorrect because it reads from the memory multiple times (especially with
-O0), say
#pragma omp atomic
  n[2] += 1;
is expanded as
  tmp1 = n[2] + 1;
  tmp2 = n[2];
  tmp3 = &n[2];
  __sync_val_compare_and_swap_4 (tmp3, tmp2, tmp1);
and if n[2] changes in between the first and second line we loose.
Inefficient because we could use __sync_fetch_and_add_N on many targets, but
we use __sync_val_compare_and_swap_N instead.
Writing a helper function for goa_lhs_expr_p that would compare arbitrary
trees and return bool whether they are the same with possible tree unsharing
in between would be hard and error-prone, so I think it is better to go with
a temporary variable (unless it is a simple address of a VAR_DECL or
SAVE_EXPR (both of which worked fine before)).

Ok for trunk?

2006-09-11  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/28046
	* c-omp.c (c_finish_omp_atomic): If ADDR is not simple enough,
	wrap it into TARGET_EXPR.

	* gcc.dg/gomp/atomic-10.c: New test.
	* g++.dg/gomp/atomic-10.C: New test.

--- gcc/c-omp.c.jj	2006-08-28 13:00:24.000000000 +0200
+++ gcc/c-omp.c	2006-09-11 21:07:18.000000000 +0200
@@ -116,6 +116,15 @@ c_finish_omp_atomic (enum tree_code code
   if (addr == error_mark_node)
     return error_mark_node;
   addr = save_expr (addr);
+  if (TREE_CODE (addr) != SAVE_EXPR
+      && (TREE_CODE (addr) != ADDR_EXPR
+	  || TREE_CODE (TREE_OPERAND (addr, 0)) != VAR_DECL))
+    {
+      /* Make sure LHS is simple enough so that goa_lhs_expr_p can recognize
+	 it even after unsharing function body.  */
+      tree var = create_tmp_var_raw (TREE_TYPE (addr), NULL);
+      addr = build4 (TARGET_EXPR, TREE_TYPE (addr), var, addr, NULL, NULL);
+    }
   lhs = build_indirect_ref (addr, NULL);
 
   /* There are lots of warnings, errors, and conversions that need to happen
--- gcc/testsuite/g++.dg/gomp/atomic-10.C.jj	2006-09-11 21:17:05.000000000 +0200
+++ gcc/testsuite/g++.dg/gomp/atomic-10.C	2006-09-11 21:19:16.000000000 +0200
@@ -0,0 +1,24 @@
+// PR middle-end/28046
+// { dg-do compile }
+// { dg-options "-fopenmp -fdump-tree-gimple" }
+
+int a[3], b;
+struct C { int x; int y; } c;
+
+int bar (void), *baz (void);
+
+void
+foo (void)
+{
+#pragma omp atomic
+  a[2] += bar ();
+#pragma omp atomic
+  b += bar ();
+#pragma omp atomic
+  c.y += bar ();
+#pragma omp atomic
+  *baz () += bar ();
+}
+
+// { dg-final { scan-tree-dump-times "__sync_fetch_and_add" 4 "gimple" { target i?86-*-* x86_64-*-* ia64-*-* powerpc*-*-* alpha*-*-* } } }
+// { dg-final { cleanup-tree-dump "gimple" } }
--- gcc/testsuite/gcc.dg/gomp/atomic-10.c.jj	2006-09-11 21:16:37.000000000 +0200
+++ gcc/testsuite/gcc.dg/gomp/atomic-10.c	2006-09-11 21:19:27.000000000 +0200
@@ -0,0 +1,24 @@
+/* PR middle-end/28046 */
+/* { dg-do compile } */
+/* { dg-options "-fopenmp -fdump-tree-gimple" } */
+
+int a[3], b;
+struct C { int x; int y; } c;
+
+int bar (void), *baz (void);
+
+void
+foo (void)
+{
+#pragma omp atomic
+  a[2] += bar ();
+#pragma omp atomic
+  b += bar ();
+#pragma omp atomic
+  c.y += bar ();
+#pragma omp atomic
+  *baz () += bar ();
+}
+
+/* { dg-final { scan-tree-dump-times "__sync_fetch_and_add" 4 "gimple" { target i?86-*-* x86_64-*-* ia64-*-* powerpc*-*-* alpha*-*-* } } } */
+/* { dg-final { cleanup-tree-dump "gimple" } } */

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]