From patchwork Fri Feb 20 13:04:20 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Martin_Li=C5=A1ka?= X-Patchwork-Id: 441964 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 1A3D6140129 for ; Sat, 21 Feb 2015 00:14:18 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=Qta3rTjdYEsGFtPRv ZvrdMQe0xC+k79k2WHLtyWO7zI8KFmSKatUXvU92KKEgmAHNs4tmOMFjfrON/mbF qrYOUhRSLAsT2VyvEEVT1111+AnaN8QrN+qZZpSqJgGh+/2ACbvxYszVwUFmrFaM OYpq5ZB7PlNURchRKQ54smbKOk= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=50xNt/5evZId8IPcpPupM27 XCo8=; b=ZIN/0SwSyzPJeUxAMc02G/HQQs6QRDc1J3qUhXAkyJByCXq6sKJwBth oBK/Dsl+HsT8jRbmLRq8Fwww8kZvcpl85jSw0lw4/v3qucoSM8IvTRbeT8IDiG9N FxlFZa2gBjsGQ5xiA1Sd7nUJAqzhB0E2aKFiDrC+JAFjQlsuMYeE= Received: (qmail 2423 invoked by alias); 20 Feb 2015 13:04:54 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 27851 invoked by uid 89); 20 Feb 2015 13:04:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 X-HELO: mx2.suse.de Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Fri, 20 Feb 2015 13:04:24 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id E189EACAB; Fri, 20 Feb 2015 13:04:20 +0000 (UTC) Message-ID: <54E730D4.1010403@suse.cz> Date: Fri, 20 Feb 2015 14:04:20 +0100 From: =?windows-1252?Q?Martin_Li=9Aka?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: gcc-patches@gcc.gnu.org CC: "hubicka >> Jan Hubicka" Subject: Re: [PATCH] Fix for PR ipa/64693 References: <54DC77A5.4020409@suse.cz> <20150212165758.GA3301@kam.mff.cuni.cz> In-Reply-To: <20150212165758.GA3301@kam.mff.cuni.cz> X-IsSubscribed: yes On 02/12/2015 05:57 PM, Jan Hubicka wrote: >> From: mliska V >> Date: Fri, 23 Jan 2015 14:58:36 +0100 >> Subject: [PATCH] Fix PR ipa/64693. >> >> gcc/ChangeLog: >> >> 2015-02-12 Martin Liska >> >> PR ipa/64693 >> * ipa-icf.c (sem_item_optimizer::execute): Call identify_address_sensitive_classes. >> (sem_item_optimizer::identify_address_sensitive_classes): New function. >> (sem_item_optimizer::merge_classes): Handle cases where we merge a class >> with a sensitive reference. >> (congruence_class::dump): Add support for a new sensitive_reference flag. >> * ipa-icf.h: Add new function. >> >> + >> +/* Identify congruence classes which have an address taken. These >> + classes can be potentially been merged just as thunks. */ > > Hmm, I would expect someting like this > > 1) break out code in sem_function::merge which decides if function A and B > can be identified or if thunk is needed > 2) when comparing references to verify congruence, actually check if merging > would result in thunk and subdivide. > > In the following: > > void f1() > { > } > void f2() > { > } > void *a=&f1; > void *b=&f1; > void *c=&f2; > void *d=&f2; > > The code bellow seems to disable all merging, while we ought to be able to mere A,B > and C,D into two congruences. > Similarly we ought to be able to merge in a case F2 is a virtual function/ctor/dtor > where address can not be compared for equality. >> + >> +void >> +sem_item_optimizer::identify_address_sensitive_classes (void) >> +{ >> + for (hash_table::iterator it = m_classes.begin (); >> + it != m_classes.end (); ++it) >> + for (unsigned i = 0; i < (*it)->classes.length (); i++) >> + { >> + congruence_class *cls = (*it)->classes[i]; >> + >> + if (cls->members.length () == 1) >> + continue; >> + >> + auto_vec references; >> + sem_item *source_node = cls->members[0]; >> + ipa_ref *r; >> + >> + for (unsigned j = 0; j < source_node->node->num_references (); j++) >> + { >> + symtab_node *ref = source_node->node->iterate_reference (j, r)->referred; >> + sem_item **symbol = m_symtab_node_map.get (ref); >> + if (symbol) >> + references.safe_push (*symbol); >> + } >> + >> + for (unsigned j = 1; j < cls->members.length (); j++) >> + { >> + source_node = cls->members[j]; >> + >> + unsigned l = 0; >> + for (unsigned k = 0; k < source_node->node->num_references (); k++) >> + { >> + symtab_node *ref = source_node->node->iterate_reference (k, r)->referred; >> + sem_item **symbol = m_symtab_node_map.get (ref); >> + if (symbol) >> + { >> + if (l >= references.length () || references[l] != *symbol) >> + { >> + cls->sensitive_reference = true; >> + break; >> + } >> + >> + l++; >> + } >> + } >> + >> + if (cls->sensitive_reference) >> + break; >> + } >> + } >> +} >> + >> /* Debug function prints all informations about congruence classes. */ >> >> void >> @@ -2374,6 +2430,15 @@ sem_item_optimizer::merge_classes (unsigned int prev_class_count) >> continue; >> } >> >> + if (c->sensitive_reference) >> + { >> + if (dump_file) >> + fprintf (dump_file, "A function from the congruence class " >> + "uses a sensitive reference.\n"); >> + >> + continue; >> + } >> + >> if (dump_file && (dump_flags & TDF_DETAILS)) >> { >> source->dump_to_file (dump_file); >> @@ -2390,8 +2455,10 @@ sem_item_optimizer::merge_classes (unsigned int prev_class_count) >> void >> congruence_class::dump (FILE *file, unsigned int indent) const >> { >> - FPRINTF_SPACES (file, indent, "class with id: %u, hash: %u, items: %u\n", >> - id, members[0]->get_hash (), members.length ()); >> + FPRINTF_SPACES (file, indent, >> + "class with id: %u, hash: %u, items: %u %s\n", >> + id, members[0]->get_hash (), members.length (), >> + sensitive_reference ? "sensitive_reference" : ""); >> >> FPUTS_SPACES (file, indent + 2, ""); >> for (unsigned i = 0; i < members.length (); i++) >> diff --git a/gcc/ipa-icf.h b/gcc/ipa-icf.h >> index adbedd6..db0bc54 100644 >> --- a/gcc/ipa-icf.h >> +++ b/gcc/ipa-icf.h >> @@ -29,7 +29,8 @@ class congruence_class >> { >> public: >> /* Congruence class constructor for a new class with _ID. */ >> - congruence_class (unsigned int _id): in_worklist (false), id(_id) >> + congruence_class (unsigned int _id): in_worklist (false), id(_id), >> + sensitive_reference (false) >> { >> } >> >> @@ -54,6 +55,9 @@ public: >> >> /* Global unique class identifier. */ >> unsigned int id; >> + >> + /* A function from the class contains a reference to another class. */ >> + bool sensitive_reference; >> }; >> >> /* Semantic item type enum. */ >> @@ -476,6 +480,10 @@ private: >> /* Iterative congruence reduction function. */ >> void process_cong_reduction (void); >> >> + /* Identify congruence classes which have an address taken. These >> + classes can be potentially been merged just as thunks. */ >> + void identify_address_sensitive_classes (void); >> + >> /* After reduction is done, we can declare all items in a group >> to be equal. PREV_CLASS_COUNT is start number of classes >> before reduction. */ >> diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c >> index 0c5a5a6..f25a251 100644 >> --- a/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c >> +++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c >> @@ -38,7 +38,7 @@ int main() >> return 0; >> } >> >> -/* { dg-final { scan-ipa-dump "Semantic equality hit:bar->foo" "icf" } } */ >> /* { dg-final { scan-ipa-dump "Semantic equality hit:remove->destroy" "icf" } } */ >> /* { dg-final { scan-ipa-dump "Equal symbols: 2" "icf" } } */ >> +/* { dg-final { scan-ipa-dump "A function from the congruence class uses a sensitive reference." "icf" } } */ >> /* { dg-final { cleanup-ipa-dump "icf" } } */ >> diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-33.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-33.c >> index d7e814d..7b6a8ae 100644 >> --- a/gcc/testsuite/gcc.dg/ipa/ipa-icf-33.c >> +++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-33.c >> @@ -23,5 +23,4 @@ int main() >> } >> >> /* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */ >> -/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */ >> /* { dg-final { cleanup-ipa-dump "icf" } } */ >> diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c >> new file mode 100644 >> index 0000000..66bcac5 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c >> @@ -0,0 +1,44 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-O0 -fipa-icf -fdump-ipa-icf-details" } */ >> + >> +#include >> +#include >> + >> +int callback1(int a) >> +{ >> + return a * a; >> +} >> + >> +int callback2(int a) >> +{ >> + return a * a; >> +} >> + >> +static int test(int (*callback) (int)) >> +{ >> + if (callback == callback1) >> + return 1; >> + >> + return 0; >> +} >> + >> +int foo() >> +{ >> + return test(&callback1); >> +} >> + >> +int bar() >> +{ >> + return test(&callback2); >> +} >> + >> +int main() >> +{ >> + assert (foo() != bar()); >> + >> + return 0; >> +} >> + >> +/* { dg-final { scan-ipa-dump "Equal symbols: 2" "icf" } } */ >> +/* { dg-final { scan-ipa-dump "A function from the congruence class uses a sensitive reference." "icf" } } */ >> +/* { dg-final { cleanup-ipa-dump "icf" } } */ >> -- >> 2.1.2 >> > Hi. This is second part which introduces better variable handling. Since readonly variable flag identification can identify new candidates, ICF should filter out non-readonly variables in execute phase. Ready for trunk? Thanks, Martin From a18a4840d14b1c0d35a9e4387daae29f5e8c906c Mon Sep 17 00:00:00 2001 From: mliska Date: Fri, 20 Feb 2015 11:15:37 +0100 Subject: [PATCH 2/2] Fix missed optimization for vars not marked by const. gcc/testsuite/ChangeLog: 2015-02-20 Martin Liska * gcc.dg/ipa/ipa-icf-35.c: New test. gcc/ChangeLog: 2015-02-20 Martin Liska * ipa-icf.c (sem_variable::parse): Ignore readonly flag that should be evaluated in driver. (sem_item_optimizer::filter_removed_items): Filter out non-readonly variables. --- gcc/ipa-icf.c | 13 ++++++++----- gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c index 859b9d1..5973b2f 100644 --- a/gcc/ipa-icf.c +++ b/gcc/ipa-icf.c @@ -1228,10 +1228,6 @@ sem_variable::parse (varpool_node *node, bitmap_obstack *stack) { tree decl = node->decl; - bool readonly = TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl); - if (!readonly) - return NULL; - bool can_handle = DECL_VIRTUAL_P (decl) || flag_merge_constants >= 2 || (!TREE_ADDRESSABLE (decl) && !node->externally_visible); @@ -1697,7 +1693,14 @@ sem_item_optimizer::filter_removed_items (void) if (!flag_ipa_icf_variables) remove_item (item); else - filtered.safe_push (item); + { + /* Filter out non-readonly variables. */ + tree decl = item->decl; + if (TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl)) + filtered.safe_push (item); + else + remove_item (item); + } } } diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c new file mode 100644 index 0000000..95d247e --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c @@ -0,0 +1,34 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-ipa-icf" } */ + +#include +#include + +void f1() +{ +} + +void f2() +{ +} + +static void (*a)(void)=&f1; +static void (*b)(void)=&f1; +static void (*c)(void)=&f2; +static void (*d)(void)=&f2; + +int main() +{ + a(); + b(); + c(); + d(); + + return 0; +} + +/* { dg-final { scan-ipa-dump "Equal symbols: 3" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:f2->f1" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:d->c" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:b->a" "icf" } } */ +/* { dg-final { cleanup-ipa-dump "icf" } } */ -- 2.1.2