From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18141 invoked by alias); 21 Aug 2015 07:47:09 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 18131 invoked by uid 89); 21 Aug 2015 07:47:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 21 Aug 2015 07:47:07 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 8ED7A14AA4; Fri, 21 Aug 2015 07:47:05 +0000 (UTC) Received: from freie.home (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t7L7kpAn008083 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 21 Aug 2015 03:47:01 -0400 Received: from livre.home (livre.home [172.31.160.2]) by freie.home (8.14.8/8.14.8) with ESMTP id t7L7kD0u019988; Fri, 21 Aug 2015 04:46:13 -0300 From: Alexandre Oliva To: Andreas Schwab Cc: Christophe Lyon , GCC Patches , Patrick Marlier , Jeff Law , James Greenhalgh , "H.J. Lu" , Segher Boessenkool , Richard Biener , David Edelsohn , Eric Botcazou Subject: Re: [PR64164] drop copyrename, integrate into expand References: <20150723203112.GB27818@gate.crashing.org> <20150810082355.GA31149@arm.com> <55C8BFC3.3030603@redhat.com> Date: Fri, 21 Aug 2015 07:57:00 -0000 In-Reply-To: (Alexandre Oliva's message of "Wed, 19 Aug 2015 21:00:49 -0300") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2015-08/txt/msg01276.txt.bz2 On Aug 19, 2015, Alexandre Oliva wrote: > On Aug 19, 2015, Alexandre Oliva wrote: >> I'm having some difficulty getting access to an ia64 box ATM, and for >> ada bootstraps, a cross won't do, so... if you still have that build >> tree around, any chance you could recompile par.o with both stage1 and >> stage2, with -fdump-rtl-expand-details, and email me the compiler dump >> files? > Thanks! > In the mean time, I have been able to duplicate the problem myself. As > you say, it is triggered by -gtoggle. However, it has nothing > whatsoever to do with the recent patches I installed. At most they > expose some latent problem in the scheduler. The above was more likely wrong than right. There may have been a latent problem in the scheduler indeed, but the patch actually made it worse, or even introduced it. The scheduler relies on alias analysis to tell whether a given pair of insns that read or modify memory should have a dependence set between them. It looks both at the RTL proper (including cselib values) and at the MEM attrs. The problem was that, for ada/par.o, we computed different dependencies, and thus different sched priorities, for a pair of insns. Specifically, one wrote to a stack spill slot, and another read from a neighbor spill slot. Both had MEM_EXPRs pointing at a %sfp decl, and different offsets. In the stage3 (-g) compilation, there were debug insns between them. They caused additional equivalent expressions to be added to some values, which in turn caused memrefs_conflict_p to return different values. In the stage2 compilation, both VALUEs resolved to PLUSes of two VALUEs, the first of each resolved to a constant, while the latter of each resolved to %sfp. When the second operands of PLUSes match, we recurse and compare the first operands, resolving both to CONST_INTs and, in this case, concluding that there's no possible overlap. In the stage3 compilation, one VALUE resolved to a PLUS of a VALUE and a CONST_INT, whereas the other resolved to a PLUS of two VALUEs. Without canonicalization of VALUE order in PLUSes, it just so happened that the VALUE that appeared as the second operand in the second PLUS was moved to the first operand in the first PLUS, and so memrefs_conflict_p couldn't tell whether or not there was an overlap. Before the initial pr64164 patch, we had another chance to detect the non-overlap analyzing the MEM attrs in nonoverlapping_memrefs_p: given the same MEM_EXPR, but different offsets, we used to conclude there was no overlap, so this got true_dependence to return the same value in both compilations. The pr64164 patch introduced an early exit from nonoverlapping_memrefs_p when either operand is a gimple_reg, because some of these wouldn't have a DECL_RTL set, and creating RTL for them at such points would not be appropriate. The problem is that the early exit would only return false if the exprs were different. If they were the same, we'd conclude an overlap was possible, even if offsets were enough to tell otherwise. My thought back then was that such exprs were not addressable anyway, so we'd always access the entire object, so offsets couldn't possible be different. Right? Well, no! Think spill slots: %sfp (a gimple_reg decl) + constant offset! Same base gimple_reg, non-overlapping memory addresses! This patch improves memrefs_conflict_p so as to handle more combinations of VALUEs in PLUSes: if both incoming addresses are PLUSes, check one operand of one against the other operand of the other; if one address is a PLUS and the other isn't, test the other against both operands of the PLUS. This causes memrefs_conflict_p to return consistent results for that given pair of insns in both stage2 and stage3 compilation. Additionally, it fixes the regression in nonoverlapping_memrefs_p, adding code to check for non-overlapping offsets when the base expr is the same (as long as offsets and sizes are known for both MEMs). Either one would suffice to fix this particular case. The latter would fix the regression proper, but the former is sufficiently lightweight (since comparing pointers is enough) that it's probably worth adding to get more accurate and consistent results earlier. I'm bootstrapping this on ia64-linux-gnu. Ok to install? fix sched compare regression From: Alexandre Oliva for gcc/ChangeLog PR rtl-optimization/64164 PR rtl-optimization/67227 * alias.c (memrefs_conflict_p): Handle VALUEs in PLUS better. (nonoverlapping_memrefs_p): Test offsets and sizes when given identical gimple_reg exprs. --- gcc/alias.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/gcc/alias.c b/gcc/alias.c index 4681e3f..f12d9d1 100644 --- a/gcc/alias.c +++ b/gcc/alias.c @@ -2228,6 +2228,13 @@ memrefs_conflict_p (int xsize, rtx x, int ysize, rtx y, HOST_WIDE_INT c) rtx x0 = XEXP (x, 0); rtx x1 = XEXP (x, 1); + /* However, VALUEs might end up in different positions even in + canonical PLUSes. Comparing their addresses is enough. */ + if (x0 == y) + return memrefs_conflict_p (xsize, x1, ysize, const0_rtx, c); + else if (x1 == y) + return memrefs_conflict_p (xsize, x0, ysize, const0_rtx, c); + if (GET_CODE (y) == PLUS) { /* The fact that Y is canonicalized means that this @@ -2235,6 +2242,11 @@ memrefs_conflict_p (int xsize, rtx x, int ysize, rtx y, HOST_WIDE_INT c) rtx y0 = XEXP (y, 0); rtx y1 = XEXP (y, 1); + if (x0 == y1) + return memrefs_conflict_p (xsize, x1, ysize, y0, c); + if (x1 == y0) + return memrefs_conflict_p (xsize, x0, ysize, y1, c); + if (rtx_equal_for_memref_p (x1, y1)) return memrefs_conflict_p (xsize, x0, ysize, y0, c); if (rtx_equal_for_memref_p (x0, y0)) @@ -2263,6 +2275,11 @@ memrefs_conflict_p (int xsize, rtx x, int ysize, rtx y, HOST_WIDE_INT c) rtx y0 = XEXP (y, 0); rtx y1 = XEXP (y, 1); + if (x == y0) + return memrefs_conflict_p (xsize, const0_rtx, ysize, y1, c); + if (x == y1) + return memrefs_conflict_p (xsize, const0_rtx, ysize, y0, c); + if (CONST_INT_P (y1)) return memrefs_conflict_p (xsize, x, ysize, y0, c + INTVAL (y1)); else @@ -2518,7 +2535,11 @@ nonoverlapping_memrefs_p (const_rtx x, const_rtx y, bool loop_invariant) able to do anything about them since no SSA information will have remained to guide it. */ if (is_gimple_reg (exprx) || is_gimple_reg (expry)) - return exprx != expry; + return exprx != expry + || (moffsetx_known_p && moffsety_known_p + && MEM_SIZE_KNOWN_P (x) && MEM_SIZE_KNOWN_P (y) + && !offset_overlap_p (moffsety - moffsetx, + MEM_SIZE (x), MEM_SIZE (y))); /* With invalid code we can end up storing into the constant pool. Bail out to avoid ICEing when creating RTL for this. -- Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer