From patchwork Sat Aug 9 05:14:21 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 378719 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 B8EA1140087 for ; Sat, 9 Aug 2014 15:14:41 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=IQsM21b1gKpq/4W/0 LDy3J76yujhUdzA5oYfJG3XQdDDCkXizX3gwFbGfdUbu6WnfdwMlnaQ/7+vRPlyp 6+pGZTdwnzBbv1jHypxMUlKcnI3atlLLvIpe4jBKateBjK58W7LeNVhm0yA+1NBl bWksQGOVi70F1s8jv3ki78wwWY= 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 :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=xbQiYKvCiERw3SFYVA5mtbe G42k=; b=GPBmoDEup1xSRbaHfA9OmWMxOA3qzolnrki7kiQpFoLTx8ik69jFXnS XDh2YPAUPrEBoTQRbiiOxiEM1BDKAPMkqIE61Mq1HpEDJF3D9kmY8lQuus6DL/QH D6lAnz48Vq1Wd4QnYK2rSmPECfc3sb2qx23X46PvdAZbyG9MZ2o8= Received: (qmail 7034 invoked by alias); 9 Aug 2014 05:14:34 -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 7020 invoked by uid 89); 9 Aug 2014 05:14:32 -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 autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 09 Aug 2014 05:14:30 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1XFyz3-0002dj-Ub from Tom_deVries@mentor.com ; Fri, 08 Aug 2014 22:14:25 -0700 Received: from SVR-IES-FEM-02.mgc.mentorg.com ([137.202.0.106]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Fri, 8 Aug 2014 22:14:25 -0700 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-02.mgc.mentorg.com (137.202.0.106) with Microsoft SMTP Server id 14.2.247.3; Sat, 9 Aug 2014 06:14:24 +0100 Message-ID: <53E5AE2D.4010509@mentor.com> Date: Sat, 9 Aug 2014 07:14:21 +0200 From: Tom de Vries User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.0 MIME-Version: 1.0 To: Richard Biener CC: Steven Bosscher , "gcc- >> GCC Patches" , Andrew Pinski Subject: Re: Fix if-conversion pass for dead type-unsafe code References: <53E4B4DC.3030901@mentor.com> <53E4EA03.90806@mentor.com> In-Reply-To: <53E4EA03.90806@mentor.com> On 08-08-14 17:17, Tom de Vries wrote: >> Maybe instead of a new mem_alias_equal_p simply compare MEM_ATTRs >> with mem_attrs_eq_p? > > I propose to fix it this way (as attached) on 4.8/4.9/trunk, and maybe do a more > efficient handling on trunk as a follow-up patch. > > I'll put this through bootstrap/test again. Bootstrapped and reg-tested on trunk x86_64. Re-attached patch OK for trunk, 4.8, 4.9 ? Thanks, - Tom 2014-08-08 Tom de Vries PR rtl-optimization/62004 PR rtl-optimization/62030 * ifcvt.c (rtx_interchangeable_p): New function. (noce_try_move, noce_process_if_block): Use rtx_interchangeable_p. * emit-rtl.c (mem_attrs_eq_p): Remove static. * emit-rtl.h (mem_attrs_eq_p): Declare. * gcc.dg/pr62004.c: New test. * gcc.dg/pr62030.c: Same. * gcc.target/mips/pr62030-octeon.c: Same. diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c index b5278bf..1cac518 100644 --- a/gcc/emit-rtl.c +++ b/gcc/emit-rtl.c @@ -263,7 +263,7 @@ mem_attrs_htab_hash (const void *x) /* Return true if the given memory attributes are equal. */ -static bool +bool mem_attrs_eq_p (const struct mem_attrs *p, const struct mem_attrs *q) { return (p->alias == q->alias diff --git a/gcc/emit-rtl.h b/gcc/emit-rtl.h index 7268090..9ccee07 100644 --- a/gcc/emit-rtl.h +++ b/gcc/emit-rtl.h @@ -69,6 +69,7 @@ extern void set_reg_attrs_for_parm (rtx, rtx); extern void set_reg_attrs_for_decl_rtl (tree t, rtx x); extern void adjust_reg_mode (rtx, enum machine_mode); extern int mem_expr_equal_p (const_tree, const_tree); +extern bool mem_attrs_eq_p (const struct mem_attrs *, const struct mem_attrs *); extern bool need_atomic_barrier_p (enum memmodel, bool); diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index e3353a5..dd6df5b 100644 --- a/gcc/ifcvt.c +++ b/gcc/ifcvt.c @@ -294,6 +294,28 @@ block_fallthru (basic_block bb) return (e) ? e->dest : NULL_BLOCK; } + +/* Return true if RTXs A and B can be safely interchanged. */ + +static bool +rtx_interchangeable_p (const_rtx a, const_rtx b) +{ + if (!rtx_equal_p (a, b)) + return false; + + if (GET_CODE (a) != MEM) + return true; + + /* A dead type-unsafe memory reference is legal, but a live type-unsafe memory + reference is not. Interchanging a dead type-unsafe memory reference with + a live type-safe one creates a live type-unsafe memory reference, in other + words, it makes the program illegal. + We check here conservatively whether the two memory references have equal + memory attributes. */ + + return mem_attrs_eq_p (get_mem_attrs (a), get_mem_attrs (b)); +} + /* Go through a bunch of insns, converting them to conditional execution format if possible. Return TRUE if all of the non-note @@ -1014,6 +1036,9 @@ noce_try_move (struct noce_if_info *if_info) || (rtx_equal_p (if_info->a, XEXP (cond, 1)) && rtx_equal_p (if_info->b, XEXP (cond, 0)))) { + if (!rtx_interchangeable_p (if_info->a, if_info->b)) + return FALSE; + y = (code == EQ) ? if_info->a : if_info->b; /* Avoid generating the move if the source is the destination. */ @@ -2483,7 +2508,7 @@ noce_process_if_block (struct noce_if_info *if_info) if (! insn_b || insn_b != last_active_insn (else_bb, FALSE) || (set_b = single_set (insn_b)) == NULL_RTX - || ! rtx_equal_p (x, SET_DEST (set_b))) + || ! rtx_interchangeable_p (x, SET_DEST (set_b))) return FALSE; } else @@ -2496,7 +2521,7 @@ noce_process_if_block (struct noce_if_info *if_info) || BLOCK_FOR_INSN (insn_b) != BLOCK_FOR_INSN (if_info->cond_earliest) || !NONJUMP_INSN_P (insn_b) || (set_b = single_set (insn_b)) == NULL_RTX - || ! rtx_equal_p (x, SET_DEST (set_b)) + || ! rtx_interchangeable_p (x, SET_DEST (set_b)) || ! noce_operand_ok (SET_SRC (set_b)) || reg_overlap_mentioned_p (x, SET_SRC (set_b)) || modified_between_p (SET_SRC (set_b), insn_b, jump) @@ -2562,7 +2587,7 @@ noce_process_if_block (struct noce_if_info *if_info) /* Look and see if A and B are really the same. Avoid creating silly cmove constructs that no one will fix up later. */ - if (rtx_equal_p (a, b)) + if (rtx_interchangeable_p (a, b)) { /* If we have an INSN_B, we don't have to create any new rtl. Just move the instruction that we already have. If we don't have an diff --git a/gcc/testsuite/gcc.dg/pr62004.c b/gcc/testsuite/gcc.dg/pr62004.c new file mode 100644 index 0000000..c994a41 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr62004.c @@ -0,0 +1,47 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fno-tree-tail-merge" } */ + +struct node +{ + struct node *next; + struct node *prev; +}; + +struct node node; + +struct head +{ + struct node *first; +}; + +struct head heads[5]; + +int k = 2; + +struct head *head = &heads[2]; + +int +main () +{ + struct node *p; + + node.next = (void*)0; + + node.prev = (void *)head; + + head->first = &node; + + struct node *n = head->first; + + struct head *h = &heads[k]; + + heads[2].first = n->next; + + if ((void*)n->prev == (void *)h) + p = h->first; + else + /* Dead tbaa-unsafe load from ((struct node *)&heads[2])->next. */ + p = n->prev->next; + + return !(p == (void*)0); +} diff --git a/gcc/testsuite/gcc.dg/pr62030.c b/gcc/testsuite/gcc.dg/pr62030.c new file mode 100644 index 0000000..b8baf93 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr62030.c @@ -0,0 +1,50 @@ +/* { dg-do run } */ +/* { dg-options "-O2" } */ + +extern void abort (void); + +struct node +{ + struct node *next; + struct node *prev; +}; + +struct node node; + +struct head +{ + struct node *first; +}; + +struct head heads[5]; + +int k = 2; + +struct head *head = &heads[2]; + +static int __attribute__((noinline)) +foo (void) +{ + node.prev = (void *)head; + head->first = &node; + + struct node *n = head->first; + struct head *h = &heads[k]; + struct node *next = n->next; + + if (n->prev == (void *)h) + h->first = next; + else + n->prev->next = next; + + n->next = h->first; + return n->next == &node; +} + +int +main (void) +{ + if (foo ()) + abort (); + return 0; +} diff --git a/gcc/testsuite/gcc.target/mips/pr62030-octeon.c b/gcc/testsuite/gcc.target/mips/pr62030-octeon.c new file mode 100644 index 0000000..5e3d3b3 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/pr62030-octeon.c @@ -0,0 +1,50 @@ +/* { dg-do run } */ +/* { dg-options "-march=octeon" } */ + +extern void abort (void); + +struct node +{ + struct node *next; + struct node *prev; +}; + +struct node node; + +struct head +{ + struct node *first; +}; + +struct head heads[5]; + +int k = 2; + +struct head *head = &heads[2]; + +static int __attribute__((noinline)) +foo (void) +{ + node.prev = (void *)head; + head->first = &node; + + struct node *n = head->first; + struct head *h = &heads[k]; + struct node *next = n->next; + + if (n->prev == (void *)h) + h->first = next; + else + n->prev->next = next; + + n->next = h->first; + return n->next == &node; +} + +int +main (void) +{ + if (foo ()) + abort (); + return 0; +} -- 1.9.1