On Wed, Oct 01, 2014 at 08:13:32PM +0400, Ilya Verbin wrote:
> @@ -1994,9 +1995,40 @@ output_in_order (bool no_reorder)
>    free (nodes);
>  }
>  
> +/* Check whether there is at least one function or global variable to 
> offload.
> +   */

The */ alone on a line is weird, put the last word on the next line too.

> +  FOR_EACH_DEFINED_VARIABLE (vnode)
> +    {
> +      if (!lookup_attribute ("omp declare target",
> +                          DECL_ATTRIBUTES (vnode->decl))
> +       || TREE_CODE (vnode->decl) != VAR_DECL
> +       || DECL_SIZE (vnode->decl) == 0)

While I hope the varpool code puts only decls that have DECL_ATTRIBUTES
into FOR_EACH_DEFINED_VARIABLE, it would be better to put the
less expensive tests first, i.e. the last two first, then lookup_attribute.
Also, DECL_SIZE is a tree, so == NULL_TREE?

And, once there is an option to select which offload targets to generate
code for (or none), initialize_offload () should supposedly return false
if the user requested no offloading on the command line.

The omp-low.c changes look good, for the cgraph/LTO stuff I defer to Honza
and/or Richard, if they are fine with the changes, so am I.

        Jakub

Reply via email to