From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 36318 invoked by alias); 6 Apr 2016 07:47:28 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 36288 invoked by uid 89); 6 Apr 2016 07:47:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=million, expenses, ten, late X-HELO: mail-io0-f170.google.com Received: from mail-io0-f170.google.com (HELO mail-io0-f170.google.com) (209.85.223.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 06 Apr 2016 07:47:12 +0000 Received: by mail-io0-f170.google.com with SMTP id 2so46942952ioy.1 for ; Wed, 06 Apr 2016 00:47:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=9pD9I86CIY3VhdwHd8wNzpaXC72PpKorr65ngkgXqQk=; b=O8morFFdB+WgGprF04UdKEhaG0JJFKz0MQJQxhdXVGr518Ia0LuBBD8IfOBhhucqYA nx+xUF0iibcDXEPavGmp2bcV15IMLKnDD2REzT7C77KhoZVyrBb5khn6l6AaQcXrXbQU K5OxBV8Xt9fI0AWdr53WC/2LAJ9kdPRNylsu74e5oJ2lRGGGo6abKfsAjoKOHWXYS9gR acfkJFvZ27FtSh9bi7SbosKNuF0INg5vbAebUn7FVqqPnlKX7GcLADvkze4AimjCj+1y GGjUNiempFdfHfobhldQT2gCifVWDILlCfQRgVe0QwlYSEoIHMxV4wZXsYyOQ+BF+1e+ HaQw== X-Gm-Message-State: AD7BkJISTO9CkQlvRFC5dL12J+cqA54r6iJa2avUjkfpvRCQc+2KVRnGnNaKz+zG+LnYKankw2uoMM4xGT9YHYAD MIME-Version: 1.0 X-Received: by 10.107.130.148 with SMTP id m20mr3309502ioi.137.1459928829933; Wed, 06 Apr 2016 00:47:09 -0700 (PDT) Received: by 10.36.196.5 with HTTP; Wed, 6 Apr 2016 00:47:09 -0700 (PDT) In-Reply-To: References: <20160404120030.GD14122@kam.mff.cuni.cz> <20160404141436.GB95176@kam.mff.cuni.cz> Date: Wed, 06 Apr 2016 07:47:00 -0000 Message-ID: Subject: Re: [RFC] introduce --param max-lto-partition for having an upper bound on partition size From: Prathamesh Kulkarni To: Richard Biener Cc: Jan Hubicka , gcc Patches , Ramana Radhakrishnan Content-Type: multipart/mixed; boundary=001a113ecd3e4ee224052fcc2beb X-IsSubscribed: yes X-SW-Source: 2016-04/txt/msg00270.txt.bz2 --001a113ecd3e4ee224052fcc2beb Content-Type: text/plain; charset=UTF-8 Content-length: 6180 On 5 April 2016 at 18:28, Richard Biener wrote: > On Tue, 5 Apr 2016, Prathamesh Kulkarni wrote: > >> On 5 April 2016 at 16:58, Richard Biener wrote: >> > On Tue, 5 Apr 2016, Prathamesh Kulkarni wrote: >> > >> >> On 4 April 2016 at 19:44, Jan Hubicka 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). >> Done in this version. > > I'd do that separately. > > Please no default parameter for lto_balanced_map (), instead change > all callers. > >> Is it OK after bootstrap+test ? > > Note that this is for stage1 only. I'll leave approval to Honza > (also verification of the default max param - not sure if for example > chromium or firefox should/will be split to more than 32 partitions > with the patch) Removed default parameter in this version. I verified with the patch for chromium LTO build: n_lto_partitions == 32, ltrans_partitions.length() == 31 Thanks, Prathamesh > > Richard. > >> Thanks, >> Prathamesh >> > >> > 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 >> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg) >> > > -- > Richard Biener > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg) --001a113ecd3e4ee224052fcc2beb Content-Type: text/plain; charset=US-ASCII; name="patch-4.diff" Content-Disposition: attachment; filename="patch-4.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_imojtkf31 Content-length: 5011 ZGlmZiAtLWdpdCBhL2djYy9kb2MvaW52b2tlLnRleGkgYi9nY2MvZG9jL2lu dm9rZS50ZXhpCmluZGV4IDllNTRiYjcuLmYwZGU3ZWMgMTAwNjQ0Ci0tLSBh L2djYy9kb2MvaW52b2tlLnRleGkKKysrIGIvZ2NjL2RvYy9pbnZva2UudGV4 aQpAQCAtOTQ3Nyw2ICs5NDc3LDExIEBAIFNpemUgb2YgbWluaW1hbCBwYXJ0 aXRpb24gZm9yIFdIT1BSIChpbiBlc3RpbWF0ZWQgaW5zdHJ1Y3Rpb25zKS4K IFRoaXMgcHJldmVudHMgZXhwZW5zZXMgb2Ygc3BsaXR0aW5nIHZlcnkgc21h bGwgcHJvZ3JhbXMgaW50byB0b28gbWFueQogcGFydGl0aW9ucy4KIAorQGl0 ZW0gbHRvLW1heC1wYXJ0aXRpb24KK1NpemUgb2YgbWF4IHBhcnRpdGlvbiBm b3IgV0hPUFIgKGluIGVzdGltYXRlZCBpbnN0cnVjdGlvbnMpLgordG8gcHJv dmlkZSBhbiB1cHBlciBib3VuZCBmb3IgaW5kaXZpZHVhbCBzaXplIG9mIHBh cnRpdGlvbi4KK01lYW50IHRvIGJlIHVzZWQgb25seSB3aXRoIGJhbGFuY2Vk IHBhcnRpdGlvbmluZy4KKwogQGl0ZW0gY3h4LW1heC1uYW1lc3BhY2VzLWZv ci1kaWFnbm9zdGljLWhlbHAKIFRoZSBtYXhpbXVtIG51bWJlciBvZiBuYW1l c3BhY2VzIHRvIGNvbnN1bHQgZm9yIHN1Z2dlc3Rpb25zIHdoZW4gQysrCiBu YW1lIGxvb2t1cCBmYWlscyBmb3IgYW4gaWRlbnRpZmllci4gIFRoZSBkZWZh dWx0IGlzIDEwMDAuCmRpZmYgLS1naXQgYS9nY2MvbHRvL2x0by1wYXJ0aXRp b24uYyBiL2djYy9sdG8vbHRvLXBhcnRpdGlvbi5jCmluZGV4IDllYjYzYzIu LmQzODVkZDkgMTAwNjQ0Ci0tLSBhL2djYy9sdG8vbHRvLXBhcnRpdGlvbi5j CisrKyBiL2djYy9sdG8vbHRvLXBhcnRpdGlvbi5jCkBAIC00NDcsNyArNDQ3 LDcgQEAgYWRkX3NvcnRlZF9ub2RlcyAodmVjPHN5bXRhYl9ub2RlICo+ICZu ZXh0X25vZGVzLCBsdHJhbnNfcGFydGl0aW9uIHBhcnRpdGlvbikKICAgIGFu ZCBpbi1wYXJ0aXRpb24gY2FsbHMgd2FzIHJlYWNoZWQuICAqLwogCiB2b2lk Ci1sdG9fYmFsYW5jZWRfbWFwIChpbnQgbl9sdG9fcGFydGl0aW9ucykKK2x0 b19iYWxhbmNlZF9tYXAgKGludCBuX2x0b19wYXJ0aXRpb25zLCBib29sIGhv bm9yX21heF9wYXJ0aXRpb24pCiB7CiAgIGludCBuX25vZGVzID0gMDsKICAg aW50IG5fdmFycG9vbF9ub2RlcyA9IDAsIHZhcnBvb2xfcG9zID0gMCwgYmVz dF92YXJwb29sX3BvcyA9IDA7CkBAIC01MTEsNiArNTExLDkgQEAgbHRvX2Jh bGFuY2VkX21hcCAoaW50IG5fbHRvX3BhcnRpdGlvbnMpCiAgIHZhcnBvb2xf b3JkZXIucXNvcnQgKHZhcnBvb2xfbm9kZV9jbXApOwogCiAgIC8qIENvbXB1 dGUgcGFydGl0aW9uIHNpemUgYW5kIGNyZWF0ZSB0aGUgZmlyc3QgcGFydGl0 aW9uLiAgKi8KKyAgaWYgKFBBUkFNX1ZBTFVFIChNSU5fUEFSVElUSU9OX1NJ WkUpID4gUEFSQU1fVkFMVUUgKE1BWF9QQVJUSVRJT05fU0laRSkpCisgICAg ZmF0YWxfZXJyb3IgKGlucHV0X2xvY2F0aW9uLCAibWluIHBhcnRpdGlvbiBz aXplIGNhbm5vdCBiZSBncmVhdGVyIHRoYW4gbWF4IHBhcnRpdGlvbiBzaXpl Iik7CisKICAgcGFydGl0aW9uX3NpemUgPSB0b3RhbF9zaXplIC8gbl9sdG9f cGFydGl0aW9uczsKICAgaWYgKHBhcnRpdGlvbl9zaXplIDwgUEFSQU1fVkFM VUUgKE1JTl9QQVJUSVRJT05fU0laRSkpCiAgICAgcGFydGl0aW9uX3NpemUg PSBQQVJBTV9WQUxVRSAoTUlOX1BBUlRJVElPTl9TSVpFKTsKQEAgLTcxOSw3 ICs3MjMsOSBAQCBsdG9fYmFsYW5jZWRfbWFwIChpbnQgbl9sdG9fcGFydGl0 aW9ucykKIAkJIGJlc3RfY29zdCwgYmVzdF9pbnRlcm5hbCwgYmVzdF9pKTsK ICAgICAgIC8qIFBhcnRpdGlvbiBpcyB0b28gbGFyZ2UsIHVud2luZCBpbnRv IHN0ZXAgd2hlbiBiZXN0IGNvc3Qgd2FzIHJlYWNoZWQgYW5kCiAJIHN0YXJ0 IG5ldyBwYXJ0aXRpb24uICAqLwotICAgICAgaWYgKHBhcnRpdGlvbi0+aW5z bnMgPiAyICogcGFydGl0aW9uX3NpemUpCisgICAgICBpZiAocGFydGl0aW9u LT5pbnNucyA+IDIgKiBwYXJ0aXRpb25fc2l6ZQorCSAgfHwgKGhvbm9yX21h eF9wYXJ0aXRpb24KKwkgICAgICAmJiBwYXJ0aXRpb24tPmluc25zID4gUEFS QU1fVkFMVUUgKE1BWF9QQVJUSVRJT05fU0laRSkpKQogCXsKIAkgIGlmIChi ZXN0X2kgIT0gaSkKIAkgICAgewpkaWZmIC0tZ2l0IGEvZ2NjL2x0by9sdG8t cGFydGl0aW9uLmggYi9nY2MvbHRvL2x0by1wYXJ0aXRpb24uaAppbmRleCAz MWUzNzY0Li5hNTU1OTRjIDEwMDY0NAotLS0gYS9nY2MvbHRvL2x0by1wYXJ0 aXRpb24uaAorKysgYi9nY2MvbHRvL2x0by1wYXJ0aXRpb24uaApAQCAtMzUs NyArMzUsNyBAQCBleHRlcm4gdmVjPGx0cmFuc19wYXJ0aXRpb24+IGx0cmFu c19wYXJ0aXRpb25zOwogCiB2b2lkIGx0b18xX3RvXzFfbWFwICh2b2lkKTsK IHZvaWQgbHRvX21heF9tYXAgKHZvaWQpOwotdm9pZCBsdG9fYmFsYW5jZWRf bWFwIChpbnQpOwordm9pZCBsdG9fYmFsYW5jZWRfbWFwIChpbnQsIGJvb2wg aG9ub3JfbWF4X3BhcnRpdGlvbik7IAogdm9pZCBsdG9fcHJvbW90ZV9jcm9z c19maWxlX3N0YXRpY3MgKHZvaWQpOwogdm9pZCBmcmVlX2x0cmFuc19wYXJ0 aXRpb25zICh2b2lkKTsKIHZvaWQgbHRvX3Byb21vdGVfc3RhdGljc19ub253 cGEgKHZvaWQpOwpkaWZmIC0tZ2l0IGEvZ2NjL2x0by9sdG8uYyBiL2djYy9s dG8vbHRvLmMKaW5kZXggOWRkNTEzZi4uZWYyY2UxNSAxMDA2NDQKLS0tIGEv Z2NjL2x0by9sdG8uYworKysgYi9nY2MvbHRvL2x0by5jCkBAIC0zMTE3LDkg KzMxMTgsOSBAQCBkb193aG9sZV9wcm9ncmFtX2FuYWx5c2lzICh2b2lkKQog ICBlbHNlIGlmIChmbGFnX2x0b19wYXJ0aXRpb24gPT0gTFRPX1BBUlRJVElP Tl9NQVgpCiAgICAgbHRvX21heF9tYXAgKCk7CiAgIGVsc2UgaWYgKGZsYWdf bHRvX3BhcnRpdGlvbiA9PSBMVE9fUEFSVElUSU9OX09ORSkKLSAgICBsdG9f YmFsYW5jZWRfbWFwICgxKTsKKyAgICBsdG9fYmFsYW5jZWRfbWFwICgxLCBm YWxzZSk7CiAgIGVsc2UgaWYgKGZsYWdfbHRvX3BhcnRpdGlvbiA9PSBMVE9f UEFSVElUSU9OX0JBTEFOQ0VEKQotICAgIGx0b19iYWxhbmNlZF9tYXAgKFBB UkFNX1ZBTFVFIChQQVJBTV9MVE9fUEFSVElUSU9OUykpOworICAgIGx0b19i YWxhbmNlZF9tYXAgKFBBUkFNX1ZBTFVFIChQQVJBTV9MVE9fUEFSVElUSU9O UyksIHRydWUpOwogICBlbHNlCiAgICAgZ2NjX3VucmVhY2hhYmxlICgpOwog CmRpZmYgLS1naXQgYS9nY2MvcGFyYW1zLmRlZiBiL2djYy9wYXJhbXMuZGVm CmluZGV4IDkzNjJjMTUuLmI1ZGEzODQgMTAwNjQ0Ci0tLSBhL2djYy9wYXJh bXMuZGVmCisrKyBiL2djYy9wYXJhbXMuZGVmCkBAIC0xMDI5LDYgKzEwMjks MTEgQEAgREVGUEFSQU0gKE1JTl9QQVJUSVRJT05fU0laRSwKIAkgICJNaW5p bWFsIHNpemUgb2YgYSBwYXJ0aXRpb24gZm9yIExUTyAoaW4gZXN0aW1hdGVk IGluc3RydWN0aW9ucykuIiwKIAkgIDEwMDAsIDAsIDApCiAKK0RFRlBBUkFN IChNQVhfUEFSVElUSU9OX1NJWkUsCisJICAibHRvLW1heC1wYXJ0aXRpb24i LAorCSAgIk1heGltYWwgc2l6ZSBvZiBhIHBhcnRpdGlvbiBmb3IgTFRPIChp biBlc3RpbWF0ZWQgaW5zdHJ1Y3Rpb25zKS4iLAorCSAgMTAwMDAwMCwgMCwg SU5UX01BWCkKKwogLyogRGlhZ25vc3RpYyBwYXJhbWV0ZXJzLiAgKi8KIAog REVGUEFSQU0gKENYWF9NQVhfTkFNRVNQQUNFU19GT1JfRElBR05PU1RJQ19I RUxQLAo= --001a113ecd3e4ee224052fcc2beb Content-Type: application/octet-stream; name=ChangeLog Content-Disposition: attachment; filename=ChangeLog Content-Transfer-Encoding: base64 X-Attachment-Id: f_imojvrt81 Content-length: 647 MjAxNi0wNC0wNiAgUHJhdGhhbWVzaCBLdWxrYXJuaSAgPHByYXRoYW1lc2gu a3Vsa2FybmlAbGluYXJvLm9yZz4KCgkqIHBhcmFtcy5kZWYgKE1BWF9QQVJU SVRJT05fU0laRSk6IE5ldyBwYXJhbS4KCSogbHRvL2x0by1wYXJ0aXRpb24u aCAobHRvX2JhbGFuY2VkX21hcCk6IE5ldyBwYXJhbWV0ZXIKCWhvbm9yX21h eF9wYXJ0aXRpb24uIAoJKiBsdG8vbHRvLXBhcnRpdGlvbi5jIChsdG9fYmFs YW5jZWRfbWFwKTogTmV3IHBhcmFtZXRlcgoJaG9ub3JfbWF4X3BhcnRpdGlv bi4KCUNoZWNrIGlmIHBhcnRpdGlvbiBzaXplIGlzIGdyZWF0ZXIgdGhhbiBt YXgtbHRvLXBhcnRpdGlvbi4KCSogbHRvL2x0by5jIChkb193aG9sZV9wcm9n cmFtX2FuYWx5c2lzKTogQWRqdXN0IGNhbGxzIHRvCglsdG9fYmFsYW5jZWRf bWFwKCkgdG8gcGFzcyAybmQgYXJndW1lbnQuCgkqIGludm9rZS50ZXhpOiBE b2N1bWVudCBsdG8tbWF4LXBhcnRpdGlvbi4K --001a113ecd3e4ee224052fcc2beb--