Kwok Cheung Yeung wrote:
I have added an extra is_ref argument to is_variable_sized that acts similarly to the argument to lower_private_allocate - when true, it returns true if the object that is referenced is variable sized. is_variable_sized (var, omp_privatize_by_reference (var)) is now used in place of is_fortran_variable_sized, and serves to handle Fortran VLAs and C++ references as well.

Just in case, I have now added checks for the size of memory allocated. Most common architectures that will be running OpenMP will have sizeof(int)==4 and sizeof(void*)==8, so I have guarded the checks with 'target int32' or 'target { lp64 || llp64 }'.

I have also added compile-time checks for Fortran statically-allocated arrays and C++ array references.

Thanks!

* * *

This patch generates calls to GOMP_alloc to allocate memory for firstprivate
and private clauses on target constructs with an allocator and alignment
as specified by the allocate clause.

The decl values of the clause need to be adjusted to refer to the allocated
memory, and the initial values of variables need to be copied into the
allocated space for firstprivate variables.

For variable-length arrays, the size of the array is stored in a separate
variable, so the allocation and initialization need to be delayed until the
size is made available on the target.
* * *
+      do_dtor:
+       if (allocator)
+         {
+           if (!is_gimple_val (allocator))
+             {
+               tree avar = create_tmp_var (TREE_TYPE (allocator));
+               gimplify_assign (avar, allocator, &alloc_dlist);
+               allocator = avar;
+             }
+           if (!is_gimple_val (allocate_ptr))
+             {
+               tree apvar = create_tmp_var (TREE_TYPE (allocate_ptr));
+               gimplify_assign (apvar, allocate_ptr, &alloc_dlist);
+               allocate_ptr = apvar;
+             }
+           tree f = builtin_decl_explicit (BUILT_IN_GOMP_FREE);
+           gimple *g = gimple_build_call (f, 2, allocate_ptr, allocator);

The call 'omp_free (ptr, allocator)' is fine, but as OpenMP permits also
'omp_free (ptr, omp_null_allocator)' (i.e. passing NULL), libgomp has to
handle the latter already - and as implemented in libgomp, the allocator
argument is completely ignored.
(The allocator is stored with the pointer at address 'ptr - offset'.)

Thus, to safe not used code, just 0 could be passed. On the other hand,
it does not seem to be too bad in terms of register used, optimization etc.
to keep the current code.

Thus, either is fine - leaving is is or passing 0.

* * *

+               if (omp_privatize_by_reference (var))
+                 {
+                   x = fold_convert (TREE_TYPE (new_var), *allocate_ptr);
+                   gimplify_assign (new_var, x, &new_body);
+                 }
+
+               new_var = omp_privatize_by_reference (var)
+                   ? build_fold_indirect_ref (new_var)
+                   : build_simple_mem_ref (*allocate_ptr);
+             }

Can you move the assignment into true path into the if block
and add false path as 'else'?

'omp_privatize_by_reference' is not really heavy weighted but
for Fortran, gfc_omp_privatize_by_reference is still some
code - and calling it twice is pointless. The compiler cannot
see that it won't change.

Repeated omp_privatize_by_reference also occurs elsewhere,
but there one can argue whether a bool variable should be use
or not (less readable code, adding another variable, ...).
But here it just makes sense.

* * *

LGTM - thanks for the patch!

Tobias

Reply via email to