From patchwork Mon Dec 1 07:13:30 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zhenqiang Chen X-Patchwork-Id: 416312 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 C3D9C140151 for ; Mon, 1 Dec 2014 18:13:58 +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:references:in-reply-to:subject:date:message-id :mime-version:content-type:content-transfer-encoding; q=dns; s= default; b=R5OWUyuGIH+E7lSBNQP/Nzn5A6QSpr0IKEMxyDl8YCk9yfcTSIOvk e3PTs0C5Iuh+VzlFApAqX3t50TfZydtyEUjqdjbr8i3Sz59LxXNaw4ol8eghC6Cl z7iC6s9U9POkFXzj1wmn9Uk/ZjBZvrOvmSfKGAkZM7aG9m3fLPpAR0= 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:references:in-reply-to:subject:date:message-id :mime-version:content-type:content-transfer-encoding; s=default; bh=Juc6tLwagstEOcdKypzVD9OEHSY=; b=UGKMslOs7eWpVRVEmykt6lXyetTU UCfwV9l7pjvvd7GPwgcKLUU+aQteaBsYnDgji3nBiaJO66OXDJTrv8Pri0EsoFsk C188jyovaR4j5mgNBIs7WwvNFuPhtUxzhjjwULblX676Lxdj+cnHcDnM9GkSUt7K xTyym/0SSIrBl5M= Received: (qmail 28342 invoked by alias); 1 Dec 2014 07:13:50 -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 28329 invoked by uid 89); 1 Dec 2014 07:13:49 -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; Mon, 01 Dec 2014 07:13:46 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by service87.mimecast.com; Mon, 01 Dec 2014 07:13:43 +0000 Received: from shawin003 ([10.164.2.32]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 1 Dec 2014 07:13:43 +0000 From: "Zhenqiang Chen" To: "'H.J. Lu'" Cc: "Richard Henderson" , "GCC Patches" References: <000001d004a7$2248cbb0$66da6310$@arm.com> <546E3287.9020900@redhat.com> <000001d00799$5311ca90$f9355fb0$@arm.com> In-Reply-To: Subject: RE: [PATCH, ifcvt] Fix PR63917 Date: Mon, 1 Dec 2014 15:13:30 +0800 Message-ID: <000001d00d36$507b7e50$f1727af0$@arm.com> MIME-Version: 1.0 X-MC-Unique: 114120107134300201 X-IsSubscribed: yes > -----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; +}