From: Richard Biener <richard.guenther@gmail.com>
To: Alexandre Oliva <aoliva@redhat.com>
Cc: Andreas Schwab <schwab@linux-m68k.org>,
Christophe Lyon <christophe.lyon@linaro.org>,
GCC Patches <gcc-patches@gcc.gnu.org>,
Patrick Marlier <patrick.marlier@gmail.com>,
Jeff Law <law@redhat.com>,
James Greenhalgh <james.greenhalgh@arm.com>,
"H.J. Lu" <hjl.tools@gmail.com>,
Segher Boessenkool <segher@kernel.crashing.org>,
David Edelsohn <dje.gcc@gmail.com>,
Eric Botcazou <ebotcazou@adacore.com>
Subject: Re: [PR64164] drop copyrename, integrate into expand
Date: Fri, 21 Aug 2015 08:38:00 -0000 [thread overview]
Message-ID: <CAFiYyc085cT_qudWTdWv3wk6z2No-O5+fVOmq7fsa2Nbqc=4OA@mail.gmail.com> (raw)
In-Reply-To: <orio89xewr.fsf@livre.home>
On Fri, Aug 21, 2015 at 9:46 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Aug 19, 2015, Alexandre Oliva <aoliva@redhat.com> wrote:
>
>> On Aug 19, 2015, Alexandre Oliva <aoliva@redhat.com> 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 <aoliva@redhat.com>
>
> 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
next prev parent reply other threads:[~2015-08-21 8:37 UTC|newest]
Thread overview: 127+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-27 18:04 Alexandre Oliva
2015-03-27 18:11 ` Alexandre Oliva
2015-03-28 19:22 ` Alexandre Oliva
2015-03-31 5:11 ` Jeff Law
2015-04-03 13:17 ` Alexandre Oliva
2015-04-06 16:08 ` Jeff Law
2015-04-24 1:56 ` Alexandre Oliva
2015-04-27 11:39 ` Richard Biener
2015-06-06 5:12 ` Alexandre Oliva
2015-06-08 8:16 ` Richard Biener
2015-06-09 8:58 ` Christophe Lyon
2015-06-10 0:28 ` Alexandre Oliva
2015-06-10 13:36 ` Richard Biener
2015-07-16 7:58 ` Alexandre Oliva
2015-07-16 8:50 ` Richard Biener
2015-07-16 21:33 ` Alexandre Oliva
2015-07-18 8:26 ` Alexandre Oliva
2015-07-21 13:25 ` Richard Biener
2015-07-22 17:13 ` Alexandre Oliva
2015-07-22 17:43 ` Alexandre Oliva
2015-07-23 11:04 ` Richard Biener
2015-07-23 15:42 ` Alexandre Oliva
2015-07-23 20:35 ` Segher Boessenkool
2015-07-23 21:24 ` H.J. Lu
2015-07-23 22:11 ` H.J. Lu
2015-07-24 1:31 ` David Edelsohn
2015-07-24 5:08 ` H.J. Lu
2015-07-24 9:26 ` Richard Biener
2015-07-24 12:50 ` H.J. Lu
2015-07-24 20:20 ` Alexandre Oliva
2015-07-25 2:37 ` David Edelsohn
2015-07-27 22:16 ` Alexandre Oliva
2015-07-27 22:31 ` H.J. Lu
2015-07-24 18:51 ` Alexandre Oliva
2015-07-24 19:12 ` H.J. Lu
2015-07-24 19:31 ` David Edelsohn
2015-07-24 20:43 ` Alexandre Oliva
2015-07-24 20:47 ` Alexandre Oliva
2015-07-24 21:53 ` H.J. Lu
2015-07-25 7:17 ` Richard Biener
2015-07-29 20:52 ` Alexandre Oliva
2015-07-29 21:06 ` H.J. Lu
2015-07-30 17:47 ` H.J. Lu
2015-08-03 23:46 ` Alexandre Oliva
2015-08-04 9:48 ` Richard Biener
2015-08-05 0:39 ` Alexandre Oliva
2015-08-05 9:14 ` Richard Biener
2015-08-05 23:03 ` Alexandre Oliva
2015-08-10 8:24 ` James Greenhalgh
2015-08-10 15:14 ` Jeff Law
2015-08-11 4:53 ` Patrick Marlier
2015-08-14 19:03 ` Alexandre Oliva
2015-08-15 8:57 ` Andreas Schwab
2015-08-16 13:00 ` Alexandre Oliva
[not found] ` <m2k2sv8s21.fsf@linux-m68k.org>
2015-08-17 5:05 ` Alexandre Oliva
2015-08-17 9:29 ` Kyrill Tkachov
2015-08-17 16:23 ` Andrew Pinski
2015-08-18 16:18 ` Kyrill Tkachov
2015-08-16 16:42 ` Andreas Schwab
2015-08-17 2:57 ` Alexandre Oliva
2015-08-17 8:23 ` Andreas Schwab
2015-08-17 9:21 ` Andreas Schwab
2015-08-17 11:58 ` Alexandre Oliva
2015-08-17 7:48 ` Christophe Lyon
2015-08-17 12:43 ` Alexandre Oliva
2015-08-17 13:39 ` Christophe Lyon
2015-08-18 6:53 ` Alexandre Oliva
2015-08-19 6:50 ` Alexandre Oliva
2015-08-19 10:17 ` Richard Biener
2015-08-19 13:35 ` Andreas Schwab
2015-08-19 13:45 ` Andreas Schwab
2015-08-19 17:48 ` Alexandre Oliva
2015-08-20 1:44 ` Alexandre Oliva
2015-08-20 17:03 ` Jeff Law
2015-08-21 7:57 ` Alexandre Oliva
2015-08-21 8:38 ` Richard Biener [this message]
2015-08-21 12:17 ` Andreas Schwab
2015-08-21 8:11 ` Alexandre Oliva
2015-08-21 8:37 ` Richard Biener
2015-09-02 17:09 ` Alan Lawrence
2015-09-02 22:34 ` Alexandre Oliva
2015-09-03 10:58 ` Alan Lawrence
2015-09-18 15:49 ` Alan Lawrence
2015-09-23 20:44 ` Alexandre Oliva
2015-09-25 11:39 ` Richard Biener
2015-10-09 5:26 ` [PR67828] don't unswitch loops on undefined SSA values (was: Re: [PR64164] drop copyrename, integrate into expand) Alexandre Oliva
2015-10-09 9:35 ` Richard Biener
2015-10-09 5:36 ` [PR67766] reorder return value copying from PARALLELs and CONCATs " Alexandre Oliva
2015-10-09 7:33 ` [PR67891] drop is_gimple_reg test from set_parm_rtl (was: [PR67766] reorder return value copying from PARALLELs and CONCATs) Alexandre Oliva
2015-10-09 9:40 ` Richard Biener
2015-10-10 13:20 ` [PR67891] drop is_gimple_reg test from set_parm_rtl Alexandre Oliva
2015-10-12 10:22 ` Richard Biener
2015-10-14 3:25 ` Alexandre Oliva
2015-10-14 9:28 ` Richard Biener
2015-11-03 1:11 ` Alexandre Oliva
2015-11-03 3:14 ` Jeff Law
2015-11-03 4:29 ` Alexandre Oliva
2022-10-17 12:08 ` Tag 'gcc/gimple-expr.cc:mark_addressable_2' as 'static' (was: [PR67891] drop is_gimple_reg test from set_parm_rtl) Thomas Schwinge
2015-10-09 9:36 ` [PR67766] reorder return value copying from PARALLELs and CONCATs (was: Re: [PR64164] drop copyrename, integrate into expand) Richard Biener
2015-09-29 11:31 ` [PR64164] drop copyrename, integrate into expand Szabolcs Nagy
2015-10-07 22:37 ` Alexandre Oliva
2015-10-08 10:00 ` Richard Biener
2015-10-09 21:10 ` Jeff Law
2015-11-05 5:09 ` Alexandre Oliva
2015-11-05 13:44 ` Richard Biener
2015-11-10 15:31 ` Alan Lawrence
2015-11-10 22:59 ` Alexandre Oliva
2015-11-10 23:43 ` Jeff Law
2015-11-11 18:10 ` Alexandre Oliva
2015-11-13 6:33 ` Jeff Law
2015-11-17 0:07 ` Alexandre Oliva
2015-11-24 5:41 ` Jeff Law
2015-07-24 18:21 ` Alexandre Oliva
2015-07-29 20:32 ` Alexandre Oliva
2015-04-29 3:51 ` Jeff Law
2015-03-31 6:55 ` Steven Bosscher
2015-03-31 13:30 ` Richard Biener
2015-03-31 14:06 ` Richard Biener
2015-04-03 13:30 ` Alexandre Oliva
2015-04-06 15:57 ` Jeff Law
2015-12-04 12:45 ` Dominik Vogt
2015-06-09 16:19 David Edelsohn
2015-06-09 18:36 ` Alexandre Oliva
2015-06-09 20:24 ` Alexandre Oliva
2015-06-09 20:59 ` Jakub Jelinek
2015-06-09 21:36 ` Eric Botcazou
2015-06-09 21:38 ` David Edelsohn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFiYyc085cT_qudWTdWv3wk6z2No-O5+fVOmq7fsa2Nbqc=4OA@mail.gmail.com' \
--to=richard.guenther@gmail.com \
--cc=aoliva@redhat.com \
--cc=christophe.lyon@linaro.org \
--cc=dje.gcc@gmail.com \
--cc=ebotcazou@adacore.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=hjl.tools@gmail.com \
--cc=james.greenhalgh@arm.com \
--cc=law@redhat.com \
--cc=patrick.marlier@gmail.com \
--cc=schwab@linux-m68k.org \
--cc=segher@kernel.crashing.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).