From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24792 invoked by alias); 10 Feb 2014 23:06:55 -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 24782 invoked by uid 89); 10 Feb 2014 23:06:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham 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 ESMTP; Mon, 10 Feb 2014 23:06:54 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s1AN6q2F012616 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 10 Feb 2014 18:06:52 -0500 Received: from stumpy.slc.redhat.com (ovpn-113-146.phx2.redhat.com [10.3.113.146]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s1AN6q6T001917 for ; Mon, 10 Feb 2014 18:06:52 -0500 Message-ID: <52F95B8C.60809@redhat.com> Date: Mon, 10 Feb 2014 23:06:00 -0000 From: Jeff Law User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 MIME-Version: 1.0 To: gcc-patches Subject: [RFA] [PATCH] [rtl-optimization/60131] Fix rtl-checking failure in REE Content-Type: multipart/mixed; boundary="------------070207030405050404030904" X-IsSubscribed: yes X-SW-Source: 2014-02/txt/msg00635.txt.bz2 This is a multi-part message in MIME format. --------------070207030405050404030904 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1951 As mentioned in the BZ, we have the following insns: (insn 24 23 25 6 (set (reg:DI 0 ax [orig:100 D.2269 ] [100]) (zero_extend:DI (reg/v:SI 0 ax [orig:91 v ] [91]))) j.c:22 133 {*zero_extendsidi2} (nil)) $13 = void (gdb) p debug_rtx (curr_insn) (insn 33 32 35 7 (set (reg/v:SI 0 ax [orig:91 v ] [91]) (sign_extend:SI (reg:HI 0 ax [orig:88 D.2271 ] [88]))) j.c:19 146 {extendhisi2} (nil)) We eliminate insn 24 by changing insn 33 into: (insn 33 32 35 7 (set (reg:DI 0 ax) (zero_extend:DI (sign_extend:SI (reg:HI 0 ax [orig:88 D.2271 ] [88])))) j.c:19 -1 (nil)) Later we call combine_reaching_defs to see if we can eliminate insn 33. Unfortunately the tests for the srcreg != destreg case assume that there are no nested extensions and we run afoul of RTL checking. There's two cases to consider with nested extensions. When the two registers are the same, we want to do nothing as the old code handles that case correctly. When the registers are different, we want to avoid the optimization simply because not all the code for handling the srcreg != destreg case has been vetted for nested extensions. So clearly the way to go is to fix the test in combine_reaching_defs not to barf on nested extensions. When the srcreg != destreg we still go into the true arm which will always return false if there was a nested extension (only way to get nested extensions at this point in code will be if state->modified != EXT_MODIFIED_NONE). When srcreg == destreg we fall-thru to the original ree code which does the right thing. A similar test in find_and_remove_re needs updating as well. It's simpler because we're simply disallowing srcreg != destreg with nested extensions. We don't need to find the regno, just check if we have (extend (reg)) or not. Bootstrapped and regression tested (rtl checking enabled) on x86_64-unknown-linux-gnu. OK for the trunk? Thanks, Jeff --------------070207030405050404030904 Content-Type: text/plain; charset=us-ascii; name="P" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="P" Content-length: 4412 diff --git a/gcc/ChangeLog b/gcc/ChangeLog index a5ad4fa..ee2165c 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,12 @@ +2014-02-10 Jeff Law + + PR rtl-optimization/60131 + * ree.c (get_extended_src_reg): New function. + (combine_reaching_defs): Use it rather than assuming location + of REG. + (find_and_remove_re): Verify first operand of extension is + a REG before adding the insns to the copy list. + 2014-02-10 Bernd Edlinger PR middle-end/60080 diff --git a/gcc/ree.c b/gcc/ree.c index 421eb6c..fcde9a0 100644 --- a/gcc/ree.c +++ b/gcc/ree.c @@ -670,6 +670,18 @@ merge_def_and_ext (ext_cand *cand, rtx def_insn, ext_state *state) return false; } +/* Given SRC, which should be one or more extensions of a REG, strip + away the extensions and return the REG. */ + +static inline rtx +get_extended_src_reg (rtx src) +{ + while (GET_CODE (src) == SIGN_EXTEND || GET_CODE (src) == ZERO_EXTEND) + src = XEXP (src, 0); + gcc_assert (REG_P (src)); + return src; +} + /* This function goes through all reaching defs of the source of the candidate for elimination (CAND) and tries to combine the extension with the definition instruction. The changes @@ -698,17 +710,23 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state) /* If the destination operand of the extension is a different register than the source operand, then additional restrictions - are needed. */ - if ((REGNO (SET_DEST (PATTERN (cand->insn))) - != REGNO (XEXP (SET_SRC (PATTERN (cand->insn)), 0)))) + are needed. Note we have to handle cases where we have nested + extensions in the source operand. */ + if (REGNO (SET_DEST (PATTERN (cand->insn))) + != REGNO (get_extended_src_reg (SET_SRC (PATTERN (cand->insn))))) { /* In theory we could handle more than one reaching def, it just makes the code to update the insn stream more complex. */ if (state->defs_list.length () != 1) return false; - /* We require the candidate not already be modified. This may - be overly conservative. */ + /* We require the candidate not already be modified. It may, + for example have been changed from a (sign_extend (reg)) + into (zero_extend (sign_extend (reg)). + + Handling that case shouldn't be terribly difficult, but the code + here and the code to emit copies would need auditing. Until + we see a need, this is the safe thing to do. */ if (state->modified[INSN_UID (cand->insn)].kind != EXT_MODIFIED_NONE) return false; @@ -999,8 +1017,13 @@ find_and_remove_re (void) if (dump_file) fprintf (dump_file, "Eliminated the extension.\n"); num_realized++; - if (REGNO (SET_DEST (PATTERN (curr_cand->insn))) - != REGNO (XEXP (SET_SRC (PATTERN (curr_cand->insn)), 0))) + /* If the RHS of the current candidate is not (extend (reg)), then + we do not allow the optimization of extensions where + the source and destination registers do not match. Thus + checking REG_P here is correct. */ + if (REG_P (XEXP (SET_SRC (PATTERN (curr_cand->insn)), 0)) + && (REGNO (SET_DEST (PATTERN (curr_cand->insn))) + != REGNO (XEXP (SET_SRC (PATTERN (curr_cand->insn)), 0)))) { reinsn_copy_list.safe_push (curr_cand->insn); reinsn_copy_list.safe_push (state.defs_list[0]); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index dd2c268..dfa1fb2 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2014-02-10 Jeff Law + + PR rtl-optimization/60131 + * g++.dg/torture/pr60131.C: New test. + 2014-02-10 Rainer Orth * gcc.dg/binop-xor1.c: Don't xfail scan-tree-dump-times. diff --git a/gcc/testsuite/g++.dg/torture/pr60131.C b/gcc/testsuite/g++.dg/torture/pr60131.C new file mode 100644 index 0000000..23dde31 --- /dev/null +++ b/gcc/testsuite/g++.dg/torture/pr60131.C @@ -0,0 +1,23 @@ +// { dg-do compile } +struct A { short a; }; +int **b; +unsigned long c; + +bool foo (); +unsigned bar (unsigned i); +extern void baz () __attribute__((noreturn)); + +int * +test (unsigned x, struct A *y) +{ + unsigned v; + if (foo () || y[x].a == -1) + { + c = bar (x); + return 0; + } + v = y[x].a; + if (v >= 23) + baz (); + return b[v]; +} --------------070207030405050404030904--