From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 104804 invoked by alias); 21 Aug 2015 08:37:44 -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 104790 invoked by uid 89); 21 Aug 2015 08:37:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.6 required=5.0 tests=AWL,BAYES_50,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ig0-f173.google.com Received: from mail-ig0-f173.google.com (HELO mail-ig0-f173.google.com) (209.85.213.173) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 21 Aug 2015 08:37:41 +0000 Received: by igbjg10 with SMTP id jg10so9294074igb.0 for ; Fri, 21 Aug 2015 01:37:39 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.50.87.74 with SMTP id v10mr1727491igz.37.1440146259507; Fri, 21 Aug 2015 01:37:39 -0700 (PDT) Received: by 10.107.32.140 with HTTP; Fri, 21 Aug 2015 01:37:39 -0700 (PDT) In-Reply-To: References: <20150723203112.GB27818@gate.crashing.org> <20150810082355.GA31149@arm.com> <55C8BFC3.3030603@redhat.com> Date: Fri, 21 Aug 2015 08:38:00 -0000 Message-ID: Subject: Re: [PR64164] drop copyrename, integrate into expand From: Richard Biener To: Alexandre Oliva Cc: Andreas Schwab , Christophe Lyon , GCC Patches , Patrick Marlier , Jeff Law , James Greenhalgh , "H.J. Lu" , Segher Boessenkool , David Edelsohn , Eric Botcazou Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg01282.txt.bz2 On Fri, Aug 21, 2015 at 9:46 AM, Alexandre Oliva wrote: > 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? Looks ok to me. Thanks, Richard. > > 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