From patchwork Tue Nov 23 23:14:47 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 72776 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]) by ozlabs.org (Postfix) with SMTP id 24E75B6EF2 for ; Wed, 24 Nov 2010 10:15:16 +1100 (EST) Received: (qmail 29631 invoked by alias); 23 Nov 2010 23:15:13 -0000 Received: (qmail 29619 invoked by uid 22791); 23 Nov 2010 23:15:11 -0000 X-SWARE-Spam-Status: No, hits=-6.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 23 Nov 2010 23:14:50 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id oANNEn12007396 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 23 Nov 2010 18:14:49 -0500 Received: from tyan-ft48-01.lab.bos.redhat.com (tyan-ft48-01.lab.bos.redhat.com [10.16.42.4]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id oANNEmhp028096 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 23 Nov 2010 18:14:49 -0500 Received: from tyan-ft48-01.lab.bos.redhat.com (localhost.localdomain [127.0.0.1]) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4) with ESMTP id oANNEm3B010240; Wed, 24 Nov 2010 00:14:48 +0100 Received: (from jakub@localhost) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4/Submit) id oANNElf0010239; Wed, 24 Nov 2010 00:14:47 +0100 Date: Wed, 24 Nov 2010 00:14:47 +0100 From: Jakub Jelinek To: Vladimir Makarov Cc: Jeff Law , gcc-patches@gcc.gnu.org Subject: [PATCH] sched-deps.c deps->last_pending_memory_flush fix (PR rtl-optimization/46614, take 2) Message-ID: <20101123231447.GW29412@tyan-ft48-01.lab.bos.redhat.com> Reply-To: Jakub Jelinek References: <20101123172211.GT29412@tyan-ft48-01.lab.bos.redhat.com> <4CEC0DF1.6070402@redhat.com> <20101123223901.GU29412@tyan-ft48-01.lab.bos.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20101123223901.GU29412@tyan-ft48-01.lab.bos.redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes 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 Hi! On Tue, Nov 23, 2010 at 11:39:01PM +0100, Jakub Jelinek wrote: > On Tue, Nov 23, 2010 at 01:54:41PM -0500, Vladimir Makarov wrote: > > On 11/23/2010 12:22 PM, Jakub Jelinek wrote: > > >Fixed by instead remembering if a JUMP_INSN stands only for itself > > >and nothing else in the deps->last_pending_memory_flush list > > >(such jumps have REG_NOTE_KIND set to REG_DEP_ANTI, could be > > >anything but REG_DEP_TRUE which is the default and used for > > >additions to the list from flush_pending_lists) and using REG_NOTE_KIND > > >instead of JUMP_P checks in the two places which care about it. > > > > > >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > > > > It looks ok for me, Jakub. Although it would be nice to add a > > comment about meaning REG_DEP_ANTI for element of list > > last_pending_memory_flush when you set it up in function > > deps_analyze_insn. > > Maybe also define macros? > > /* In deps->last_pending_memory_flush marks JUMP_INSNs that weren't > added to the list because of flush_pending_lists, stands just > for itself and not for any other pending memory reads/writes. */ > #define NON_FLUSH_JUMP_KIND REG_DEP_ANTI > #define NON_FLUSH_JUMP_P(x) (REG_NOTE_KIND (x) == REG_DEP_ANTI) Here is updated patch, ok if it passes bootstrap/regtest? 2010-11-23 Jakub Jelinek PR rtl-optimization/46614 * sched-deps.c (NON_FLUSH_JUMP_KIND, NON_FLUSH_JUMP_P): Define. (deps_analyze_insn): Mark JUMP_INSNs in last_pending_memory_flush that weren't added through flush_pending_lists with NON_FLUSH_JUMP_KIND. (sched_analyze_2, sched_analyze_insn): Check NON_FLUSH_JUMP_P on INSN_LIST instead of JUMP_P check on its operand. * sched-rgn.c (concat_INSN_LIST): Copy over REG_NOTE_KIND. * gcc.dg/pr46614.c: New test. Jakub --- gcc/sched-deps.c.jj 2010-10-14 21:25:16.783248849 +0200 +++ gcc/sched-deps.c 2010-11-24 00:11:20.177510963 +0100 @@ -53,6 +53,12 @@ along with GCC; see the file COPYING3. #define CHECK (false) #endif +/* In deps->last_pending_memory_flush marks JUMP_INSNs that weren't + added to the list because of flush_pending_lists, stands just + for itself and not for any other pending memory reads/writes. */ +#define NON_FLUSH_JUMP_KIND REG_DEP_ANTI +#define NON_FLUSH_JUMP_P(x) (REG_NOTE_KIND (x) == NON_FLUSH_JUMP_KIND) + /* Holds current parameters for the dependency analyzer. */ struct sched_deps_info_def *sched_deps_info; @@ -2484,7 +2490,7 @@ sched_analyze_2 (struct deps_desc *deps, for (u = deps->last_pending_memory_flush; u; u = XEXP (u, 1)) { - if (! JUMP_P (XEXP (u, 0))) + if (! NON_FLUSH_JUMP_P (u)) add_dependence (insn, XEXP (u, 0), REG_DEP_ANTI); else if (deps_may_trap_p (x)) { @@ -2796,8 +2802,7 @@ sched_analyze_insn (struct deps_desc *de REG_DEP_ANTI); for (u = deps->last_pending_memory_flush; u; u = XEXP (u, 1)) - if (! JUMP_P (XEXP (u, 0)) - || !sel_sched_p ()) + if (! NON_FLUSH_JUMP_P (u) || !sel_sched_p ()) add_dependence (insn, XEXP (u, 0), REG_DEP_ANTI); EXECUTE_IF_SET_IN_REG_SET (reg_pending_uses, 0, i, rsi) @@ -3242,8 +3247,15 @@ deps_analyze_insn (struct deps_desc *dep if (deps->pending_flush_length++ > MAX_PENDING_LIST_LENGTH) flush_pending_lists (deps, insn, true, true); else - deps->last_pending_memory_flush - = alloc_INSN_LIST (insn, deps->last_pending_memory_flush); + { + deps->last_pending_memory_flush + = alloc_INSN_LIST (insn, deps->last_pending_memory_flush); + /* Signal to sched_analyze_insn that this jump stands + just for its own, not any other pending memory + reads/writes flush_pending_lists had to flush. */ + PUT_REG_NOTE_KIND (deps->last_pending_memory_flush, + NON_FLUSH_JUMP_KIND); + } } sched_analyze_insn (deps, PATTERN (insn), insn); --- gcc/sched-rgn.c.jj 2010-10-26 05:42:51.487398449 +0200 +++ gcc/sched-rgn.c 2010-11-23 16:17:21.730405086 +0100 @@ -2574,7 +2574,10 @@ concat_INSN_LIST (rtx copy, rtx old) { rtx new_rtx = old; for (; copy ; copy = XEXP (copy, 1)) - new_rtx = alloc_INSN_LIST (XEXP (copy, 0), new_rtx); + { + new_rtx = alloc_INSN_LIST (XEXP (copy, 0), new_rtx); + PUT_REG_NOTE_KIND (new_rtx, REG_NOTE_KIND (copy)); + } return new_rtx; } --- gcc/testsuite/gcc.dg/pr46614.c.jj 2010-11-23 16:17:21.731301502 +0100 +++ gcc/testsuite/gcc.dg/pr46614.c 2010-11-23 16:17:21.731301502 +0100 @@ -0,0 +1,56 @@ +/* PR rtl-optimization/46614 */ +/* { dg-do run } */ +/* { dg-options "-O -fno-rename-registers -fsched2-use-superblocks -fschedule-insns2 -funroll-loops" } */ + +extern void abort (void); + +struct S +{ + unsigned char a; + unsigned char b; + unsigned int c; + unsigned int e; + unsigned char f; + unsigned int g; +}; + +void bar (struct S *x) +{ + int i; + struct S *p = x; + struct S r[16]; + unsigned j; + for (i = 0; i < 16; i++) + { + r[i].c = p->b + p->c; + j = p->c + p->f; + r[i].a = j + p->b; + r[i].f = p->f + p->e; + r[i].g = p->b + p->c; + } + for (i = 0; i < 16; i++) + { + if (r[i].c != x[i].b + x[i].c + || r[i].a != x[i].c + x[i].f + x[i].b + || r[i].f != x[i].f + x[i].e + || r[i].g != x[i].b + x[i].c) + abort (); + } + for (i = 0; i < 16; i++) + { + r[i].b = p->c; + if (r[i].b != x[i].c) + abort (); + } +} + +int +main () +{ + int i; + struct S x[16]; + for (i = 0; i < 16; i++) + x[i].b = x[i].c = x[i].e = x[i].f = 5; + bar (x); + return 0; +}