From patchwork Fri Dec 5 01:16:18 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zhenqiang Chen X-Patchwork-Id: 417972 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 96484140082 for ; Fri, 5 Dec 2014 12:16:50 +1100 (AEDT) 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:cc:subject:date:message-id:mime-version:content-type :content-transfer-encoding; q=dns; s=default; b=ItmDD81z0C1qBHwM aaM1BqoRwQ/PjzvByqgVYRHeMy3hz+Q/EUNci7MTBuU7fDhg8aomfAGzyGSeWrxZ l/d2TZlUHf4kr2j9Z0mx9xjrdWPwFXNOsr/fP3TrJBk+UHsKDifW6oJSJ39VhDxO 73tByt3pLpFoWqaJV1rwMp6PYsY= 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:cc:subject:date:message-id:mime-version:content-type :content-transfer-encoding; s=default; bh=KeO3aV4NW0tFoAMOrYdzAI 8TKKA=; b=ZUhWIXb1SeJphCXyoHikY9NjRdJySo0qRrsRXtQU1DnqtATh5n/CaD 5chOHseGF+eu22cE66XQ3kW56rkUNIBpBbnTF24rQKSEQs5BoSwjVHMp2rtbv5Uq ny+aR08vutRrR9x9F86G5qFtgAcMLuzhzp4a1hiJ5ysjM7jVgfk5Y= Received: (qmail 3535 invoked by alias); 5 Dec 2014 01:16:42 -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 3504 invoked by uid 89); 5 Dec 2014 01:16:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: service87.mimecast.com Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 05 Dec 2014 01:16:38 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by service87.mimecast.com; Fri, 05 Dec 2014 01:16:36 +0000 Received: from shawin003 ([10.164.2.24]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Fri, 5 Dec 2014 01:16:34 +0000 From: "Zhenqiang Chen" To: "Richard Henderson" Cc: Subject: [Ping] [PATCH, ifcvt] Fix PR63917 Date: Fri, 5 Dec 2014 09:16:18 +0800 Message-ID: <000001d01029$14249630$3c6dc290$@arm.com> MIME-Version: 1.0 X-MC-Unique: 114120501163600401 X-IsSubscribed: yes Ping? Thanks! -Zhenqiang -----Original Message----- From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] On Behalf Of Zhenqiang Chen Sent: Monday, December 01, 2014 3:14 PM To: 'H.J. Lu' Cc: Richard Henderson; GCC Patches Subject: RE: [PATCH, ifcvt] Fix PR63917 > -----Original Message----- > From: H.J. Lu [mailto:hjl.tools@gmail.com] > Sent: Friday, November 28, 2014 10:45 PM > To: Zhenqiang Chen > Cc: Richard Henderson; GCC Patches > Subject: Re: [PATCH, ifcvt] Fix PR63917 > > On Sun, Nov 23, 2014 at 7:47 PM, Zhenqiang Chen > wrote: > > > >> -----Original Message----- > >> From: Richard Henderson [mailto:rth@redhat.com] > >> Sent: Friday, November 21, 2014 2:27 AM > >> To: Zhenqiang Chen; gcc-patches@gcc.gnu.org > >> Subject: Re: [PATCH, ifcvt] Fix PR63917 > >> > >> On 11/20/2014 10:48 AM, Zhenqiang Chen wrote: > >> > +/* Check X clobber CC reg or not. */ > >> > + > >> > +static bool > >> > +clobber_cc_p (rtx x) > >> > +{ > >> > + RTX_CODE code = GET_CODE (x); > >> > + int i; > >> > + > >> > + if (code == CLOBBER > >> > + && REG_P (XEXP (x, 0)) > >> > + && (GET_MODE_CLASS (GET_MODE (XEXP (x, 0))) == MODE_CC)) > >> > + return TRUE; > >> > + else if (code == PARALLEL) > >> > + for (i = 0; i < XVECLEN (x, 0); i++) > >> > + if (clobber_cc_p (XVECEXP (x, 0, i))) > >> > + return TRUE; > >> > + return FALSE; > >> > +} > >> > >> Why would you need something like this when modified_between_p or > one > >> of its kin ought to do the job? > > > > Thanks for the comments. Patch is updated to use set_of. > > > > And it is also enhanced to make sure that the new generated insns > > can not clobber CC. > > > > Bootstrap and no make check regression on X86-64 and IA32. > > > > OK for trunk? > > > > Thanks! > > -Zhenqiang > > > > ChangeLog: > > 2014-11-24 Zhenqiang Chen > > > > PR rtl-optimization/63917 > > * ifcvt.c (cc_in_cond): New function. > > (end_ifcvt_sequence): Make sure new generated insns do not > > clobber CC. > > (noce_process_if_block, check_cond_move_block): Check CC > references. > > > > testsuite/ChangeLog: > > 2014-11-24 Zhenqiang Chen > > > > * gcc.dg/pr63917.c: New test. > > > > diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index 21f08c2..1acd0ff 100644 > > --- a/gcc/ifcvt.c > > +++ b/gcc/ifcvt.c > > @@ -1016,6 +1016,18 @@ noce_emit_move_insn (rtx x, rtx y) > > 0, 0, outmode, y); } > > > > +/* Return the CC reg if it is used in COND. */ > > + > > +static rtx > > +cc_in_cond (rtx cond) > > +{ > > + if ((HAVE_cbranchcc4) && cond > > + && (GET_MODE_CLASS (GET_MODE (XEXP (cond, 0))) == MODE_CC)) > > + return XEXP (cond, 0); > > + > > + return NULL_RTX; > > +} > > + > > /* Return sequence of instructions generated by if conversion. This > > function calls end_sequence() to end the current stream, ensures > > that are instructions are unshared, recognizable non-jump insns. > > @@ -1026,6 +1038,7 @@ end_ifcvt_sequence (struct noce_if_info > > *if_info) { > > rtx_insn *insn; > > rtx_insn *seq = get_insns (); > > + rtx cc = cc_in_cond (if_info->cond); > > > > set_used_flags (if_info->x); > > set_used_flags (if_info->cond); > > @@ -1040,7 +1053,9 @@ end_ifcvt_sequence (struct noce_if_info *if_info) > > allows proper placement of required clobbers. */ > > for (insn = seq; insn; insn = NEXT_INSN (insn)) > > if (JUMP_P (insn) > > - || recog_memoized (insn) == -1) > > + || recog_memoized (insn) == -1 > > + /* Make sure new generated code does not clobber CC. */ > > + || (cc && set_of (cc, insn))) > > return NULL; > > > > return seq; > > @@ -2544,6 +2559,7 @@ noce_process_if_block (struct noce_if_info > *if_info) > > rtx_insn *insn_a, *insn_b; > > rtx set_a, set_b; > > rtx orig_x, x, a, b; > > + rtx cc; > > > > /* We're looking for patterns of the form > > > > @@ -2655,6 +2671,13 @@ noce_process_if_block (struct noce_if_info > *if_info) > > if_info->a = a; > > if_info->b = b; > > > > + /* Skip it if the instruction to be moved might clobber CC. */ > > + cc = cc_in_cond (cond); if (cc) > > + if (set_of (cc, insn_a) > > + || (insn_b && set_of (XEXP (cond, 0), insn_b))) > > + return FALSE; > > + > > /* Try optimizations in some approximation of a useful order. */ > > /* ??? Should first look to see if X is live incoming at all. If it > > isn't, we don't need anything but an unconditional set. */ @@ > > -2811,6 +2834,7 @@ check_cond_move_block (basic_block bb, > > rtx cond) > > { > > rtx_insn *insn; > > + rtx cc = cc_in_cond (cond); > > > > /* We can only handle simple jumps at the end of the basic block. > > It is almost impossible to update the CFG otherwise. */ @@ > > -2868,6 +2892,10 @@ check_cond_move_block (basic_block bb, > > && modified_between_p (src, insn, NEXT_INSN (BB_END (bb)))) > > return FALSE; > > > > + /* Skip it if the instruction to be moved might clobber CC. */ > > + if (cc && set_of (cc, insn)) > > + return FALSE; > > + > > vals->put (dest, src); > > > > regs->safe_push (dest); > > diff --git a/gcc/testsuite/gcc.dg/pr63917.c > > b/gcc/testsuite/gcc.dg/pr63917.c new file mode 100644 index > > 0000000..422b15d > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/pr63917.c > > It should be pr64007.c. > > > @@ -0,0 +1,45 @@ > > +/* { dg-options " -O3 " } */ > > You need > > /* { dg-do run } */ > > since it is a run-time test. > > > + > > +int d, i; > > + > > +struct S > > +{ > > + int f0; > > +} *b, c, e, h, **g = &b; > > + > > +static struct S *f = &e; > > + > > +int > > +fn1 (int p) > > +{ > > + int a = 0; > > + return a || p < 0 || p >= 2 || 1 >> p; } > > + > > +int > > +main () > > +{ > > + int k = 1, l, *m = &c.f0; > > + > > + for (;;) > > + { > > + l = fn1 (i); > > + *m = k && i; > > + if (l) > > + { > > + int n[1] = {0}; > > + } > > + break; > > + } > > + > > + *g = &h; > > + > > + if (d) > > + (*m)--; > > + d = (f != 0) | (i >= 0); > > + > > + if (c.f0 != 0) > > + __builtin_abort (); > > + > > + return 0; > > +} > > > > You test doesn't fail without the fix since your test is a little bit > different from the test at > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64007 > > You missed: > > assert (b); > > Without it, the bug won't be triggered. Thanks for the comments. Test case in the patch is updated. diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index 21f08c2..1acd0ff 100644 --- a/gcc/ifcvt.c +++ b/gcc/ifcvt.c @@ -1016,6 +1016,18 @@ noce_emit_move_insn (rtx x, rtx y) 0, 0, outmode, y); } +/* Return the CC reg if it is used in COND. */ + +static rtx +cc_in_cond (rtx cond) +{ + if ((HAVE_cbranchcc4) && cond + && (GET_MODE_CLASS (GET_MODE (XEXP (cond, 0))) == MODE_CC)) + return XEXP (cond, 0); + + return NULL_RTX; +} + /* Return sequence of instructions generated by if conversion. This function calls end_sequence() to end the current stream, ensures that are instructions are unshared, recognizable non-jump insns. @@ -1026,6 +1038,7 @@ end_ifcvt_sequence (struct noce_if_info *if_info) { rtx_insn *insn; rtx_insn *seq = get_insns (); + rtx cc = cc_in_cond (if_info->cond); set_used_flags (if_info->x); set_used_flags (if_info->cond); @@ -1040,7 +1053,9 @@ end_ifcvt_sequence (struct noce_if_info *if_info) allows proper placement of required clobbers. */ for (insn = seq; insn; insn = NEXT_INSN (insn)) if (JUMP_P (insn) - || recog_memoized (insn) == -1) + || recog_memoized (insn) == -1 + /* Make sure new generated code does not clobber CC. */ + || (cc && set_of (cc, insn))) return NULL; return seq; @@ -2544,6 +2559,7 @@ noce_process_if_block (struct noce_if_info *if_info) rtx_insn *insn_a, *insn_b; rtx set_a, set_b; rtx orig_x, x, a, b; + rtx cc; /* We're looking for patterns of the form @@ -2655,6 +2671,13 @@ noce_process_if_block (struct noce_if_info *if_info) if_info->a = a; if_info->b = b; + /* Skip it if the instruction to be moved might clobber CC. */ + cc = cc_in_cond (cond); + if (cc) + if (set_of (cc, insn_a) + || (insn_b && set_of (XEXP (cond, 0), insn_b))) + return FALSE; + /* Try optimizations in some approximation of a useful order. */ /* ??? Should first look to see if X is live incoming at all. If it isn't, we don't need anything but an unconditional set. */ @@ -2811,6 +2834,7 @@ check_cond_move_block (basic_block bb, rtx cond) { rtx_insn *insn; + rtx cc = cc_in_cond (cond); /* We can only handle simple jumps at the end of the basic block. It is almost impossible to update the CFG otherwise. */ @@ -2868,6 +2892,10 @@ check_cond_move_block (basic_block bb, && modified_between_p (src, insn, NEXT_INSN (BB_END (bb)))) return FALSE; + /* Skip it if the instruction to be moved might clobber CC. */ + if (cc && set_of (cc, insn)) + return FALSE; + vals->put (dest, src); regs->safe_push (dest); diff --git a/gcc/testsuite/gcc.dg/pr64007.c b/gcc/testsuite/gcc.dg/pr64007.c new file mode 100644 index 0000000..cb0e50f --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr64007.c @@ -0,0 +1,50 @@ +/* { dg-options " -O3 " } */ +/* { dg-do run } */ + +#include + +int d, i; + +struct S +{ + int f0; +} *b, c, e, h, **g = &b; + +static struct S *f = &e; + +int +fn1 (int p) +{ + int a = 0; + return a || p < 0 || p >= 2 || 1 >> p; } + +int +main () +{ + int k = 1, l, *m = &c.f0; + + for (;;) + { + l = fn1 (i); + *m = k && i; + if (l) + { + int n[1] = {0}; + } + break; + } + + *g = &h; + + assert (b); + + if (d) + (*m)--; + d = (f != 0) | (i >= 0); + + if (c.f0 != 0) + __builtin_abort (); + + return 0; +}