[gomp] Don't gimplify to DECL_VALUE_EXPR vars that are going to be remapped

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

gfortran.dg/gomp/appendix-a/a.24.1.f90 showed a problem where
for a COMMON var extra bogus error has been emitted.  omp_notice_variable
reported correctly error about the COMMON var not being predetermined
nor explicitly determined, but then it was gimplified to its
DECL_VALUE_EXPR, which is COMPONENT_REF of the COMMON block variable
and as the COMMON block variable wasn't mentioned either, another error
has been reported.
But as shown on the attached testcase, the problem is much worse.  By
gimplifying the variable to its DECL_VALUE_EXPR we loose the variable
references in the source before omp lowering and thus the vars end up going
shared even when they ought to be privatized.

The following patch fixes it.  Is it ok this way or should I instead
create a langhook that would decide if a var should be gimplified to
its DECL_VALUE_EXPR immediately or it should be kept as is until
remapping during omp lowering?

2005-10-24  Jakub Jelinek  <jakub@redhat.com>

	* gimplify.c (omp_notice_variable): Change return type to bool.
	Return true if DECL is going to be remapped during omp lowering.
	(gimplify_var_or_parm_decl): Don't gimplify DECL to its
	DECL_VALUE_EXPR if omp_notice_variables returned true and DECL
	is not variable-sized.
libgomp/
	* libgomp.fortran/appendix-a/a.28.5.f90: Change into compile
	only test.
	* libgomp.fortran/sharing1.f90: New test.

--- gcc/gimplify.c.jj	2005-10-24 08:56:41.000000000 +0200
+++ gcc/gimplify.c	2005-10-24 13:22:13.000000000 +0200
@@ -281,7 +281,7 @@ delete_omp_context (struct gimplify_omp_
 }
 
 static void omp_add_variable (struct gimplify_omp_ctx *, tree, unsigned int);
-static void omp_notice_variable (struct gimplify_omp_ctx *, tree, bool);
+static bool omp_notice_variable (struct gimplify_omp_ctx *, tree, bool);
 
 /* A subroutine of append_to_statement_list{,_force}.  T is not NULL.  */
 
@@ -1589,7 +1589,14 @@ gimplify_var_or_parm_decl (tree *expr_p)
 
   /* When within an OpenMP context, notice uses of variables.  */
   if (gimplify_omp_ctxp)
-    omp_notice_variable (gimplify_omp_ctxp, decl, true);
+    if (omp_notice_variable (gimplify_omp_ctxp, decl, true))
+      {
+	/* Variable-sized DECLs need to be gimplified to its DECL_VALUE_EXPR
+	   immediately.  Don't consider DECL_VALUE_EXPR of other DECLs if they
+	   are going to be remapped until their remapping.  */
+	if (TREE_CONSTANT (TYPE_SIZE_UNIT (TREE_TYPE (decl))))
+	  return GS_ALL_DONE;
+      }
 
   /* If the decl is an alias for another expression, substitute it now.  */
   if (DECL_HAS_VALUE_EXPR_P (decl))
@@ -4274,26 +4281,29 @@ omp_add_variable (struct gimplify_omp_ct
 
 /* Record the fact that DECL was used within the OpenMP context CTX.
    IN_CODE is true when real code uses DECL, and false when we should
-   merely emit default(none) errors.  */
+   merely emit default(none) errors.  Return true if DECL is going to
+   be remapped and thus DECL shouldn't be gimplified into its
+   DECL_VALUE_EXPR (if any).  */
 
-static void
+static bool
 omp_notice_variable (struct gimplify_omp_ctx *ctx, tree decl, bool in_code)
 {
   splay_tree_node n;
   unsigned flags = in_code ? GOVD_SEEN : 0;
+  bool ret = false;
 
   /* Threadprivate variables are predetermined.  */
   if (is_global_var (decl))
     {
       if (DECL_THREAD_LOCAL_P (decl))
-	return;
+	return false;
 
       if (DECL_HAS_VALUE_EXPR_P (decl))
 	{
 	  tree value = get_base_address (DECL_VALUE_EXPR (decl));
 
 	  if (value && DECL_P (value) && DECL_THREAD_LOCAL_P (value))
-	    return;
+	    return false;
 	}
     }
 
@@ -4305,6 +4315,8 @@ omp_notice_variable (struct gimplify_omp
       if (!ctx->is_parallel)
 	goto do_outer;
 
+      ret = true;
+
       /* ??? Some compiler-generated variables (like SAVE_EXPRs) could be
 	 remapped firstprivate instead of shared.  To some extent this is
 	 addressed in omp_firstprivatize_type_sizes, but not effectively.  */
@@ -4339,9 +4351,11 @@ omp_notice_variable (struct gimplify_omp
       goto do_outer;
     }
 
+  ret = true;
+
   /* If nothing changed, there's nothing left to do.  */
   if ((n->value & flags) == flags)
-    return;
+    return ret;
   flags |= n->value;
   n->value = flags;
 
@@ -4349,9 +4363,11 @@ omp_notice_variable (struct gimplify_omp
   /* If the variable is private in the current context, then we don't
      need to propagate anything to an outer context.  */
   if (flags & GOVD_PRIVATE)
-    return;
-  if (ctx->outer_context)
-    omp_notice_variable (ctx->outer_context, decl, in_code);
+    return ret;
+  if (ctx->outer_context
+      && omp_notice_variable (ctx->outer_context, decl, in_code))
+    return true;
+  return ret;
 }
 
 /* Verify that DECL is private within CTX.  If there's specific information
--- libgomp/testsuite/libgomp.fortran/appendix-a/a.28.5.f90.jj	2005-10-18 01:10:59.000000000 +0200
+++ libgomp/testsuite/libgomp.fortran/appendix-a/a.28.5.f90	2005-10-24 12:01:33.000000000 +0200
@@ -1,4 +1,4 @@
-! { dg-do run }
+! { dg-do compile }
 
       SUBROUTINE SUB1(X)
         DIMENSION X(10)
--- libgomp/testsuite/libgomp.fortran/sharing1.f90.jj	2005-10-24 11:31:31.000000000 +0200
+++ libgomp/testsuite/libgomp.fortran/sharing1.f90	2005-10-24 11:31:26.000000000 +0200
@@ -0,0 +1,29 @@
+! { dg-do run }
+
+  use omp_lib
+  integer :: i, j, k
+  logical :: l
+  common /b/ i, j
+  i = 4
+  j = 8
+  l = .false.
+!$omp parallel private (k) firstprivate (i) shared (j) num_threads (2) &
+!$omp& reduction (.or.:l)
+  if (i .ne. 4 .or. j .ne. 8) l = .true.
+!$omp barrier
+  k = omp_get_thread_num ()
+  if (k .eq. 0) then
+    i = 14
+    j = 15
+  end if
+!$omp barrier
+  if (k .eq. 1) then
+    if (i .ne. 4 .or. j .ne. 15) l = .true.
+    i = 24
+    j = 25
+  end if
+!$omp barrier
+  if (j .ne. 25 .or. i .ne. (k * 10 + 14)) l = .true.
+!$omp end parallel
+  if (l .or. j .ne. 25) call abort
+end


	Jakub