Re: [Patch 1/2][ipa] Add target hook to sanitize cloned declaration's attributes

[Martin Jambor <mjambor at suse dot cz>]

Hi,

On Wed, Oct 09 2019, Andre Vieira (lists) wrote:
> Hi Martin,
>
> Thanks for the suggestion.  I have moved the target hook to the 
> following functions:
> 'create_clone'
> 'create_version_clone'

I am not sure that calling the hook in create_version_clone is what you
want.  It should perhaps be made clearer in the API but unlike
(functions that call) create_clone and create_version_clone_with_body,
the result is not a local symbol but it may also be an externally
visible "version" of the original.  OpenMP uses it to create nodes for
external declarations (not definitions!) of variants of functions marked
with omp declare simd and the function calling it in trans-mem.c also
apparently uses it to create possibly externally visible clones.

So, if the idea of the patch is to allow stripping some attributes from
functions that are not externally visible - which I believe it is but I
may be wrong - you do not want to do that here.  Alternatively, the hook
itself could check DECL_EXTERNAL (node->decl).  Whether you decide to
invoke the call for non-local clones or not, please include it in the
hook description.

> 'create_version_clone_with_body'

The other two additions look OK to me.  You might want to check that the
attribute stripping works with LTO (but I tend to think that attributes
are streamed for WPA so it should be OK).

>
> I looked for all 'cgraph_node::create_*' functions and added the target 
> hook to them. The exception is 'create_virtual_clone' which calls 
> 'create_clone' so it should be caught there.
>
> Is this OK for trunk?

I'm afraid you'll need an ACK from a global reviewer for something like
this.

Martin


>
> Cheers,
> Andre
>
> gcc/ChangeLog:
>
> 2019-10-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
>
> 	* cgraphclones.c(create_clone): Call new target hook when
>          creating a new node.
>          (create_version_clone): Likewise.
>          (create_version_clone_with_body): Likewise.
>          * target.def(sanitize_clone_attributes): New target hook.
>          * targhooks.c(default_sanitize_clone_attributes): New.
>          * targhooks.h(default_sanitize_clone_attributes): New.
>          * doc/tm.texi.in: Document new target hook.
>          * doc/tm.texi: Regenerated