[gomp4,05/14] omp-low: set 'omp target entrypoint' only on entypoints

On Wed, 21 Oct 2015, Jakub Jelinek wrote:
> > At the moment the attribute setting logic in omp-low.c is such that if a
> > function that should be present in target code does not already have 'omp
> > declare target' attribute, it receives 'omp target entrypoint'.  That is
> > wasteful: clearly not all user-declared target functions will be target region
> > entry points in OpenMP.
> > 
> > The motivating example for this change is OpenMP parallel target regions.  The
> > 'parallel' part is outlined into its own function.  We don't want that
> > function be an 'entrypoint' on PTX (but only as a matter of optimality rather
> > than correctness).
> > 
> > 	* omp-low.c (create_omp_child_function): Set "omp target entrypoint"
> >         or "omp declare target" attribute based on is_gimple_omp_offloaded.
> 
> This is principally ok, but you want to change it for 01/14.
> After that I think it is ready for trunk.

As you suggested in patch 01/14, I'll want to use different attributes to
distinguish between OpenACC and OpenMP entrypoints.  I've tried your
suggestion "acc target entrypoint", and unfortunately it doesn't work because
ipa-icf does: 

sem_function::parse (cgraph_node *node, bitmap_obstack *stack)
{
  ...
  if (lookup_attribute_by_prefix ("omp ", DECL_ATTRIBUTES (node->decl)) != NULL)
    return NULL;
  ...
}

so changing the prefix affects code generation.  I don't see why IPA-ICF needs
that (it's there from when the file was added to trunk, and there's no comment).
I went with "omp acc target entrypoint" instead, as below.

OK? (for trunk?)

Thanks.
Alexander

omp-low: set more precise attributes on offloaded functions

At the moment the attribute setting logic in omp-low.c is such that if a
function that should be present in target code does not already have 'omp
declare target' attribute, it receives 'omp target entrypoint'.  However, that
is inaccurate for OpenMP: functions outlined for e.g. omp-parallel regions do
not have 'omp declare target' attribute, but they cannot be entrypoints either.

Detect entrypoints using 'is_gimple_omp_offloaded'.  Additionally, distinguish
between OpenACC and OpenMP entrypoints, and use 'omp acc target entrypoint'
for the former.

        * omp-low.c (create_omp_child_function): Set "omp target entrypoint",
        "omp acc target entrypoint" or "omp declare target" attribute based on
        is_gimple_omp_offloaded and is_gimple_omp_oacc.
        * config/nvptx/nvptx.c (write_as_kernel): Test OpenACC-specific
        attribute "omp acc target entrypoint".  Add a comment about the OpenMP
        attribute handling.

On 10/30/2015 05:44 PM, Alexander Monakov wrote:
> +  /* Ignore "omp target entrypoint" here: OpenMP target region functions are
> +     called from gomp_nvptx_main.  The corresponding kernel entry is emitted
> +     from write_omp_entry.  */
>   }

I'm probably confused, but didn't we agree that this should be changed 
so that the entry point isn't gomp_nvptx_main but instead something that 
wraps a call to that function?

This patch creates a new "omp target entrypoint" annotation that appears 
not to be used - it would be better to just not annotate a function if 
it's not going to need entrypoint treatment. IMO a single type of 
attribute should be sufficient for that.


Bernd

On Fri, Nov 06, 2015 at 03:05:05PM +0100, Bernd Schmidt wrote:
> This patch creates a new "omp target entrypoint" annotation that appears not
> to be used - it would be better to just not annotate a function if it's not
> going to need entrypoint treatment. IMO a single type of attribute should be
> sufficient for that.

But NVPTX is just one backend, perhaps other backends need different
treatment of the entry points?

	Jakub

On 11/06/2015 03:08 PM, Jakub Jelinek wrote:
> On Fri, Nov 06, 2015 at 03:05:05PM +0100, Bernd Schmidt wrote:
>> This patch creates a new "omp target entrypoint" annotation that appears not
>> to be used - it would be better to just not annotate a function if it's not
>> going to need entrypoint treatment. IMO a single type of attribute should be
>> sufficient for that.
>
> But NVPTX is just one backend, perhaps other backends need different
> treatment of the entry points?

If we don't know, then it's not a problem we have to solve now. We can 
change it at any time later. For now, let's just keep it simple - no 
need to invent special annotations that end up unused.


Bernd