On 12/30/2017 10:54 AM, Jakub Jelinek wrote:
> On Fri, Dec 29, 2017 at 02:07:49PM +0100, Tom de Vries wrote:
>> --- a/gcc/lto-streamer-out.c
>> +++ b/gcc/lto-streamer-out.c
>> @@ -41,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see
>>   #include "builtins.h"
>>   #include "gomp-constants.h"
>>   #include "debug.h"
>> +#include "omp-offload.h"
>>   
>>   
>>   static void lto_write_tree (struct output_block*, tree, bool);
>> @@ -2355,6 +2356,31 @@ lto_output (void)
>>     int i, n_nodes;
>>     lto_symtab_encoder_t encoder = lto_get_out_decl_state ()->symtab_node_encoder;
>>   
>> +  bool truncated_p = false;
> 
> I don't think you need this var.
> 

Removed.

>> +  unsigned int write_index = 0;
>> +  for (unsigned read_index = 0; read_index < vec_safe_length (offload_funcs);
>> +       read_index++)
>> +    {
>> +      tree fn_decl = (*offload_funcs)[read_index];
>> +      bool remove_p = cgraph_node::get (fn_decl) == NULL;
>> +      if (remove_p)
>> +	{
>> +	  truncated_p = true;
>> +	  continue;
>> +	}
>> +
>> +      if (write_index != read_index)
>> +	(*offload_funcs)[write_index] = (*offload_funcs)[read_index];
>> +
>> +      write_index++;
>> +    }
>> +  if (truncated_p)
>> +    offload_funcs->truncate (write_index);
> 
> Either you truncate unconditionally, truncate is extremely cheap operation,
> or if you really wanted to guard it, you could just do
>    if (read_index != write_index)
> 

My concern was not the cost, but offload_funcs == NULL.

I've fixed this now by moving the code into a separate function and 
using the offload_funcs == NULL as early exit test.

>> +
>> +  if (!flag_lto)
>> +    for (unsigned i = 0; i < vec_safe_length (offload_funcs); i++)
>> +      DECL_PRESERVE_P ((*offload_funcs)[i]) = 1;
> 
> Can you please do this inside of the above loop, you have fn_decl already
> there, just do it after the
>        if (remove_p)
> 	continue;

Done.

> And, I think you can do it unconditionally at that point, 

Done. [ I wonder though if we can use in_lto_p as early exit test as well. ]

> or, can you use
> in_lto_p instead of flag_lto?  flag_lto is set even during the -flto
> compilation of the sources before LTO is streamed, there is no need to
> pessimize that code, we can still remove it, we just can't remove anything
> after we've streamed LTO bytecode (for either the host or offloading
> targets).
> 
>> @@ -7058,7 +7058,11 @@ expand_omp_target (struct omp_region *region)
>>   
>>         /* Add the new function to the offload table.  */
>>         if (ENABLE_OFFLOADING)
>> -	vec_safe_push (offload_funcs, child_fn);
>> +	{
>> +	  if (flag_lto)
>> +	    DECL_PRESERVE_P (child_fn) = 1;
> 
> And use if (in_lto_p) here too.
> 

Done.

> Ok for trunk with those changes.

Will commit after another round of testing.

Thanks,
- Tom