------- Comment #2 from jakub at gcc dot gnu dot org 2006-04-28 15:07 -------
One incremental fix for the global var case is:
--- omp-low.c.jj5 2006-04-28 13:29:49.000000000 +0200
+++ omp-low.c 2006-04-28 16:22:36.000000000 +0200
@@ -674,9 +674,6 @@ omp_copy_decl (tree var, copy_body_data
omp_context *ctx = (omp_context *) cb;
tree new_var;
- if (is_global_var (var) || decl_function_context (var) != ctx->cb.src_fn)
- return var;
-
if (TREE_CODE (var) == LABEL_DECL)
{
new_var = create_artificial_label ();
@@ -695,6 +692,9 @@ omp_copy_decl (tree var, copy_body_data
return new_var;
}
+ if (is_global_var (var) || decl_function_context (var) != ctx->cb.src_fn)
+ return var;
+
return error_mark_node;
}
without this we don't remap privatized global vars except directly in the
omp context that privatized them.
Testcase I was looking at is:
extern void abort (void);
extern void omp_set_dynamic (int);
int n = 6;
int
main (void)
{
int i, x = 0;
omp_set_dynamic (0);
#pragma omp parallel for num_threads (16) firstprivate (n) lastprivate (n) \
schedule (static, 1) reduction (+: x)
for (i = 0; i < 16; i++)
{
if (n != 6)
++x;
n = i;
}
if (x || n != 15)
abort ();
return 0;
}
With the above patch, we still create wrong code, with 2 different bugs:
1) there is no barrier to separate firstprivate assignments from lastprivate
2) on the sender side, we store the global var n into
.omp_data_o.4D.1945.nD.1938, but on the receiver side we access either the
remapped private n (assuming the above patch is in) or, when accessing the
outside n we use the global variable n rather than .omp_data_iD.1937.nD.1938
To fix 2), we need to decide where to pass global vars, either the sender side
will do nothing and receiver side will remain as is, or the receiver side will
use .omp_data_i fields even for global vars. I'd say the sender side should be
changed unless we have some important reason why we need to do an extra copy
(if passed by reference, there is certainly no point in doing that).
As for 1), I believe that whenever there is any
OMP_CLAUSE_LASTPRIVATE_FIRSTPRIVATE clause we need to do the same as we do
for copyin_by_ref, at least for the time being (later on we could replace that
by a more sofisticated special case barrier implementation), because
even if the firstprivate/lastprivate var is a scalar passed by value in
.omp_data_* struct, firstprivate copying copies it from that struct and
lastprivate copying writes it into the same struct same field, so if say
one of threads is delayed for whatever reason, it will execute firstprivate
copying after other thread that handled the last case performed the lastprivate
copying back (well, scalar by value could perhaps be done even without that
by allocating two fields instead of one, one write only and one readonly, but
by reference/is_reference/VLA need to be with barrier in all cases).
In addition to this, I'm not sure what exactly the standard requires say when
firstprivate is used on a global var. The global var can be visible to the
threads and they can modify it, do we need to make any guarantees about what
exact value gets assigned to the privatized n variable in various threads?
As in, is the following program making a valid assumption or not?
int n = 2;
void bar (void) { n = 4; }
void
foo (void)
{
if (n != 2)
return;
#pragma omp parallel firstprivate (n)
{
if (n != 2)
abort ();
bar ();
}
}
}
--
jakub at gcc dot gnu dot org changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |dnovillo at gcc dot gnu dot
| |org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26943
