Re: [PATCH] Fix PR78027

[Cesar Philippidis <cesar at codesourcery dot com>]

On 12/09/2016 07:49 AM, Alexander Monakov wrote:
> On Fri, 9 Dec 2016, Cesar Philippidis wrote:
>>> Normally all offloaded code has either "omp declare target" (most functions) or
>>> "omp target entrypoint" (only toplevel offloaded code).  This used to include
>>> OpenACC functions: they would have "oacc fnattrib" _in_ _addition_ to those
>>> attributes, not _instead_ of them!  If something related to a recent OpenACC
>>> patch adds "oacc fnattrib" without at the same time adding "omp declare target",
>>> it's probable that other places in the compiler may break, not just IPA ICF.
>>>
>>> The explanation and the patch are written as if "oacc fnattrib" can appear
>>> without "omp declare target" -- ttbomk this could never happen before.
>>>
>>> Can this please be cleared up?
>>
>> It was more so to point out that there were cases where OpenACC
>> offloaded regions may not have omp target attributes. Referring back to
>> case 2 in the original email:
>>
>>   2. Nested offloaded regions inside 'omp declare target' functions do
>>   not have 'omp target entrypoint' attributes either.
>>
>> See the logic in omp-low.c:create_omp_child_function.
> 
> Let's see:
> 
>>  if (cgraph_node::get_create (decl)->offloadable
>>      && !lookup_attribute ("omp declare target",
>>                           DECL_ATTRIBUTES (current_function_decl)))
>>    {
>>      const char *target_attr = (is_gimple_omp_offloaded (ctx->stmt)
>>                                 ? "omp target entrypoint"
>>                                 : "omp declare target");
>>      DECL_ATTRIBUTES (decl)
>>        = tree_cons (get_identifier (target_attr),
>>                     NULL_TREE, DECL_ATTRIBUTES (decl));
>>    }
> 
> So, like I said -- offloaded functions have either "omp target entrypoint" or
> "omp declare function".  Either of those are caught by the _existing_ "omp "
> prefix check in IPA ICF.

How did you interpret this from that code?

> It doesn't matter that nested entrypoints might not have "omp target entrypoint"
> on them, because even if they have just "omp declare target" it's still subject
> to that ICF blacklist.
> 
> My objection still stands -- if you have "oacc fnattr" functions that have
> neither of those attributes, that doesn't seem right.

I'm confused. Are you implying that create_omp_child_function is
correct, or are you arguing for omp function attributes to always be
present on all offloaded functions, or both? Looking at this example again:

real function f()
   !$omp declare target(f)
   f = 1.
   !$acc parallel
   !$acc loop
   do i = 1, 8
   end do
   !$acc end parallel
   !$acc parallel
   !$acc loop
   do i = 1, 8
   end do
   !$acc end parallel
 end

Here an 'acc parallel' region is nested inside a function with an 'omp
declare target' attribute. Therefore, because current_function_decl, f,
already has an 'omp target declare' attribute, neither of the acc
parallel regions will get marked with an omp function attribute. Hence
the four solutions I listed earlier.

Cesar