On 13.10.23 13:01, Jakub Jelinek wrote:
> On Tue, Oct 10, 2023 at 06:46:35PM +0200, Tobias Burnus wrote:
> +++ b/gcc/gimplify.cc
>> @@ -1400,23 +1400,53 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
>> +          else if (errorcount
>> +                   || (align == NULL_TREE
>> +                       && alloc != NULL_TREE
>> +                       && integer_onep (alloc)))
>> +            DECL_ATTRIBUTES (t) = remove_attribute ("omp allocate",
>> +                                                    DECL_ATTRIBUTES (t));
> Probably already preexisting, by I wonder how safe remove_attribute is.
> Aren't attributes shared in some cases
> (like __attribute__((attr1, attr2, attr3)) int a, b, c, d;)?

I think there should be no problem – new attributes get added as:

DECL_ATTRIBUTES (var) = tree_cons (get_identifier ("omp allocate"),
                                    t, DECL_ATTRIBUTES (var));

Thus, attr = DECL_ATTRIBUTES (var) is not shared, even if
TREE_CHAIN (attr) might be shared.

Thus, as long as new attributes get added at the head of the chain,
there should be no issue. And "omp allocate" is added once per decl
and is therefore not shared - removing might create again a shared
tree, but any following 'tree_cons' will again unshare it.

I think in your case, there would be indeed a problem when doing:
   'tree attr = remove_attr (...("attr2") ...)'
as this would remove "attr2" for 4 decls.

* * *

However, as 'omp allocate' is not used later on, I also do not need
to remove it.

=> Updated patch attached + interdiff for gimplify.cc attached.

Changes:
* Condition now the same as previously
* Keep 'omp allocate' also for DECL_VALUE_EXPR variable
* Add assert that we either have Fortran's expression the GOMP_FREE
   location - or a DECL_VALUE_EXPR.
* Make use of the assertion by keeping the HAS_DECL_VALUE expr below;
   this avoids adding an align/allocator == default check.
   => same code as old code, except for creating + using 'v' variable
   and adding a clobber.

* * *

> Otherwise LGTM (though, I didn't spot anything about allocatables in the
> patch, am I just looking wrong or are they still unsupported)?

It's unsupported – albeit Chung-Lin has some patch for it. The code path
is completely different. It already starts by 'omp allocators' (+
legacy: 'omp allocate') being not declarative but executable and
associated with a Fortran 'allocate'  statement and ...

Sorry for being slow - but I keep getting distracted by other tasks ...

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955