Re: [RFC] introduce --param max-lto-partition for having an upper bound on partition size

[Jan Hubicka <hubicka at ucw dot cz>]

> diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
> index 9eb63c2..bc0c612 100644
> --- a/gcc/lto/lto-partition.c
> +++ b/gcc/lto/lto-partition.c
> @@ -511,9 +511,20 @@ lto_balanced_map (int n_lto_partitions)
>    varpool_order.qsort (varpool_node_cmp);
>  
>    /* Compute partition size and create the first partition.  */
> +  if (PARAM_VALUE (MIN_PARTITION_SIZE) > PARAM_VALUE (MAX_PARTITION_SIZE))
> +    fatal_error (input_location, "min partition size cannot be greater than max partition size");
> +
>    partition_size = total_size / n_lto_partitions;
>    if (partition_size < PARAM_VALUE (MIN_PARTITION_SIZE))
>      partition_size = PARAM_VALUE (MIN_PARTITION_SIZE);
> +  else if (partition_size > PARAM_VALUE (MAX_PARTITION_SIZE))
> +    {
> +      n_lto_partitions = total_size / PARAM_VALUE (MAX_PARTITION_SIZE);
> +      if (total_size % PARAM_VALUE (MAX_PARTITION_SIZE))
> +	n_lto_partitions++;
> +      partition_size = total_size / n_lto_partitions;
> +    }

lto_balanced_map actually works in a way that looks for cheapest cutpoint in range
3/4*parittion_size to 2*partition_size and picks the cheapest range.
Setting partition_size to this value will thus not cause partitioner to produce smaller
partitions only.  I suppose modify the conditional:

      /* Partition is too large, unwind into step when best cost was reached and
         start new partition.  */                                               
      if (partition->insns > 2 * partition_size)                                

and/or in the code above set the partition_size to half of total_size/max_size.

I know this is somewhat sloppy.  This was really just first cut implementation
many years ago. I expected to reimplement it marter soon, but then there was
never really a need for it (I am trying to avoid late IPA optimizations so the
partitioning decisions should mostly affect compile time performance only).
If ARM is more sensitive for partitining, perhaps it would make sense to try to
look for something smarter.

> +
>    npartitions = 1;
>    partition = new_partition ("");
>    if (symtab->dump_file)
> diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c
> index 9dd513f..294b8a4 100644
> --- a/gcc/lto/lto.c
> +++ b/gcc/lto/lto.c
> @@ -3112,6 +3112,12 @@ do_whole_program_analysis (void)
>    timevar_pop (TV_WHOPR_WPA);
>  
>    timevar_push (TV_WHOPR_PARTITIONING);
> +
> +  if (flag_lto_partition != LTO_PARTITION_BALANCED
> +      && PARAM_VALUE (MAX_PARTITION_SIZE) != INT_MAX)
> +    fatal_error (input_location, "--param max-lto-partition should only"
> +		 " be used with balanced partitioning\n");
> +

I think we should wire in resonable MAX_PARTITION_SIZE default.  THe value you
found experimentally may be a good start. For that reason we can't really
refuse a value when !LTO_PARTITION_BALANCED.  Just document it as parameter for
balanced partitioning only and add a parameter to lto_balanced_map specifying whether
this param should be honored (because the same path is used for partitioning to one partition)

Otherwise the patch looks good to me modulo missing documentation.

Honza