From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 37428 invoked by alias); 3 Apr 2015 13:17:57 -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 37412 invoked by uid 89); 3 Apr 2015 13:17:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 03 Apr 2015 13:17:55 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t33DHr0N007688 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Fri, 3 Apr 2015 09:17:53 -0400 Received: from freie.home (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t33DHmdn018681 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 3 Apr 2015 09:17:52 -0400 Received: from livre.home (livre.home [172.31.160.2]) by freie.home (8.14.8/8.14.8) with ESMTP id t33DHfYo017045; Fri, 3 Apr 2015 10:17:41 -0300 From: Alexandre Oliva To: Jeff Law Cc: gcc-patches@gcc.gnu.org Subject: Re: [PR64164] drop copyrename, integrate into expand References: <551A2C7C.8060005@redhat.com> Date: Fri, 03 Apr 2015 13:17:00 -0000 In-Reply-To: <551A2C7C.8060005@redhat.com> (Jeff Law's message of "Mon, 30 Mar 2015 23:11:24 -0600") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2015-04/txt/msg00117.txt.bz2 On Mar 31, 2015, Jeff Law wrote: >> - if (var1 != var2) >> + if (var1 != var2 && !optimize) >> return false; > So when when the base variables are different and we are optimizing, > this allows coalescing, right? Yeah. > What I don't see is a corresponding change to var_map_base_init to > ensure we build a conflict graph which includes objects when > SSA_NAME_VARs are not the same. I see a vague reference in > var_map_base_init's header comment that refers us to > coalesce_ssa_name. > It appears that compute_optimized_partition_bases handles this by > creating a partitions of things that are related by copies/phis > regardless of their underlying named object, type, etc. Right? Correct. I guess it makes sense to move partition base computation to a single location. Since compute_optimized_partition_bases relies on data structures local to this source file, I'm moving the non-optimized version to tree-ssa-coalesce.c, and dropping support for basevar initialization from tree-ssa-live.c. > Hard to argue with removing a pass that gets called 5 times! :-) >> @@ -890,6 +900,36 @@ build_ssa_conflict_graph (tree_live_info_p liveinfo) >> live_track_process_def (live, result, graph); >> } >> >> + /* Pretend there are defs for params' default defs at the start >> + of the (post-)entry block. We run after abnormal coalescing, >> + so we can't assume the leader variable is the default >> + definition, but because of SSA_NAME_VAR adjustments in >> + attempt_coalesce, we can assume that if there is any >> + PARM_DECL in the partition, it will be the leader's >> + SSA_NAME_VAR. */ This comment is outdated. Since we no longer have abnormal coalescing before building the conflict graph, we can just test whether each SSA_NAME is a default def for a PARM_DECL and be done with it. > So the issue here is you want to iterate over the objects live at the > entry block, which would include any SSA_NAMEs which result from > PARM_DECLs. I don't guess there's an easier way to do that other than > iterating over everything live in that initial block? We could iterate over all SSA_NAMEs, but that would probably be more costly. There shouldn't be very many live variables at the function entry, so using the live bitmaps is likely to save us time, especially on functions with lots of SSA_NAMEs. > And the second second EXECUTE_IF_SET_IN_BITMAP iterates over > everything in the partitions associated with the SSA_NAMES that are > live at the the entry block, right? Yeah, we iterate over the bases in live_base_var, because the per-base bitmaps are only accurate when the corresponding live_base_var bit is iset. >> @@ -1126,11 +1166,12 @@ create_outofssa_var_map (coalesce_list_p cl, bitmap used_in_copy) >> >> static inline bool >> attempt_coalesce (var_map map, ssa_conflicts_p graph, int x, int y, >> - FILE *debug) >> + bitmap param_defaults, FILE *debug) > [ ... ] > So the bulk of the changes into this routine are really about picking > a good leader, which presumably is how we're able to get the desired > effects on debuginfo that we used to get from tree-ssa-copyrename.c? This has nothing to do with debuginfo, I'm afraid. We just had to keep track of parm and result decls to avoid coalescing them together, and parm decl default defs to promote them to leaders, because expand copies incoming REGs to pseudos in PARM_DECL's DECL_RTL. We should fill that in with the RTL created for the default def for the PARM_DECL. At the end, I believe we also copy the RESULT_DECL DECL_RTL to the actual return register or rtl. I didn't want to tackle the reworking of these expanders to avoid problems out of copying incoming parms to one pseudo and then reading it from another, as I observed before I made this change. I'm now tackling that, so that we can refrain from touching the base vars altogether, and not refrain from coalescing parms and results. > So some comments about the various cases here might help. I can sort > them out if I read the code, but one could argue that a block comment > on the rules for how to select the partition leader would be better. *nod*. I won't bother, though, if this code ends up gone in the next iteration of the patch ;-) > Is the special casing of PARM_DECLs + RESULT_DECLs really a failing of > not handling one or both properly when computing liveness information? No, it's about RTL assignment and copying to/from hard regs. We assign RTL to PARM_DECLs and RESULT_DECLs explicitly in the expander, but we can't assign different RTL to them if they are coalesced in a single partition. > I'm not aware of an inherent reason why a PARM_DECL couldn't coalesce > with a related RESULT_DECL if they are otherwise non-conflicting and > related by a copy/phi. Indeed, there isn't any inherent reason. It was just a restriction I carried over from copyrename, and that I postponed cleaning up. > Presumably ordering of unioning of the partitions doesn't matter here > as we're looking at coalesce possibilities rather than things we have > actually coalesced? Thus it's OK (?) to handle the names occurring in > abnormal PHIs after those names that are associated by a copy. Yeah, they'll end up with the same basevar one way or another. -- Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer