OpenMP: detach - fix firstprivate handling

Message ID 9e2b02be-abf4-7bf6-e12d-11fcbeb201fc@codesourcery.com
State New
Headers show
Series
  • OpenMP: detach - fix firstprivate handling
Related show

Commit Message

Tobias Burnus May 11, 2021, 9:54 p.m.
The sfield / firstprivate lookup used the wrong var decl
for the lookup – hence it failed.
I used an extra long diff to make it easier to follow why
'c' and not 'detach_clause' has the proper clause for the
decl to be used as key.

Testsuite run ongoing.
OK for mainline, when it passes?

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

Comments

Richard Sandiford via Gcc-patches May 12, 2021, 2:55 p.m. | #1
On Tue, May 11, 2021 at 11:54:58PM +0200, Tobias Burnus wrote:
> The sfield / firstprivate lookup used the wrong var decl

> for the lookup – hence it failed.

> I used an extra long diff to make it easier to follow why

> 'c' and not 'detach_clause' has the proper clause for the

> decl to be used as key.

> 

> Testsuite run ongoing.

> OK for mainline, when it passes?

> 

> Tobias

> 

> -----------------

> Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf


> OpenMP: detach - fix firstprivate handling 

> gcc/ChangeLog:

> 

> 	* omp-low.c (finish_taskreg_scan): Use the proper detach decl.

> 

> libgomp/ChangeLog:

> 

> 	* testsuite/libgomp.fortran/detach-1.f90: New test.


LGTM, but please add also a libgomp.c-c++-common/task-detach-12.c testcase
like:
/* { dg-do run } */
/* { dg-options "-fopenmp" } */

#include <omp.h>

int
main ()
{
  struct S { int a[7]; } s = { { 1, 2, 3, 4, 5, 6, 7 } };
  omp_event_handle_t x;
  #pragma omp parallel master
  #pragma omp task firstprivate (s) detach (x)
    {
      if (s.a[3] != 4)
        __builtin_abort ();
      omp_fulfill_event (x);
    }
  return 0;
}

Also, please backport to 11 after a while.

Thanks.

	Jakub

Patch

OpenMP: detach - fix firstprivate handling 
gcc/ChangeLog:

	* omp-low.c (finish_taskreg_scan): Use the proper detach decl.

libgomp/ChangeLog:

	* testsuite/libgomp.fortran/detach-1.f90: New test.

diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index c0ce1a4990e..cadca7e201f 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -2436,55 +2436,55 @@  finish_taskreg_scan (omp_context *ctx)
 	  /* Look for a firstprivate clause with the detach event handle.  */
 	  for (c = gimple_omp_taskreg_clauses (ctx->stmt);
 	       c; c = OMP_CLAUSE_CHAIN (c))
 	    {
 	      if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_FIRSTPRIVATE)
 		continue;
 	      if (maybe_lookup_decl_in_outer_ctx (OMP_CLAUSE_DECL (c), ctx)
 		  == OMP_CLAUSE_DECL (detach_clause))
 		break;
 	    }
 
 	  gcc_assert (c);
 	  field = lookup_field (OMP_CLAUSE_DECL (c), ctx);
 
 	  /* Move field corresponding to the detach clause first.
 	     This is filled by GOMP_task and needs to be in a
 	     specific position.  */
 	  p = &TYPE_FIELDS (ctx->record_type);
 	  while (*p)
 	    if (*p == field)
 	      *p = DECL_CHAIN (*p);
 	    else
 	      p = &DECL_CHAIN (*p);
 	  DECL_CHAIN (field) = TYPE_FIELDS (ctx->record_type);
 	  TYPE_FIELDS (ctx->record_type) = field;
 	  if (ctx->srecord_type)
 	    {
-	      field = lookup_sfield (OMP_CLAUSE_DECL (detach_clause), ctx);
+	      field = lookup_sfield (OMP_CLAUSE_DECL (c), ctx);
 	      p = &TYPE_FIELDS (ctx->srecord_type);
 	      while (*p)
 		if (*p == field)
 		  *p = DECL_CHAIN (*p);
 		else
 		  p = &DECL_CHAIN (*p);
 	      DECL_CHAIN (field) = TYPE_FIELDS (ctx->srecord_type);
 	      TYPE_FIELDS (ctx->srecord_type) = field;
 	    }
 	}
       layout_type (ctx->record_type);
       fixup_child_record_type (ctx);
       if (ctx->srecord_type)
 	layout_type (ctx->srecord_type);
       tree t = fold_convert_loc (loc, long_integer_type_node,
 				 TYPE_SIZE_UNIT (ctx->record_type));
       if (TREE_CODE (t) != INTEGER_CST)
 	{
 	  t = unshare_expr (t);
 	  walk_tree (&t, finish_taskreg_remap, ctx, NULL);
 	}
       gimple_omp_task_set_arg_size (ctx->stmt, t);
       t = build_int_cst (long_integer_type_node,
 			 TYPE_ALIGN_UNIT (ctx->record_type));
       gimple_omp_task_set_arg_align (ctx->stmt, t);
     }
 }
diff --git a/libgomp/testsuite/libgomp.fortran/detach-1.f90 b/libgomp/testsuite/libgomp.fortran/detach-1.f90
new file mode 100644
index 00000000000..88546fe473b
--- /dev/null
+++ b/libgomp/testsuite/libgomp.fortran/detach-1.f90
@@ -0,0 +1,22 @@ 
+program test
+    use omp_lib
+    implicit none
+    integer(omp_event_handle_kind) :: oevent, ievent
+    integer :: i
+    integer, allocatable :: temp(:)
+    ALLOCATE(temp(5))
+
+    !$omp parallel num_threads(3)
+    !$omp single
+    DO i=1,5
+    !$omp task firstprivate(i) firstprivate(temp)  detach(oevent)
+          temp(:) = 0;
+          temp(1) = -1;
+          !print *,temp
+          call omp_fulfill_event(oevent)
+    !$omp end task
+    ENDDO
+    !$omp taskwait
+    !$omp end single
+    !$omp end parallel
+end program