From patchwork Fri Mar 20 07:54:27 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Bosscher X-Patchwork-Id: 452504 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 1AD73140157 for ; Fri, 20 Mar 2015 18:55:30 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass reason="1024-bit key; unprotected key" header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=BHVs3XEH; dkim-adsp=none (unprotected policy); dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; q=dns; s=default; b=Hfx86WTiEwQtJNn nTN6JS3a1/mUofzLRJTwRKiVhuRP1sG3gc3iCjzjdl61LHaOJ4XvNqxvw2RPtXnu g2y7pzOX2dJ9NZUCKwH1SQgBVBpv4SXdaXPPvcbPy3zq0IIQxFsDmNf1O7ABZEvV BGuBfyvc5wzUzpQFNarPVTmvJ5Mo= 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 :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; s=default; bh=u1vFnqpF3/YDJNfftuMEL Kx/GSQ=; b=BHVs3XEHKxVDNHZy6bvU3IpipyoHzE2knk41JpYM/qL/EjrgSrvow m3ZjJDAA8kpENO4/R8Swckn22iXW7N9bNMVJ0KmBtmcT2JQd8vLkZY5D8AdA+qLL s5XaG4gCB+e0AetvowEKwWioGGTXdDHRBVUTp0t/z46l1hbAz6TDUA= Received: (qmail 15819 invoked by alias); 20 Mar 2015 07:55:14 -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 15724 invoked by uid 89); 20 Mar 2015 07:55:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.1 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, KAM_FROM_URIBL_PCCC, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-lb0-f171.google.com Received: from mail-lb0-f171.google.com (HELO mail-lb0-f171.google.com) (209.85.217.171) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 20 Mar 2015 07:55:12 +0000 Received: by lbblx11 with SMTP id lx11so47538234lbb.3 for ; Fri, 20 Mar 2015 00:55:08 -0700 (PDT) X-Received: by 10.112.143.195 with SMTP id sg3mr22081520lbb.95.1426838108381; Fri, 20 Mar 2015 00:55:08 -0700 (PDT) MIME-Version: 1.0 Received: by 10.25.83.130 with HTTP; Fri, 20 Mar 2015 00:54:27 -0700 (PDT) In-Reply-To: <000601d04a5c$a0c0f030$e242d090$@arm.com> References: <000501d049d3$079385a0$16ba90e0$@arm.com> <000601d04a5c$a0c0f030$e242d090$@arm.com> From: Steven Bosscher Date: Fri, 20 Mar 2015 08:54:27 +0100 Message-ID: Subject: Re: [PATCH, GCC, stage1] Fallback to copy-prop if constant-prop not possible To: "Thomas Preud'homme" Cc: GCC Patches X-IsSubscribed: yes On Tue, Feb 17, 2015 at 3:51 AM, Thomas Preud'homme wrote: >> > - else if (REG_P (src) >> > - && REGNO (src) >= FIRST_PSEUDO_REGISTER >> > - && REGNO (src) != regno) >> > - { >> > - if (try_replace_reg (reg_used, src, insn)) >> > + else if (src_reg && REG_P (src_reg) >> > + && REGNO (src_reg) >= FIRST_PSEUDO_REGISTER >> > + && REGNO (src_reg) != regno >> > + && try_replace_reg (reg_used, src_reg, insn)) >> >> Likewise for the REG_P and ">= FIRST_PSEUDO_REGISTER" tests here >> (with >> the equivalent and IMHO preferable HARD_REGISTER_P test in >> find_avail_set()). > > I'm not sure I follow you here. First, it seems to me that the equivalent > test is rather REG_P && !HARD_REGISTER_P since here it checks if it's > a pseudo register. > > Then, do you mean the test can be simply removed because of the > REG_P && !HARD_REGISTER_P in hash_scan_set () called indirectly by > compute_hash_table () when called in one_cprop_pass () before any > cprop_insn ()? Or do you mean I should move the check in > find_avail_set ()? What I meant, is that I believe the tests are already done in hash_scan_set and should be redundant in cprop_insn (i.e. the test can be replaced with gcc_[checking_]assert). I've attached a patch with some changes to it: introduce cprop_reg_p() to get rid of all the "REG_P && regno > FIRST_PSEUDO_REGISTER" tests. I still have the cprop_constant_p and cprop_reg_p tests in cprop_insn but this weekend I'll try with gcc_checking_asserts instead. Please have a look at the patch and let me know if you like it (given it's mostly yours I hope you do like it ;-) Bootstrapped & tested on powerpc64-unknown-linux-gnu. In building all of cc1, 35 extra copies are propagated with the patch. Ciao! Steven Index: cprop.c =================================================================== --- cprop.c (revision 221523) +++ cprop.c (working copy) @@ -285,6 +285,15 @@ cprop_constant_p (const_rtx x) return CONSTANT_P (x) && (GET_CODE (x) != CONST || shared_const_p (x)); } +/* Determine whether the rtx X should be treated as a register that can + be propagated. Any pseudo-register is fine. */ + +static bool +cprop_reg_p (const_rtx x) +{ + return REG_P (x) && !HARD_REGISTER_P (x); +} + /* Scan SET present in INSN and add an entry to the hash TABLE. IMPLICIT is true if it's an implicit set, false otherwise. */ @@ -295,8 +304,7 @@ hash_scan_set (rtx set, rtx_insn *insn, struct has rtx src = SET_SRC (set); rtx dest = SET_DEST (set); - if (REG_P (dest) - && ! HARD_REGISTER_P (dest) + if (cprop_reg_p (dest) && reg_available_p (dest, insn) && can_copy_p (GET_MODE (dest))) { @@ -321,9 +329,8 @@ hash_scan_set (rtx set, rtx_insn *insn, struct has src = XEXP (note, 0), set = gen_rtx_SET (VOIDmode, dest, src); /* Record sets for constant/copy propagation. */ - if ((REG_P (src) + if ((cprop_reg_p (src) && src != dest - && ! HARD_REGISTER_P (src) && reg_available_p (src, insn)) || cprop_constant_p (src)) insert_set_in_table (dest, src, insn, table, implicit); @@ -821,15 +828,15 @@ try_replace_reg (rtx from, rtx to, rtx_insn *insn) return success; } -/* Find a set of REGNOs that are available on entry to INSN's block. Return - NULL no such set is found. */ +/* Find a set of REGNOs that are available on entry to INSN's block. If found, + SET_RET[0] will be assigned a set with a register source and SET_RET[1] a + set with a constant source. If not found the corresponding entry is set to + NULL. */ -static struct cprop_expr * -find_avail_set (int regno, rtx_insn *insn) +static void +find_avail_set (int regno, rtx_insn *insn, struct cprop_expr *set_ret[2]) { - /* SET1 contains the last set found that can be returned to the caller for - use in a substitution. */ - struct cprop_expr *set1 = 0; + set_ret[0] = set_ret[1] = NULL; /* Loops are not possible here. To get a loop we would need two sets available at the start of the block containing INSN. i.e. we would @@ -869,8 +876,10 @@ try_replace_reg (rtx from, rtx to, rtx_insn *insn) If the source operand changed, we may still use it for the next iteration of this loop, but we may not use it for substitutions. */ - if (cprop_constant_p (src) || reg_not_set_p (src, insn)) - set1 = set; + if (cprop_constant_p (src)) + set_ret[1] = set; + else if (reg_not_set_p (src, insn)) + set_ret[0] = set; /* If the source of the set is anything except a register, then we have reached the end of the copy chain. */ @@ -881,10 +890,6 @@ try_replace_reg (rtx from, rtx to, rtx_insn *insn) and see if we have an available copy into SRC. */ regno = REGNO (src); } - - /* SET1 holds the last set that was available and anticipatable at - INSN. */ - return set1; } /* Subroutine of cprop_insn that tries to propagate constants into @@ -1050,7 +1055,8 @@ cprop_insn (rtx_insn *insn) int changed = 0, changed_this_round; rtx note; -retry: + do + { changed_this_round = 0; reg_use_count = 0; note_uses (&PATTERN (insn), find_used_regs, NULL); @@ -1064,8 +1070,8 @@ cprop_insn (rtx_insn *insn) { rtx reg_used = reg_use_table[i]; unsigned int regno = REGNO (reg_used); - rtx src; - struct cprop_expr *set; + rtx src_cst = NULL, src_reg = NULL; + struct cprop_expr *set[2]; /* If the register has already been set in this block, there's nothing we can do. */ @@ -1074,17 +1080,16 @@ cprop_insn (rtx_insn *insn) /* Find an assignment that sets reg_used and is available at the start of the block. */ - set = find_avail_set (regno, insn); - if (! set) - continue; + find_avail_set (regno, insn, set); + if (set[0]) + src_reg = set[0]->src; + if (set[1]) + src_cst = set[1]->src; - src = set->src; - /* Constant propagation. */ - if (cprop_constant_p (src)) + if (src_cst && cprop_constant_p (src_cst) + && constprop_register (reg_used, src_cst, insn)) { - if (constprop_register (reg_used, src, insn)) - { changed_this_round = changed = 1; global_const_prop_count++; if (dump_file != NULL) @@ -1093,19 +1098,17 @@ cprop_insn (rtx_insn *insn) "GLOBAL CONST-PROP: Replacing reg %d in ", regno); fprintf (dump_file, "insn %d with constant ", INSN_UID (insn)); - print_rtl (dump_file, src); + print_rtl (dump_file, src_cst); fprintf (dump_file, "\n"); } if (insn->deleted ()) return 1; } - } - else if (REG_P (src) - && REGNO (src) >= FIRST_PSEUDO_REGISTER - && REGNO (src) != regno) + /* Copy propagation. */ + else if (src_reg && cprop_reg_p (src_reg) + && REGNO (src_reg) != regno + && try_replace_reg (reg_used, src_reg, insn)) { - if (try_replace_reg (reg_used, src, insn)) - { changed_this_round = changed = 1; global_copy_prop_count++; if (dump_file != NULL) @@ -1113,7 +1116,7 @@ cprop_insn (rtx_insn *insn) fprintf (dump_file, "GLOBAL COPY-PROP: Replacing reg %d in insn %d", regno, INSN_UID (insn)); - fprintf (dump_file, " with reg %d\n", REGNO (src)); + fprintf (dump_file, " with reg %d\n", REGNO (src_reg)); } /* The original insn setting reg_used may or may not now be @@ -1123,12 +1126,10 @@ cprop_insn (rtx_insn *insn) and made things worse. */ } } - + } /* If try_replace_reg simplified the insn, the regs found by find_used_regs may not be valid anymore. Start over. */ - if (changed_this_round) - goto retry; - } + while (changed_this_round); if (changed && DEBUG_INSN_P (insn)) return 0; @@ -1191,7 +1192,7 @@ do_local_cprop (rtx x, rtx_insn *insn) /* Rule out USE instructions and ASM statements as we don't want to change the hard registers mentioned. */ if (REG_P (x) - && (REGNO (x) >= FIRST_PSEUDO_REGISTER + && (cprop_reg_p (x) || (GET_CODE (PATTERN (insn)) != USE && asm_noperands (PATTERN (insn)) < 0))) { @@ -1207,7 +1208,7 @@ do_local_cprop (rtx x, rtx_insn *insn) if (cprop_constant_p (this_rtx)) newcnst = this_rtx; - if (REG_P (this_rtx) && REGNO (this_rtx) >= FIRST_PSEUDO_REGISTER + if (cprop_reg_p (this_rtx) /* Don't copy propagate if it has attached REG_EQUIV note. At this point this only function parameters should have REG_EQUIV notes and if the argument slot is used somewhere @@ -1328,9 +1329,8 @@ implicit_set_cond_p (const_rtx cond) if (GET_CODE (cond) != EQ && GET_CODE (cond) != NE) return false; - /* The first operand of COND must be a pseudo-reg. */ - if (! REG_P (XEXP (cond, 0)) - || HARD_REGISTER_P (XEXP (cond, 0))) + /* The first operand of COND must be a register we can propagate. */ + if (cprop_reg_p (XEXP (cond, 0))) return false; /* The second operand of COND must be a suitable constant. */