[PATCH] PR53992 - openmp lower transaction code

[PATCH] PR53992 - openmp lower transaction code

[Patrick Marlier]

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

In this PR, OMP lowering is not going into the transaction code.
So if GIMPLE_TRANSACTION is found, we lower its body.
(Patch also fixes a format issue.)

Note that PR53992 component in Bugzilla must be change from c to libgomp 
(I don't have bugzilla account with admin rights, who should I ask for 
that?).

Tested on trunk / i686.

Ok for trunk? Ok to backport to 4.7 branch if no regression?
Thanks.

gcc/
2012-08-17  Patrick Marlier  <patrick.marlier@gmail.com>

	PR libgomp/53992
	* omp-low.c (lower_omp_1): Handle GIMPLE_TRANSACTION.

[-- Attachment #2: pr53992.patch --]
[-- Type: text/x-patch, Size: 2265 bytes --]

Index: omp-low.c
===================================================================
--- omp-low.c	(revision 190488)
+++ omp-low.c	(working copy)
@@ -6827,6 +6827,9 @@ lower_omp_1 (gimple_stmt_iterator *gsi_p, omp_cont
       lower_omp (gimple_try_eval_ptr (stmt), ctx);
       lower_omp (gimple_try_cleanup_ptr (stmt), ctx);
       break;
+    case GIMPLE_TRANSACTION:
+      lower_omp (gimple_transaction_body_ptr (stmt), ctx);
+      break;
     case GIMPLE_BIND:
       lower_omp (gimple_bind_body_ptr (stmt), ctx);
       break;
@@ -7108,24 +7111,24 @@ diagnose_sb_2 (gimple_stmt_iterator *gsi_p, bool *
       break;
 
     case GIMPLE_COND:
-	{
-	  tree lab = gimple_cond_true_label (stmt);
-	  if (lab)
-	    {
-	      n = splay_tree_lookup (all_labels,
-				     (splay_tree_key) lab);
-	      diagnose_sb_0 (gsi_p, context,
-			     n ? (gimple) n->value : NULL);
-	    }
-	  lab = gimple_cond_false_label (stmt);
-	  if (lab)
-	    {
-	      n = splay_tree_lookup (all_labels,
-				     (splay_tree_key) lab);
-	      diagnose_sb_0 (gsi_p, context,
-			     n ? (gimple) n->value : NULL);
-	    }
-	}
+      {
+	tree lab = gimple_cond_true_label (stmt);
+	if (lab)
+	  {
+	    n = splay_tree_lookup (all_labels,
+				   (splay_tree_key) lab);
+	    diagnose_sb_0 (gsi_p, context,
+			   n ? (gimple) n->value : NULL);
+	  }
+	lab = gimple_cond_false_label (stmt);
+	if (lab)
+	  {
+	    n = splay_tree_lookup (all_labels,
+				   (splay_tree_key) lab);
+	    diagnose_sb_0 (gsi_p, context,
+			   n ? (gimple) n->value : NULL);
+	  }
+      }
       break;
 
     case GIMPLE_GOTO:
Index: testsuite/gcc.dg/gomp/pr53992.c
===================================================================
--- testsuite/gcc.dg/gomp/pr53992.c	(revision 0)
+++ testsuite/gcc.dg/gomp/pr53992.c	(working copy)
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -fopenmp" } */
+/* { dg-require-effective-target fgnu_tm } */
+
+int main() {
+    long data[10000];
+    long i, min=10000;
+    for (i=0; i<10000; i++) data[i] = -i;
+            
+#pragma omp parallel for
+    for (i=0; i<10000; i++) {
+        __transaction_atomic
+        {
+            if (data[i] < min)
+                min = data[i];
+        }
+    }
+
+    return !(min == -9999);
+}

Re: [PATCH] PR53992 - openmp lower transaction code

[Richard Henderson]

On 08/20/2012 07:20 AM, Patrick Marlier wrote:
> 2012-08-17  Patrick Marlier  <patrick.marlier@gmail.com>
> 
>     PR libgomp/53992
>     * omp-low.c (lower_omp_1): Handle GIMPLE_TRANSACTION.

Ok everywhere.

The Bugzilla must not change to libgomp, as that is
reserved for the runtime library.


r~

Re: [PATCH] PR53992 - openmp lower transaction code

[Jakub Jelinek]

On Mon, Aug 20, 2012 at 10:20:33AM -0400, Patrick Marlier wrote:
> Ok for trunk? Ok to backport to 4.7 branch if no regression?

Ok for both, with the following nits resolved:

> gcc/
> 2012-08-17  Patrick Marlier  <patrick.marlier@gmail.com>
> 
> 	PR libgomp/53992

Use PR middle-end/53992 instead, libgomp is for library issues only.

> 	* omp-low.c (lower_omp_1): Handle GIMPLE_TRANSACTION.

> @@ -7108,24 +7111,24 @@ diagnose_sb_2 (gimple_stmt_iterator *gsi_p, bool *
>        break;
>  
>      case GIMPLE_COND:
> -	{
> -	  tree lab = gimple_cond_true_label (stmt);
> -	  if (lab)
> -	    {
> -	      n = splay_tree_lookup (all_labels,
> -				     (splay_tree_key) lab);
> -	      diagnose_sb_0 (gsi_p, context,
> -			     n ? (gimple) n->value : NULL);
> -	    }
> -	  lab = gimple_cond_false_label (stmt);
> -	  if (lab)
> -	    {
> -	      n = splay_tree_lookup (all_labels,
> -				     (splay_tree_key) lab);
> -	      diagnose_sb_0 (gsi_p, context,
> -			     n ? (gimple) n->value : NULL);
> -	    }
> -	}
> +      {
> +	tree lab = gimple_cond_true_label (stmt);
> +	if (lab)
> +	  {
> +	    n = splay_tree_lookup (all_labels,
> +				   (splay_tree_key) lab);
> +	    diagnose_sb_0 (gsi_p, context,
> +			   n ? (gimple) n->value : NULL);
> +	  }
> +	lab = gimple_cond_false_label (stmt);
> +	if (lab)
> +	  {
> +	    n = splay_tree_lookup (all_labels,
> +				   (splay_tree_key) lab);
> +	    diagnose_sb_0 (gsi_p, context,
> +			   n ? (gimple) n->value : NULL);
> +	  }
> +      }
>        break;
>  
>      case GIMPLE_GOTO:

Please leave this hunk out.  Formatting can be normally fixed only
if you touch the code in question or at least lines around it, not
in an unrelated patch that touches completely different function.

> --- testsuite/gcc.dg/gomp/pr53992.c	(revision 0)
> +++ testsuite/gcc.dg/gomp/pr53992.c	(working copy)
> @@ -0,0 +1,20 @@

Please add
/* PR middle-end/53992 */
line to the beginning of the file.

> +/* { dg-do compile } */
> +/* { dg-options "-fgnu-tm -fopenmp" } */
> +/* { dg-require-effective-target fgnu_tm } */
> +
> +int main() {
> +    long data[10000];
> +    long i, min=10000;
> +    for (i=0; i<10000; i++) data[i] = -i;
> +            
> +#pragma omp parallel for
> +    for (i=0; i<10000; i++) {
> +        __transaction_atomic
> +        {
> +            if (data[i] < min)
> +                min = data[i];
> +        }
> +    }
> +
> +    return !(min == -9999);
> +}

	Jakub