From patchwork Mon Aug 18 08:33:44 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: 380745 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 DF61A140110 for ; Mon, 18 Aug 2014 18:34:18 +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=YadTWzy3/Cih8Z8+s 0+d+FhplMWnXXDRgMckTtW+BlXcWyzKpEV+9WO3VE8yKHP9MErbIierv8ZWgHAgp s/BEnsA1EKTMxwZ4pBALaaB4FEUspHOITFOqWHeLSqY6AOwZDCYpMc1H1Z6aAkTz hrirS5siI3GlkThsszsben5vgw= 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=FC1FuoeeO7A2sy/Dob6klQX ztPA=; b=qlRw+8rwejyH6GHaydDK5F0t1ji3NiqAIr6Xk6vXlvRtkBYtihPRFXc 9hn7BToGXvCyD2gsEKbE1LC8j2uY5l/K0RR3uuTVF40xejBNEtB64MTdAvdG2oPd pAdNg8dqY5NKgrQLDU4Uulhm7bZg7N9KJm1/vmLGOGQKU6Mvg5BE= Received: (qmail 4105 invoked by alias); 18 Aug 2014 08:33:59 -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 4089 invoked by uid 89); 18 Aug 2014 08:33:57 -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; Mon, 18 Aug 2014 08:33:55 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1XJINy-00043S-W5 from Tom_deVries@mentor.com ; Mon, 18 Aug 2014 01:33:51 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Mon, 18 Aug 2014 01:33:50 -0700 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.2.247.3; Mon, 18 Aug 2014 09:33:48 +0100 Message-ID: <53F1BA68.2030104@mentor.com> Date: Mon, 18 Aug 2014 10:33:44 +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: Richard Biener , 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> <53E5AE2D.4010509@mentor.com> In-Reply-To: On 14-08-14 16:34, Richard Biener wrote: > On Sat, Aug 9, 2014 at 7:14 AM, Tom de Vries wrote: >> 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 ? > > Ok. Backported to 4.8 and 4.9 as attached. The emit-rtl.{c,h} part of my patch was superfluous on trunk given that you already exported mem_attrs_eq_p, something I failed to notice when rebasing. So the backport contains that part of your patch. I've tested the backport patch on 4.9. > (did you check the effect on code-generation? that is, how many > opportunities compiling GCC do we "lose"?) > I haven't done that. I can look into that (after I fix the tail-merge part of pr62004). Thanks, - Tom 2014-08-15 Tom de Vries Backport from mainline: 2014-08-14 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. * gcc.dg/pr62004.c: New test. * gcc.dg/pr62030.c: Same. * gcc.target/mips/pr62030-octeon.c: Same. 2014-08-05 Richard Biener * emit-rtl.h (mem_attrs_eq_p): Declare. * emit-rtl.c (mem_attrs_eq_p): Export. diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c index 4736f8d..89b6768 100644 --- a/gcc/emit-rtl.c +++ b/gcc/emit-rtl.c @@ -245,7 +245,7 @@ const_fixed_htab_eq (const void *x, const void *y) /* 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 fe68de9..36eb0c8 100644 --- a/gcc/emit-rtl.h +++ b/gcc/emit-rtl.h @@ -20,6 +20,9 @@ along with GCC; see the file COPYING3. If not see #ifndef GCC_EMIT_RTL_H #define GCC_EMIT_RTL_H +/* Return whether two MEM_ATTRs are equal. */ +bool mem_attrs_eq_p (const struct mem_attrs *, const struct mem_attrs *); + /* Set the alias set of MEM to SET. */ extern void set_mem_alias_set (rtx, alias_set_type); diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index 0d1adce..49ff85c 100644 --- a/gcc/ifcvt.c +++ b/gcc/ifcvt.c @@ -306,6 +306,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 @@ -1034,6 +1056,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. */ @@ -2504,7 +2529,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 @@ -2517,7 +2542,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) @@ -2583,7 +2608,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