On Tue, 5 Apr 2016, Prathamesh Kulkarni wrote:
> On 4 April 2016 at 19:44, Jan Hubicka <hubi...@ucw.cz> wrote:
> >
> >> 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.
> Thanks for the review. I have updated the patch.
> Does this version look OK ?
> I had randomly chosen 10000, not sure if that's an appropriate value
> for default.
I think it's way too small. This is roughly the number of GIMPLE stmts
(thus roughly the number of instructions). So with say a 8 byte
instruction format it is on the order of 80kB. You'd want to have a
default of at least several ten times of large-unit-insns (also 10000).
I'd choose sth like 1000000 (one million). I find the lto-min-partition
number quite small as well (and up it by a factor of 10).
Richard.
> I have a silly question about partitioning: Does it hamper
> transformations on ipa optimizations if caller and
> callee get placed in separate partitions ? For instance if callee is
> supposed to be inlined
> into caller, would inlining still take place if callee and caller get
> placed in separate partitions ?
> I tried with a trivial example with -flto-partition=max
> which created 3 partitions for 3 functions (bar, foo and main), and it was
> able to inline bar into foo and foo into main. I am not sure how that
> happens.
> I thought ltrans can perform transformations on functions only within
> a single partition
> and not across partitions ?
>
> Thanks,
> Prathamesh
> >
> > Honza
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
