From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28626 invoked by alias); 12 Jun 2013 04:44:23 -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 28612 invoked by uid 89); 12 Jun 2013 04:44:23 -0000 X-Spam-SWARE-Status: No, score=-7.5 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 12 Jun 2013 04:44:22 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r5C4iKcm019020 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 12 Jun 2013 00:44:21 -0400 Received: from stumpy.slc.redhat.com (ovpn-113-93.phx2.redhat.com [10.3.113.93]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r5C4iKvU005022; Wed, 12 Jun 2013 00:44:20 -0400 Message-ID: <51B7FCA4.4080201@redhat.com> Date: Wed, 12 Jun 2013 04:44:00 -0000 From: Jeff Law User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 MIME-Version: 1.0 To: Richard Biener CC: gcc-patches Subject: Re: Improve uncprop and coalescing References: <51B178BA.1000802@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2013-06/txt/msg00644.txt.bz2 On 06/07/13 03:14, Richard Biener wrote: > > >> +/* Given SSA_NAMEs NAME1 and NAME2, return true if they are candidates for >> + coalescing together, false otherwise. >> + >> + This must stay consistent with the code in tree-ssa-live.c which >> + sets up base values in the var map. */ >> + >> +bool >> +gimple_can_coalesce_p (tree name1, tree name2) >> +{ >> + /* First check the SSA_NAME's associated DECL. We only want to >> + coalesce if they have the same DECL or both have no associated DECL. >> */ >> + if (SSA_NAME_VAR (name1) != SSA_NAME_VAR (name2)) >> + return false; >> + >> + /* Now check the types. If the types are the same, then we should >> + try to coalesce V1 and V2. */ >> + tree t1 = TREE_TYPE (name1); >> + tree t2 = TREE_TYPE (name2); >> + if (t1 == t2) >> + return true; >> + >> + /* If the types are not the same, check for a canonical type match. This >> + (for example) allows coalescing when the types are fundamentally the >> + same, but just have different names. */ >> + if (TYPE_CANONICAL (t1) && TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2)) > > Please use types_compatible_p (t1, t2) here, that's the correct API to use > here. No it isn't. types_compatible_p is far too permissive in this context without more surgery to tree-ssa-live.c. So let's take two objects, P1 and P2 which are pointers to types T1 and T2 where T1 != T2. Assume P1 and P2 are somehow connected by a copy or PHI node and that they are anonymously named objects. types_compatible_p will return true for (T1, T2) unless T1 or T2 is a pointer to a function. So if we used types_compatible_p we will try to coalesce P1 and P2 (which is seemingly a good thing). Now in tree-ssa-live.c::var_map_base_init we have: /* Build the base variable list, and point partitions at their bases. */ for (x = 0; x < num_part; x++) { ... if (SSA_NAME_VAR (var)) m->base.from = SSA_NAME_VAR (var); else /* This restricts what anonymous SSA names we can coalesce as it restricts the sets we compute conflicts for. Using TREE_TYPE to generate sets is the easies as type equivalency also holds for SSA names with the same underlying decl. */ m->base.from = TREE_TYPE (var); ... } The way we set up base.from in var_map_base_init imposes requirements on what objects can be added to the coalescing lists. We can only allow coalescing of objects with the same underlying SSA_NAME_VAR or anonymous objects with the same underlying TREE_TYPE (as the code is written today). That's a side effect of restricting the sets we compute conflicts for. If we don't compute the conflicts, then we'll assume P1 and P2 don't conflict and happily coalesce them, regardless of whether or not they actually do conflict. To use types_compatible_p to drive decisions in tree-ssa-coalesce.c, ISTM we'd have to change this code from tree-ssa-live.c to put all anonymous objects with a compatible type in the same hash entry. I don't see a good way to do that without iterating over the hash table entries looking for a compatible match or completely reimplementing the hash. By first checking if an anonymous variable's type has an underlying canonical type and using that instead to key decisions in var_map_base_init, we can allow more objects onto the coalescing lists in tree-ssa-coalesce.c. In particular we can allow two anonymous objects where both have an underlying TYPE_CANONICAL that is the same. > > > if (TYPE_CANONICAL (t1) && TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2) > && types_compatible_p (t1, t2)) > > because looking at useless_type_conversion_p it looks like pointer types > with different address-spaces may have the same canonical type. A comment > on why we check both, refering to var_map_base_init should also be added. > > Ok with that change. Seems to make sense. Though we still have a point of contention around whether or not we can use types_compatible_p to drive decisions in tree-ssa-coalesce.c. I'm pretty sure we can't without some significant surgery in tree-ssa-live.c. Jeff