Re: [Patch] OpenMP: Handle cpp_implicit_alias in declare-target discovery (PR96390)

[Jakub Jelinek <jakub@redhat.com>]

On Mon, Sep 14, 2020 at 03:25:29PM +0200, Tobias Burnus wrote:
> Updated version attached. Does it seem to make sense?

I think you want Honza on this primarily, I'm always lost in the cgraph
alias code.

> --- a/gcc/omp-offload.c
> +++ b/gcc/omp-offload.c
> @@ -196,21 +196,34 @@ omp_declare_target_var_p (tree decl)
>  static tree
>  omp_discover_declare_target_tgt_fn_r (tree *tp, int *walk_subtrees, void *data)
>  {
> -  if (TREE_CODE (*tp) == FUNCTION_DECL
> -      && !omp_declare_target_fn_p (*tp)
> -      && !lookup_attribute ("omp declare target host", DECL_ATTRIBUTES (*tp)))
> +  if (TREE_CODE (*tp) == FUNCTION_DECL)
>      {
> -      tree id = get_identifier ("omp declare target");
> -      if (!DECL_EXTERNAL (*tp) && DECL_SAVED_TREE (*tp))
> -	((vec<tree> *) data)->safe_push (*tp);
> -      DECL_ATTRIBUTES (*tp) = tree_cons (id, NULL_TREE, DECL_ATTRIBUTES (*tp));
> +      tree decl = *tp;
>        symtab_node *node = symtab_node::get (*tp);
>        if (node != NULL)
>  	{
> +	  while (node->alias_target)
> +	    node = symtab_node::get (node->alias_target);
> +	  node = node->ultimate_alias_target ();

I think the above is either you walk the aliases yourself, or use
ultimate_alias_target, but not both.  For the first one, e.g.
ultimate_alias_target_1 uses
      if (node->alias && node->analyzed)
        node = node->get_alias_target ();
in the loop.

And the second thing is, I'm not sure how the aliases behave if the
ultimate alias target is properly marked as omp declare target, but some
of the aliases are not.  The offloaded code will still call the alias,
so do we somehow arrange for the aliases to be also emitted into the
offloading LTO IL?
We don't really need to push the aliases into ((vec<tree> *) data),
that is only for function definitions (what will needs to be scanned for
further references), but I wonder if the aliases that are needed shouldn't
be marked node->offloadable and have "omp declare target" attribute added
for them too.

Perhaps use ultimate_alias_target (); first, handle the ultimate alias,
but keep the old node around, and if different from the ultimate alias,
do the node = node->get_alias_target () loop until you reach the ultimate
alias and for each add "omp declare target" and set node->offloadable
if not already there?

> +	  decl = node->decl;
> +	  if (omp_declare_target_fn_p (decl)
> +	      || lookup_attribute ("omp declare target host",
> +				   DECL_ATTRIBUTES (decl)))
> +	    return NULL_TREE;
> +
>  	  node->offloadable = 1;
>  	  if (ENABLE_OFFLOADING)
>  	    g->have_offload = true;
>  	}
> +      else if (omp_declare_target_fn_p (decl)
> +	       || lookup_attribute ("omp declare target host",
> +				    DECL_ATTRIBUTES (decl)))
> +	return NULL_TREE;
> +
> +      tree id = get_identifier ("omp declare target");
> +      if (!DECL_EXTERNAL (decl) && DECL_SAVED_TREE (decl))
> +	((vec<tree> *) data)->safe_push (decl);
> +      DECL_ATTRIBUTES (decl) = tree_cons (id, NULL_TREE, DECL_ATTRIBUTES (decl));
>      }
>    else if (TYPE_P (*tp))
>      *walk_subtrees = 0;

	Jakub