From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 47692 invoked by alias); 23 Nov 2015 15:12:41 -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 47680 invoked by uid 89); 23 Nov 2015 15:12:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 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; Mon, 23 Nov 2015 15:12:39 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-11-8yXttbnKSd6CE3ZqEqJDUg-1; Mon, 23 Nov 2015 15:12:33 +0000 Received: from [10.2.206.200] ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 23 Nov 2015 15:12:32 +0000 Message-ID: <56532CE0.9050802@arm.com> Date: Mon, 23 Nov 2015 15:12:00 -0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Bernd Schmidt , GCC Patches CC: Jeff Law Subject: Re: [PATCH][RTL-ree] PR rtl-optimization/68194: Restrict copy instruction in presence of conditional moves References: <5649E333.4090904@arm.com> <564A2339.3030308@redhat.com> <564AEE94.3070708@arm.com> <564B1934.6050300@redhat.com> <564B259A.90206@arm.com> <564BB42C.1020401@redhat.com> <564C40D1.80409@arm.com> <564DA454.2080704@arm.com> <564E7A47.1090404@redhat.com> In-Reply-To: <564E7A47.1090404@redhat.com> X-MC-Unique: 8yXttbnKSd6CE3ZqEqJDUg-1 Content-Type: multipart/mixed; boundary="------------090103000704050602090304" X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg02740.txt.bz2 This is a multi-part message in MIME format. --------------090103000704050602090304 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Content-length: 2050 Hi Bernd, On 20/11/15 01:41, Bernd Schmidt wrote: >>> I1 is def_insn, I3 is cand->insn. tmp_reg is 'ax'. What we want to do >>> is reject this transformation >>> because the destination of def_insn (aka I1), that is 'ax', is not the >>> operand of the extend operation >>> in cand->insn (aka I3). As you said, rtx_equal won't work on just >>> SET_SRC (PATTERN (cand->insn)) because >>> it's an extend operation. So reg_overlap_mentioned should be appropriat= e. > > Yeah, so just use the src_reg variable for the comparison. I still don't = see why you wouldn't want to use the stronger test. But the whole thing sti= ll feels not completely ideal somehow, so after reading through ree.c for a= while and=20 > getting a better feeling for how it works, I think the following (which y= ou said is equivalent) would be the most understandable and direct fix. > > You said that the two tests should be equivalent, and I agree. I've not f= ound cases where the change makes a difference, other than the testcase. Wo= uld you mind running this version through the testsuite and committing if i= t passes? > > I've shrunk the comment; massive explanations like this for every bug are= inappropriate IMO, and the example also duplicates an earlier comment in t= he same function. And, as I said earlier, the way you placed the comment is= confusing=20 > because only one part of the following if statement is related to it. > Thanks for the comments, here is the final patch that I'll be committing. It passed testing on arm, aarch64, x86_64. Thanks, Kyrill 2015-11-23 Bernd Schmidt Kyrylo Tkachov PR rtl-optimization/68194 PR rtl-optimization/68328 PR rtl-optimization/68185 * ree.c (combine_reaching_defs): Reject copy_needed case if copies_list is not empty. 2015-11-23 Kyrylo Tkachov PR rtl-optimization/68194 * gcc.c-torture/execute/pr68185.c: Likewise. * gcc.c-torture/execute/pr68328.c: Likewise. > > Bernd --------------090103000704050602090304 Content-Type: text/x-patch; name=ree-fix.patch Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="ree-fix.patch" Content-length: 2426 commit ceecbb45212e2c2a6650000fabba03e07f6fcbe4 Author: Kyrylo Tkachov Date: Fri Nov 13 15:01:47 2015 +0000 [RTL-ree] PR rtl-optimization/68194: Restrict copy instruction in prese= nce of conditional moves diff --git a/gcc/ree.c b/gcc/ree.c index b8436f2..9d94843 100644 --- a/gcc/ree.c +++ b/gcc/ree.c @@ -770,6 +770,12 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_p= at, ext_state *state) if (state->defs_list.length () !=3D 1) return false; =20 + /* We don't have the structure described above if there are + conditional moves in between the def and the candidate, + and we will not handle them correctly. See PR68194. */ + if (state->copies_list.length () > 0) + return false; + /* 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))). diff --git a/gcc/testsuite/gcc.c-torture/execute/pr68185.c b/gcc/testsuite/= gcc.c-torture/execute/pr68185.c new file mode 100644 index 0000000..826531b --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr68185.c @@ -0,0 +1,29 @@ +int a, b, d =3D 1, e, f, o, u, w =3D 1, z; +short c, q, t; + +int +main () +{ + char g; + for (; d; d--) + { + while (o) + for (; e;) + { + c =3D b; + int h =3D o =3D z; + for (; u;) + for (; a;) + ; + } + if (t < 1) + g =3D w; + f =3D g; + g && (q =3D 1); + } + + if (q !=3D 1) + __builtin_abort (); + + return 0; +} diff --git a/gcc/testsuite/gcc.c-torture/execute/pr68328.c b/gcc/testsuite/= gcc.c-torture/execute/pr68328.c new file mode 100644 index 0000000..edf244b --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr68328.c @@ -0,0 +1,44 @@ +int a, b, c =3D 1, d =3D 1, e; + +__attribute__ ((noinline, noclone)) + int foo (void) +{ + asm volatile ("":::"memory"); + return 4195552; +} + +__attribute__ ((noinline, noclone)) + void bar (int x, int y) +{ + asm volatile (""::"g" (x), "g" (y):"memory"); + if (y =3D=3D 0) + __builtin_abort (); +} + +int +baz (int x) +{ + char g, h; + int i, j; + + foo (); + for (;;) + { + if (c) + h =3D d; + g =3D h < x ? h : 0; + i =3D (signed char) ((unsigned char) (g - 120) ^ 1); + j =3D i > 97; + if (a - j) + bar (0x123456, 0); + if (!b) + return e; + } +} + +int +main () +{ + baz (2); + return 0; +} --------------090103000704050602090304--