This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [gomp] Fix multiple threadprivate declarations (PR 24455)
On Friday 21 October 2005 01:13, Richard Henderson wrote:
> I'd much rather you use the lang bit in the c front end and the lang_decl
> sub field in the c++ front end.
>
Sure.
> > + if (DECL_THREAD_LOCAL_P (newdecl) != DECL_THREAD_LOCAL_P
> > (olddecl) + && !DECL_THREADPRIVATE_P (olddecl))
>
> extern int x;
> #pragma omp threadprivate(x)
> extern __thread int x __attribute__((tls_model ("initial-exec")));
>
> should, I would think, yield an error. So something like
>
> if (DECL_THREADPRIVATE_P (olddecl)
> && !DECL_THREAD_LOCAL_P (newdecl))
> ok;
> else if (DECL_TLS_MODEL (olddecl) != DECL_TLS_MODEL (newdecl))
> error;
>
Well, OK. But I think this also exposes another bug in the generic TLS
code. With a check like this we would start failing gcc.dg/tls/diag-3.c.
I had added the following hunk to diagnose_mismatched_decls:
+ else if (DECL_TLS_MODEL (newdecl) != DECL_TLS_MODEL (olddecl))
+ {
+ error ("thread-local declaration of %q+D inconsistent "
+ "with previous thread-local declaration", newdecl);
+
+ locate_old_decl (olddecl, error);
+ return false;
+ }
9 extern __thread int k; /* This is fine. */
10 __thread int k;
gcc.dg/tls/diag-3.c:10: error: thread-local declaration of 'k' inconsistent
with previous thread-local declaration
gcc.dg/tls/diag-3.c:9: error: previous declaration of 'k' was here
Sure enough, we get different TLS models for these two declarations:
1408 error ("thread-local declaration of %q+D inconsistent "
(gdb) p newdecl.decl_with_vis.tls_model
$1 = TLS_MODEL_LOCAL_EXEC
(gdb) p olddecl.decl_with_vis.tls_model
$2 = TLS_MODEL_INITIAL_EXEC
(gdb)
I don't know enough about TLS to determine whether the check should be more
careful and accept certain combinations of different TLS models, or if
this is indeed some bug upstream from it.
My plan is to not add this hunk for now (it's not included in the patch
that I'm planning to commit). If you give me a couple of pointers, I
could look at fixing this in mainline.
2005-10-21 Diego Novillo <dnovillo@redhat.com>
gcc/
PR 24455
* c-tree.h (C_DECL_THREADPRIVATE_P): Define.
* c-parser.c (c_parser_omp_threadprivate): Set.
Do not error out if C_DECL_THREADPRIVATE_P is set already.
* c-decl.c (diagnose_mismatched_decls): Do not check for
mismatched thread-local attributes when OLDDECL is marked
threadprivate and NEWDECL has no thread-local attributes.
(merge_decls): Merge C_DECL_THREADPRIVATE_P.
gcc/cp/
PR 24455
* cp/cp-tree.h (struct lang_decl_flags): Add field
threadprivate_p.
(CP_DECL_IS_THREADPRIVATE): Define.
* cp/semantics.c (finish_omp_threadprivate): Set.
Do not error out if CP_DECL_IS_THREADPRIVATE is set already.
* cp/decl.c (duplicate_decls): Merge CP_DECL_THREADPRIVATE_P.
libgomp/
PR 24455
* testsuite/libgomp.c++/pr24455-1.C: New test.
* testsuite/libgomp.c++/pr24455.C: New test.
* testsuite/libgomp.dg/pr24455-1.c: New test.
* testsuite/libgomp.dg/pr24455.c: New test.
Index: gcc/c-decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-decl.c,v
retrieving revision 1.662.4.12
diff -d -u -p -r1.662.4.12 c-decl.c
--- gcc/c-decl.c 9 Oct 2005 01:01:21 -0000 1.662.4.12
+++ gcc/c-decl.c 21 Oct 2005 15:21:42 -0000
@@ -1384,7 +1384,14 @@ diagnose_mismatched_decls (tree newdecl,
{
/* Only variables can be thread-local, and all declarations must
agree on this property. */
- if (DECL_THREAD_LOCAL_P (newdecl) != DECL_THREAD_LOCAL_P (olddecl))
+ if (C_DECL_THREADPRIVATE_P (olddecl) && !DECL_THREAD_LOCAL_P (newdecl))
+ {
+ /* Nothing to check. Since OLDDECL is marked threadprivate
+ and NEWDECL does not have a thread-local attribute, we
+ will merge the threadprivate attribute into NEWDECL. */
+ ;
+ }
+ else if (DECL_THREAD_LOCAL_P (newdecl) != DECL_THREAD_LOCAL_P (olddecl))
{
if (DECL_THREAD_LOCAL_P (newdecl))
error ("thread-local declaration of %q+D follows "
@@ -1671,6 +1686,13 @@ merge_decls (tree newdecl, tree olddecl,
if (DECL_INITIAL (newdecl) == 0)
DECL_INITIAL (newdecl) = DECL_INITIAL (olddecl);
+ /* Merge the threadprivate attribute. */
+ if (TREE_CODE (olddecl) == VAR_DECL && C_DECL_THREADPRIVATE_P (olddecl))
+ {
+ DECL_TLS_MODEL (newdecl) = DECL_TLS_MODEL (olddecl);
+ C_DECL_THREADPRIVATE_P (newdecl) = 1;
+ }
+
if (CODE_CONTAINS_STRUCT (TREE_CODE (olddecl), TS_DECL_WITH_VIS))
{
/* Merge the unused-warning information. */
Index: gcc/c-parser.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-parser.c,v
retrieving revision 2.17.4.37
diff -d -u -p -r2.17.4.37 c-parser.c
--- gcc/c-parser.c 21 Oct 2005 00:04:23 -0000 2.17.4.37
+++ gcc/c-parser.c 21 Oct 2005 15:21:43 -0000
@@ -7698,10 +7698,15 @@ c_parser_omp_threadprivate (c_parser *pa
{
tree v = TREE_PURPOSE (t);
- if (TREE_USED (v))
+ /* If V had already been marked threadprivate, it doesn't matter
+ whether it had been used prior to this point. */
+ if (TREE_USED (v) && !C_DECL_THREADPRIVATE_P (v))
error ("%qE declared %<threadprivate%> after first use", v);
else
- DECL_TLS_MODEL (v) = decl_default_tls_model (v);
+ {
+ DECL_TLS_MODEL (v) = decl_default_tls_model (v);
+ C_DECL_THREADPRIVATE_P (v) = 1;
+ }
}
c_parser_skip_to_pragma_eol (parser);
Index: gcc/c-tree.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-tree.h,v
retrieving revision 1.203.4.10
diff -d -u -p -r1.203.4.10 c-tree.h
--- gcc/c-tree.h 9 Oct 2005 01:01:21 -0000 1.203.4.10
+++ gcc/c-tree.h 21 Oct 2005 15:21:43 -0000
@@ -129,6 +129,10 @@ struct lang_type GTY(())
#define C_DECL_UNDEFINABLE_VM(EXP) \
DECL_LANG_FLAG_5 (LABEL_DECL_CHECK (EXP))
+/* Record whether a variable has been declared threadprivate by
+ #pragma omp threadprivate. */
+#define C_DECL_THREADPRIVATE_P(DECL) DECL_LANG_FLAG_3 (VAR_DECL_CHECK (DECL))
+
/* Nonzero for a decl which either doesn't exist or isn't a prototype.
N.B. Could be simplified if all built-in decls had complete prototypes
(but this is presently difficult because some of them need FILE*). */
Index: gcc/cp/cp-tree.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/cp-tree.h,v
retrieving revision 1.1144.4.13
diff -d -u -p -r1.1144.4.13 cp-tree.h
--- gcc/cp/cp-tree.h 21 Oct 2005 00:04:25 -0000 1.1144.4.13
+++ gcc/cp/cp-tree.h 21 Oct 2005 15:21:46 -0000
@@ -1519,7 +1519,8 @@ struct lang_decl_flags GTY(())
unsigned this_thunk_p : 1;
unsigned repo_available_p : 1;
unsigned hidden_friend_p : 1;
- unsigned dummy : 2;
+ unsigned threadprivate_p : 1;
+ /* One unused bit. */
union lang_decl_u {
/* In a FUNCTION_DECL for which DECL_THUNK_P holds, this is
@@ -2346,6 +2347,11 @@ extern void decl_shadowed_for_var_insert
#define DECL_HIDDEN_FRIEND_P(NODE) \
(DECL_LANG_SPECIFIC (DECL_COMMON_CHECK (NODE))->decl_flags.hidden_friend_p)
+/* Nonzero if DECL has been declared threadprivate by
+ #pragma omp threadprivate. */
+#define CP_DECL_THREADPRIVATE_P(DECL) \
+ (DECL_LANG_SPECIFIC (VAR_DECL_CHECK (DECL))->decl_flags.threadprivate_p)
+
/* Record whether a typedef for type `int' was actually `signed int'. */
#define C_TYPEDEF_EXPLICITLY_SIGNED(EXP) DECL_LANG_FLAG_1 (EXP)
Index: gcc/cp/decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.1403.4.9
diff -d -u -p -r1.1403.4.9 decl.c
--- gcc/cp/decl.c 17 Oct 2005 00:40:49 -0000 1.1403.4.9
+++ gcc/cp/decl.c 21 Oct 2005 15:21:48 -0000
@@ -1546,6 +1546,18 @@ duplicate_decls (tree newdecl, tree oldd
|= DECL_NONTRIVIALLY_INITIALIZED_P (olddecl);
DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (newdecl)
|= DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (olddecl);
+
+ /* Merge the threadprivate attribute from OLDDECL into NEWDECL. */
+ if (DECL_LANG_SPECIFIC (olddecl)
+ && CP_DECL_THREADPRIVATE_P (olddecl))
+ {
+ /* Allocate a LANG_SPECIFIC structure for NEWDECL, if needed. */
+ if (!DECL_LANG_SPECIFIC (newdecl))
+ retrofit_lang_decl (newdecl);
+
+ DECL_TLS_MODEL (newdecl) = DECL_TLS_MODEL (olddecl);
+ CP_DECL_THREADPRIVATE_P (newdecl) = 1;
+ }
}
/* Do this after calling `merge_types' so that default
Index: gcc/cp/semantics.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/semantics.c,v
retrieving revision 1.475.4.12
diff -d -u -p -r1.475.4.12 semantics.c
--- gcc/cp/semantics.c 20 Oct 2005 09:54:19 -0000 1.475.4.12
+++ gcc/cp/semantics.c 21 Oct 2005 15:21:49 -0000
@@ -3534,10 +3534,21 @@ finish_omp_threadprivate (tree vars)
{
tree v = TREE_PURPOSE (t);
- if (TREE_USED (v))
+ /* If V had already been marked threadprivate, it doesn't matter
+ whether it had been used prior to this point. */
+ if (TREE_USED (v)
+ && (DECL_LANG_SPECIFIC (v) == NULL_TREE
+ || !CP_DECL_THREADPRIVATE_P (v)))
error ("%qE declared %<threadprivate%> after first use", v);
else
- DECL_TLS_MODEL (v) = decl_default_tls_model (v);
+ {
+ /* Allocate a LANG_SPECIFIC structure for V, if needed. */
+ if (!DECL_LANG_SPECIFIC (v))
+ retrofit_lang_decl (v);
+
+ DECL_TLS_MODEL (v) = decl_default_tls_model (v);
+ CP_DECL_THREADPRIVATE_P (v) = 1;
+ }
}
}
Index: libgomp/testsuite/libgomp.c++/pr24455-1.C
===================================================================
RCS file: libgomp/testsuite/libgomp.c++/pr24455-1.C
diff -N libgomp/testsuite/libgomp.c++/pr24455-1.C
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ libgomp/testsuite/libgomp.c++/pr24455-1.C 21 Oct 2005 15:21:59 -0000
@@ -0,0 +1,5 @@
+/* { dg-do compile } */
+extern int i;
+#pragma omp threadprivate (i)
+
+int i;
Index: libgomp/testsuite/libgomp.c++/pr24455.C
===================================================================
RCS file: libgomp/testsuite/libgomp.c++/pr24455.C
diff -N libgomp/testsuite/libgomp.c++/pr24455.C
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ libgomp/testsuite/libgomp.c++/pr24455.C 21 Oct 2005 15:21:59 -0000
@@ -0,0 +1,22 @@
+/* { dg-do run } */
+/* { dg-additional-sources pr24455-1.C } */
+
+extern "C" void abort (void);
+
+extern int i;
+#pragma omp threadprivate(i)
+
+int main()
+{
+ i = 0;
+
+#pragma omp parallel default(none) num_threads(10)
+ {
+ i++;
+#pragma omp barrier
+ if (i != 1)
+ abort ();
+ }
+
+ return 0;
+}
Index: libgomp/testsuite/libgomp.dg/pr24455-1.c
===================================================================
RCS file: libgomp/testsuite/libgomp.dg/pr24455-1.c
diff -N libgomp/testsuite/libgomp.dg/pr24455-1.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ libgomp/testsuite/libgomp.dg/pr24455-1.c 21 Oct 2005 15:21:59 -0000
@@ -0,0 +1,5 @@
+/* { dg-do compile } */
+extern int i;
+#pragma omp threadprivate (i)
+
+int i;
Index: libgomp/testsuite/libgomp.dg/pr24455.c
===================================================================
RCS file: libgomp/testsuite/libgomp.dg/pr24455.c
diff -N libgomp/testsuite/libgomp.dg/pr24455.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ libgomp/testsuite/libgomp.dg/pr24455.c 21 Oct 2005 15:21:59 -0000
@@ -0,0 +1,22 @@
+/* { dg-do run } */
+/* { dg-additional-sources pr24455-1.c } */
+
+extern void abort (void);
+
+extern int i;
+#pragma omp threadprivate(i)
+
+int main()
+{
+ i = 0;
+
+#pragma omp parallel default(none) num_threads(10)
+ {
+ i++;
+#pragma omp barrier
+ if (i != 1)
+ abort ();
+ }
+
+ return 0;
+}