From patchwork Thu Apr 16 08:43:33 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Preud'homme X-Patchwork-Id: 461725 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 7ACE01401AB for ; Thu, 16 Apr 2015 18:44:12 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass reason="1024-bit key; unprotected key" header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=l69zHh1b; 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:from :to:references:in-reply-to:subject:date:message-id:mime-version :content-type:content-transfer-encoding; q=dns; s=default; b=iSd 3st7qAo+FbS6YrrI14lwb/ChlbJgqs+ZaDkOqg2f53E0m9zU0+KsIjQigh3ZIypX BeBUoAulQsVFNIKkDc16L1xiRbRImqG7oKG1mf+iGOSm8fwr8cMTnIPtt6Yo7KdF 2hW755AfOfpBjAONq81KfVKs5pVuyCC1isFnNf3w= 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:from :to:references:in-reply-to:subject:date:message-id:mime-version :content-type:content-transfer-encoding; s=default; bh=wwzx//uD/ sEmdJVSPs1W4aV4YJo=; b=l69zHh1b2pqV8IoMcco1OkLxuudxrUBG010EjpE3x VLh1kcOwX8paWhDSVkPkL9Ip41K5z9qrC/7k2ytnPAMPYuLF08Eph5i68h83BhAw lOaMno1peGa7oNr8WTVTvqdfK3HwTdxbXRRwaRx7aYDcTU0xOMMF3njlrD9/+0Uf cE= Received: (qmail 86450 invoked by alias); 16 Apr 2015 08:44:03 -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 86432 invoked by uid 89); 16 Apr 2015 08:44:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 16 Apr 2015 08:43:59 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by uk-mta-5.uk.mimecast.lan; Thu, 16 Apr 2015 09:43:49 +0100 Received: from SHAWIN202 ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Thu, 16 Apr 2015 09:43:48 +0100 From: "Thomas Preud'homme" To: "Steven Bosscher" , "'Jeff Law'" , , "'Richard Biener'" References: <000501d049d3$079385a0$16ba90e0$@arm.com> <552BBAF9.2010504@redhat.com> In-Reply-To: <552BBAF9.2010504@redhat.com> Subject: RE: [PATCH, GCC, stage1] Fallback to copy-prop if constant-prop not possible Date: Thu, 16 Apr 2015 16:43:33 +0800 Message-ID: <000001d07821$6fb82f60$4f288e20$@arm.com> MIME-Version: 1.0 X-MC-Unique: ymfSdA5sS1WIWOi2eNIiPQ-1 X-IsSubscribed: yes > From: Jeff Law [mailto:law@redhat.com] > Sent: Monday, April 13, 2015 8:48 PM > > I know there were several followups between Steven and yourself. > With > stage1 now open, can you post a final version and do a final > bootstrap/test with it? Here is what came out of our discussion with Steven: The RTL cprop pass in GCC operates by doing a local constant/copy propagation first and then a global one. In the local one, if a constant cannot be propagated (eg. due to constraints of the destination instruction) a copy propagation is done instead. However, at the global level copy propagation is only tried if no constant can be propagated, ie. if a constant can be propagated but the constraints of the destination instruction forbids it, no copy propagation will be tried. This patch fixes this issue. ChangeLog entries are as follows: *** gcc/ChangeLog *** 2015-04-15 Thomas Preud'homme Steven Bosscher * cprop.c (cprop_reg_p): New. (hash_scan_set): Use above function to check if register can be propagated. (find_avail_set): Return up to two sets, one whose source is a register and one whose source is a constant. Sets are returned in an array passed as parameter rather than as a return value. (cprop_insn): Use a do while loop rather than a goto. Try each of the sets returned by find_avail_set, starting with the one whose source is a constant. Use cprop_reg_p to check if register can be propagated. (do_local_cprop): Use cprop_reg_p to check if register can be propagated. (implicit_set_cond_p): Likewise. *** gcc/testsuite/ChangeLog *** 2015-04-15 Thomas Preud'homme Steven Bosscher * gcc.target/arm/pr64616.c: New file. And the patch is: Best regards, Thomas diff --git a/gcc/cprop.c b/gcc/cprop.c index c9fb2fc..78541cf 100644 --- a/gcc/cprop.c +++ b/gcc/cprop.c @@ -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 hash_table_d *table, 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 hash_table_d *table, 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 @@ find_avail_set (int regno, 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 @@ find_avail_set (int regno, 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,40 +1055,40 @@ cprop_insn (rtx_insn *insn) int changed = 0, changed_this_round; rtx note; -retry: - changed_this_round = 0; - reg_use_count = 0; - note_uses (&PATTERN (insn), find_used_regs, NULL); - - /* We may win even when propagating constants into notes. */ - note = find_reg_equal_equiv_note (insn); - if (note) - find_used_regs (&XEXP (note, 0), NULL); - - for (i = 0; i < reg_use_count; i++) + do { - rtx reg_used = reg_use_table[i]; - unsigned int regno = REGNO (reg_used); - rtx src; - struct cprop_expr *set; + changed_this_round = 0; + reg_use_count = 0; + note_uses (&PATTERN (insn), find_used_regs, NULL); - /* If the register has already been set in this block, there's - nothing we can do. */ - if (! reg_not_set_p (reg_used, insn)) - continue; + /* We may win even when propagating constants into notes. */ + note = find_reg_equal_equiv_note (insn); + if (note) + find_used_regs (&XEXP (note, 0), NULL); - /* 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; + for (i = 0; i < reg_use_count; i++) + { + rtx reg_used = reg_use_table[i]; + unsigned int regno = REGNO (reg_used); + rtx src_cst = NULL, src_reg = NULL; + struct cprop_expr *set[2]; - src = set->src; + /* If the register has already been set in this block, there's + nothing we can do. */ + if (! reg_not_set_p (reg_used, insn)) + continue; - /* Constant propagation. */ - if (cprop_constant_p (src)) - { - if (constprop_register (reg_used, src, insn)) + /* Find an assignment that sets reg_used and is available + at the start of the block. */ + find_avail_set (regno, insn, set); + if (set[0]) + src_reg = set[0]->src; + if (set[1]) + src_cst = set[1]->src; + + /* Constant propagation. */ + if (src_cst && cprop_constant_p (src_cst) + && constprop_register (reg_used, src_cst, insn)) { changed_this_round = changed = 1; global_const_prop_count++; @@ -1093,18 +1098,16 @@ retry: "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) - { - if (try_replace_reg (reg_used, src, insn)) + /* Copy propagation. */ + else if (src_reg && cprop_reg_p (src_reg) + && REGNO (src_reg) != regno + && try_replace_reg (reg_used, src_reg, insn)) { changed_this_round = changed = 1; global_copy_prop_count++; @@ -1113,7 +1116,7 @@ retry: 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 @@ retry: 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; } + /* If try_replace_reg simplified the insn, the regs found by find_used_regs + may not be valid anymore. Start over. */ + 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. */ diff --git a/gcc/testsuite/gcc.target/arm/pr64616.c b/gcc/testsuite/gcc.target/arm/pr64616.c new file mode 100644 index 0000000..c686ffa --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pr64616.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +int f (int); +unsigned int glob; + +void +g () +{ + while (f (glob)); + glob = 5; +} + +/* { dg-final { scan-assembler-times "ldr" 2 } } */