This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch, Fortran] PR32359 OpenMP threadprivate: SAVE is implied by explicit initialization
- From: Tobias Burnus <burnus at net-b dot de>
- To: Daniel Franke <franke dot daniel at gmail dot com>
- Cc: fortran at gcc dot gnu dot org, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Sat, 16 Jun 2007 16:36:33 +0200
- Subject: Re: [Patch, Fortran] PR32359 OpenMP threadprivate: SAVE is implied by explicit initialization
- References: <4673A625.2080700@net-b.de> <200706161255.37372.franke.daniel@gmail.com>
Daniel Franke wrote:
> any specific reason why you high-jacked that PR? I had it already assigned to me :)
>
I somehow missed that :-(
> Comments to your patch: it fixes only the one issue with threadprivate,
>
> Instead of fixes them one by one, I'd suggest to change the save-attribute in
> in 'symbol_attribute' from a boolean to a tri-state, say: SAVE_NONE = 0,
> SAVE_EXPLICIT, SAVE_IMPLICIT.
Granted, but attr.save is used only very rarely in the code and the use
seems to be ok. (I have to admit, I got lost in trans-array.c and
trans-common.c; I don't see whether attr.save or attr.save ==
SAVE_EXPLICIT is needed there. I think the change is ok with regards to
trans-*.c.)
However, your patch missed the following use of the "SAVE" statement:
integer :: y
save
How about something like the attached patch?
The testsuites libgomp & gfortran have no failures but:
gfortran.dg/module_md5_1.f90; after glancing at module.c I still don't
quite understand why, but I get the MD5 sum:
MD5:2acedc3d584fad6aae6248078b02aa38 instead of
MD5:18a257e13c90e3872b7b9400c2fc6e4b. As the distinction between
SAVE_IMPLICIT and SAVE_EXPLICIT is lost, when reading/writing a mod file
(the distinction should not be needed for use-associated symbols), I
don't see how the MD5 sum could possibly change.
Tobias
2007-06-16 Daniel Franke <franke.daniel@gmail.com>
Tobias Burnus <burnus@net-b.de>
PR fortran/32359
* gfortran.h (symbol_attribute): Change save attribute into an enum.
* decl.c (add_init_expr_to_sym): Set it to SAVE_IMPLICIT.
* symbol.c (gfc_add_save): Check for SAVE_EXPLICIT.
* resolve.c (resolve_fl_variable): Check for SAVE_EXPLICIT.
(resolve_symbol): Allow OMP threadprivate with
initialization SAVEd and save_all variable.
* trans-decl.c (gfc_finish_var_decl): Remove obsolete sym->value check.
2007-06-16 Tobias Burnus <burnus@net-b.de>
PR fortran/32359
* testsuite/libgomp.fortran/pr32359.f90: New.
Index: gcc/fortran/symbol.c
===================================================================
--- gcc/fortran/symbol.c (revision 125755)
+++ gcc/fortran/symbol.c (working copy)
@@ -885,7 +885,7 @@ gfc_add_save (symbol_attribute *attr, co
return FAILURE;
}
- if (attr->save)
+ if (attr->save == SAVE_EXPLICIT)
{
if (gfc_notify_std (GFC_STD_LEGACY,
"Duplicate SAVE attribute specified at %L",
@@ -894,7 +894,7 @@ gfc_add_save (symbol_attribute *attr, co
return FAILURE;
}
- attr->save = 1;
+ attr->save = SAVE_EXPLICIT;
return check_conflict (attr, name, where);
}
Index: gcc/fortran/decl.c
===================================================================
--- gcc/fortran/decl.c (revision 125755)
+++ gcc/fortran/decl.c (working copy)
@@ -1021,6 +1021,7 @@ add_init_expr_to_sym (const char *name,
}
sym->value = init;
+ sym->attr.save = SAVE_IMPLICIT;
*initp = NULL;
}
Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h (revision 125755)
+++ gcc/fortran/gfortran.h (working copy)
@@ -291,6 +291,12 @@ typedef enum ifsrc
}
ifsrc;
+/* Whether a SAVE attribute was set explicitly or implictly. */
+typedef enum save_state
+{ SAVE_NONE = 0, SAVE_EXPLICIT, SAVE_IMPLICIT
+}
+save_state;
+
/* Strings for all symbol attributes. We use these for dumping the
parse tree, in error messages, and also when reading and writing
modules. In symbol.c. */
@@ -558,10 +564,12 @@ typedef struct
{
/* Variable attributes. */
unsigned allocatable:1, dimension:1, external:1, intrinsic:1,
- optional:1, pointer:1, save:1, target:1, value:1, volatile_:1,
+ optional:1, pointer:1, target:1, value:1, volatile_:1,
dummy:1, result:1, assign:1, threadprivate:1, not_always_present:1,
implied_index:1;
+ ENUM_BITFIELD (save_state) save:2;
+
unsigned data:1, /* Symbol is named in a DATA statement. */
protected:1, /* Symbol has been marked as protected. */
use_assoc:1, /* Symbol has been use-associated. */
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c (revision 125755)
+++ gcc/fortran/resolve.c (working copy)
@@ -5780,8 +5780,9 @@ resolve_fl_variable (gfc_symbol *sym, in
}
}
- /* Also, they must not have the SAVE attribute. */
- if (flag && sym->attr.save)
+ /* Also, they must not have the SAVE attribute;
+ SAVE_IMPLICIT is checked below. */
+ if (flag && sym->attr.save == SAVE_EXPLICIT)
{
gfc_error (auto_save_msg, sym->name, &sym->declared_at);
return FAILURE;
@@ -6428,7 +6429,7 @@ resolve_symbol (gfc_symbol *sym)
gfc_resolve (sym->formal_ns);
/* Check threadprivate restrictions. */
- if (sym->attr.threadprivate && !sym->attr.save
+ if (sym->attr.threadprivate && !sym->attr.save && !sym->ns->save_all
&& (!sym->attr.in_common
&& sym->module == NULL
&& (sym->ns->proc_name == NULL
Index: libgomp/testsuite/libgomp.fortran/pr32359.f90
===================================================================
--- libgomp/testsuite/libgomp.fortran/pr32359.f90 (revision 0)
+++ libgomp/testsuite/libgomp.fortran/pr32359.f90 (revision 0)
@@ -0,0 +1,34 @@
+! { dg-do compile }
+!
+! PR fortran/32359
+! Contributed by Bill Long <longb@cray.com>
+
+subroutine test
+ use omp_lib
+ implicit none
+ integer, parameter :: NT = 4
+ integer :: a
+ save
+!$omp threadprivate(a)
+ a = 1
+
+!$ call omp_set_num_threads(NT)
+!$omp parallel
+ print *, omp_get_thread_num(), a
+!$omp end parallel
+
+end subroutine test
+
+! Derived from OpenMP test omp1/F2_6_2_8_5i.f90
+ use omp_lib
+ implicit none
+ integer, parameter :: NT = 4
+ integer :: a = 1
+!$omp threadprivate(a)
+
+!$ call omp_set_num_threads(NT)
+!$omp parallel
+ print *, omp_get_thread_num(), a
+!$omp end parallel
+
+ END