Patchwork [trans-mem] issue with openmp

login
register
mail settings
Submitter Aldy Hernandez
Date June 22, 2010, 6:17 p.m.
Message ID <20100622181750.GA23011@redhat.com>
Download mbox | patch
Permalink /patch/56559/
State New
Headers show

Comments

Aldy Hernandez - June 22, 2010, 6:17 p.m.
On Fri, Jun 18, 2010 at 11:26:00AM -0400, Aldy Hernandez wrote:
> The problem here is that gimplify_transaction() places the temporaries
> that were generated for a transaction in cfun->local_decls, but
> omp_copy_decl() will only look in the enclosing contexts, not in
> cfun->local_decls.
> 
> rth suggested we make a better attempt at putting temporaries into the
> proper context so OMP can figure out how to pull pieces out to make a
> new function.
> 
> The patch below wraps the transaction bodies into a BIND_EXPR, which
> gimplify_transaction() can later use for its temporaries, thus allowing
> the OMP code to find a proper context.
> 
> OK for branch?

Meanwhile, back at the ranch... rth complains that we should do this in
the gimplifier and save the front-end work.

Yay, less code!

OK for branch?

	* gimplify.c (gimplify_transaction): Wrap transaction body
	in a BIND_EXPR.
Richard Henderson - June 22, 2010, 6:41 p.m.
On 06/22/2010 11:17 AM, Aldy Hernandez wrote:
> On Fri, Jun 18, 2010 at 11:26:00AM -0400, Aldy Hernandez wrote:
>> The problem here is that gimplify_transaction() places the temporaries
>> that were generated for a transaction in cfun->local_decls, but
>> omp_copy_decl() will only look in the enclosing contexts, not in
>> cfun->local_decls.
>>
>> rth suggested we make a better attempt at putting temporaries into the
>> proper context so OMP can figure out how to pull pieces out to make a
>> new function.
>>
>> The patch below wraps the transaction bodies into a BIND_EXPR, which
>> gimplify_transaction() can later use for its temporaries, thus allowing
>> the OMP code to find a proper context.
>>
>> OK for branch?
> 
> Meanwhile, back at the ranch... rth complains that we should do this in
> the gimplifier and save the front-end work.
> 
> Yay, less code!
> 
> OK for branch?
> 
> 	* gimplify.c (gimplify_transaction): Wrap transaction body
> 	in a BIND_EXPR.


Ok.


r~

Patch

Index: testsuite/c-c++-common/tm/omp.c
===================================================================
--- testsuite/c-c++-common/tm/omp.c	(revision 0)
+++ testsuite/c-c++-common/tm/omp.c	(revision 0)
@@ -0,0 +1,22 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -fopenmp" } */
+
+__attribute__ ((transaction_pure))
+unsigned long rdtsc();
+
+typedef struct ENTER_EXIT_TIMES
+{
+  unsigned long enter;
+} times_t;
+
+void ParClassify()
+{
+  void * Parent;
+#pragma omp parallel private(Parent)
+  {
+    times_t inside;
+    __transaction [[atomic]] {
+       inside.enter = rdtsc();
+    }
+  }
+}
Index: gimplify.c
===================================================================
--- gimplify.c	(revision 161187)
+++ gimplify.c	(working copy)
@@ -6385,20 +6385,27 @@  gimplify_omp_atomic (tree *expr_p, gimpl
 static enum gimplify_status
 gimplify_transaction (tree *expr_p, gimple_seq *pre_p)
 {
-  tree expr = *expr_p, temp;
+  tree expr = *expr_p, temp, tbody = TRANSACTION_EXPR_BODY (expr);
   gimple g;
   gimple_seq body = NULL;
   struct gimplify_ctx gctx;
   int subcode = 0;
 
+  /* Wrap the transaction body in a BIND_EXPR so we have a context
+     where to put decls for OpenMP.  */
+  if (TREE_CODE (tbody) != BIND_EXPR)
+    {
+      tree bind = build3 (BIND_EXPR, void_type_node, NULL, tbody, NULL);
+      TREE_SIDE_EFFECTS (bind) = 1;
+      SET_EXPR_LOCATION (bind, EXPR_LOCATION (tbody));
+      TRANSACTION_EXPR_BODY (expr) = bind;
+    }
+
   push_gimplify_context (&gctx);
   temp = voidify_wrapper_expr (*expr_p, NULL);
 
   g = gimplify_and_return_first (TRANSACTION_EXPR_BODY (expr), &body);
-  if (g && gimple_code (g) == GIMPLE_BIND)
-    pop_gimplify_context (g);
-  else
-    pop_gimplify_context (NULL);
+  pop_gimplify_context (g);
 
   g = gimple_build_transaction (body, NULL);
   if (TRANSACTION_EXPR_OUTER (expr))