Re: [PING^2][PATCH][3/3][PR65460] Mark offloaded functions as parallelized

[Thomas Schwinge <thomas@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 2794 bytes --]

Hi Tom!

On Tue, 9 Jun 2015 16:12:12 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 09/06/15 13:07, Richard Biener wrote:
> > On Mon, 8 Jun 2015, Tom de Vries wrote:
> >
> >> On 17/04/15 12:08, Tom de Vries wrote:
> >>> On 20-03-15 12:38, Tom de Vries wrote:
> >>>> On 19-03-15 12:05, Tom de Vries wrote:
> >>>>> On 18-03-15 18:22, Tom de Vries wrote:
> >>>>>> this patch fixes PR65460.
> >>>>>>
> >>>>>> The patch marks offloaded functions as parallelized, which means the
> >>>>>> parloops
> >>>>>> pass no longer attempts to modify that function.

> > Ok, but shouldn't it be set before calling add_new_function as
> > add_new_function might run passes that wouldn't identify the
> > function as parallelized?
> 
> Hm, indeed sometimes add_new_function executes some passes itself, 
> besides queueing the function for further processing. I suppose the 
> existing settings of parallelized_function should be modified in a 
> similar way.

(I took note of this small change of the trunk patch vs. the version
present on gomp-4_0-branch, and will adapt the latter as required as part
of the next merge.)

> I'll bootstrap and reg-test attached two patches on x86_64, and commit 
> unless objections.

> Mark offloaded functions as parallelized
> 
> 2015-06-09  Tom de Vries  <tom@codesourcery.com>
> 
> 	PR tree-optimization/65460
> 	* omp-low.c (expand_omp_target): Set parallelized_function on
> 	cgraph_node for child_fn.

(Committed to trunk in r224303.)

> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -8959,6 +8959,8 @@ expand_omp_target (struct omp_region *region)
>  
>        /* Inform the callgraph about the new function.  */
>        DECL_STRUCT_FUNCTION (child_fn)->curr_properties = cfun->curr_properties;
> +      cgraph_node *node = cgraph_node::get_create (child_fn);

Would you please committ a fix: in offloading configurations
(ENABLE_OFFLOADING), this new node variable will clash with an existing
definition a little later:

> +      node->parallelized_function = 1;
>        cgraph_node::add_new_function (child_fn, true);
>  
>  #ifdef ENABLE_OFFLOADING
|        /* Add the new function to the offload table.  */
|        vec_safe_push (offload_funcs, child_fn);
|  #endif
|  
|        /* Fix the callgraph edges for child_cfun.  Those for cfun will be
|  	 fixed in a following pass.  */
|        push_cfun (child_cfun);
|        cgraph_edge::rebuild_edges ();
|  
|  #ifdef ENABLE_OFFLOADING
|        /* Prevent IPA from removing child_fn as unreachable, since there are no
|  	 refs from the parent function to child_fn in offload LTO mode.  */
|        struct cgraph_node *node = cgraph_node::get (child_fn);
|        node->mark_force_output ();
|  #endif


Grüße,
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]