Re: Order symbols before section copying in the lto streamer

["Martin Liška" <mliska@suse.cz>]

On 10/23/19 10:02 PM, Jan Hubicka wrote:
>> Hi,
>> this patch orders symbols where we copy sections to match the order
>> of files in the command line.  This optimizes streaming process since we
>> are not opening and closing files randomly and also we read them more
>> sequentially.  This saves some kernel time though I think more can be
>> done if we avoid doing pair of mmap/unmap for every file section we
>> read.
>>
>> We also read files in random order in ipa-cp and during devirt.
>> I guess also summary streaming can be refactored to stream all summaries
>> for a given file instead of reading one sumarry from all files.
>>
>> Bootstrapped/regtested x86_64-linux, plan to commit it this afternoon if
>> there are no complains.
>>
>> Honza
>>
>> 	* lto-common.c (lto_file_finalize): Add order attribute.
>> 	(lto_create_files_from_ids): Pass order.
>> 	(lto_file_read): UPdate call of lto_create_files_from_ids.
>> 	* lto-streamer-out.c (output_constructor): Push CTORS_OUT timevar.
>> 	(cmp_symbol_files): New.
>> 	(lto_output): Copy sections in file order.
>> 	* lto-streamer.h (lto_file_decl_data): Add field order.
> Hi,
> I have commited the patch but messed up testing so it broke builds with
> static libraries and checking enabled. This is fixes by this patch
> 
> 	* lto-streamer-out.c (cmp_symbol_files): Watch for overflow.
> Index: lto-streamer-out.c
> ===================================================================
> --- lto-streamer-out.c	(revision 277346)
> +++ lto-streamer-out.c	(working copy)
> @@ -2447,7 +2447,12 @@ cmp_symbol_files (const void *pn1, const
>  
>    /* Order within static library.  */
>    if (n1->lto_file_data && n1->lto_file_data->id != n2->lto_file_data->id)
> -    return n1->lto_file_data->id - n2->lto_file_data->id;
> +    {
> +      if (n1->lto_file_data->id > n2->lto_file_data->id)
> +	return 1;
> +      if (n1->lto_file_data->id < n2->lto_file_data->id)
> +	return -1;
> +    }

Hi.

It's unclear to me why you need the patch. Isn't that equivalent?
Why you need only 1 and -1 return values?

Martin

>  
>    /* And finaly order by the definition order.  */
>    return n1->order - n2->order;
>