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

[Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>]

On 4 April 2016 at 13:56, Richard Biener <rguenther@suse.de> wrote:
> On Mon, 4 Apr 2016, Prathamesh Kulkarni wrote:
>
>> On 1 April 2016 at 23:02, Richard Biener <rguenther@suse.de> wrote:
>> > On April 1, 2016 3:48:35 PM GMT+02:00, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote:
>> >>Hi,
>> >>The attached patch introduces param max-lto-partition which creates an
>> >>upper
>> >>bound for partition size.
>> >>
>> >>My primary motivation for this patch is to fix building chromium for
>> >>arm
>> >>with -flto-partition=one.
>> >>Chromium fails to build with -flto-partition={none, one} with assembler
>> >>error:
>> >>"branch out of range error"
>> >>because in both these cases LTO creates a single text section of 18 mb
>> >>which exceeds thumb's limit of 16 mb and arm backend emits a short
>> >>call if caller and callee are in same section.
>> >>This is binutils PR18625:
>> >>https://sourceware.org/bugzilla/show_bug.cgi?id=18625
>> >>With patch, chromium builds for -flto-partition=one (by creating more
>> >>than one but minimal number of partitions to honor 16 mb limit).
>> >>I haven't tested with -flto-partition=none but I suppose the build
>> >>will still fail for none, because it won't involve partitioning?  I am
>> >>not sure how to fix for none case.
>> >>
>> >>As suggested by Jim in binutils PR18625, the proper fix would be to
>> >>implement branch relaxation in arm's port of gas, however I suppose
>> >>only LTO will realistically create such large sections,
>> >>and implementing branch relaxation appears to be quite complicated and
>> >>probably too much of
>> >>an effort for this single use case, so this patch serves as a
>> >>work-around to the issue.
>> >>I am looking into fine-tuning the param value for ARM backend to
>> >>roughly match limit
>> >>of 16 mb.
>> >>
>> >>AFAIU, this would change semantics of --param n_lto_partitions (or
>> >>-flto-partition=one) from
>> >>"exactly n_lto_partitions" to "at-least n_lto_partitions". If that's
>> >>not desirable maybe we could add
>> >>another param/option ?
>> >>Cross-tested on arm*-*-*.
>> >>Would this patch be OK for stage-1 (after getting param value right
>> >>for ARM target) ?
>> >
>> > What do you want to achieve?  Changing =one semantics doesn't look right to me.
>> > Adding a param for maximum size sounds good in general, but only to increase the maximum number of partitions for =balanced (the default).
>>
>> Well, chromium fails to build on ARM with -flto-partition={none, one}
>> because the size of text section created with LTO,
>> exceeds the limit of 16 mb for thumb2 which results in assembler
>> errors: "branch out of range".
>> I was trying to fix that by creating minimal number of partitions such
>> that size of each partition is not greater than section size limit.
>
> Ok, but you simply shouldn't use -flto-partition={none,one} if it doesn't
> work.  Note that "partition size" and text section size do not have
> a 1:1 correspondence so a safe limit is hard to achieve anyway.
>
>> I suppose in theory the problem could also present with balanced
>> partitioning if total_size / n_lto_partitions exceeds section size
>> limit,
>> although not sure if this will be a practical case.
>
> I guess an artificial testcase can easily hit this.  Or you can
> hit this by adjusting --param lto-partitions to 1.  I think
> adding a --param lto-max-partition is missing given that we
> already have a --param lto-min-partition and the partitioning
> algorithm tries to create lto-partitions partitions (but not smaller
> than lto-min-partition) but it never creates more than lto-partitions
> partitions as there is no upper bound on individual partition size.
>
> This is also why lto-partitions has such a high default (to exploit
> parallelism - but if there is only a very small number of CPU cores
> available it doesn't make sense to split up so much for small programs).
>
> That said, lto-partitions is a hint currently but also an upper bound
> because we lack lto-max-partition.  Let's fix that instead.
Um not sure if I understood correctly.
Do we want to constrain individual partition size by adding parameter
lto-max-partition
for balanced partitioning but not for -flto-partition=one
case (since latter would also change semantics of =one) ?

Thanks,
Prathamesh
>
> Richard.
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)