public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Andrew Pinski <pinskia@gmail.com>
Cc: Michael Collison <michael.collison@linaro.org>,
	       GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
Date: Thu, 25 Sep 2014 10:12:00 -0000	[thread overview]
Message-ID: <20140925101234.GA14286@gate.crashing.org> (raw)
In-Reply-To: <CA+=Sn1ko=eHfibkRe1xoWYTNp5E3_DjWgL3hMK5Dsx94ayJhjQ@mail.gmail.com>

On Wed, Sep 24, 2014 at 09:17:23PM -0700, Andrew Pinski wrote:
> On Wed, Sep 24, 2014 at 9:13 PM, Michael Collison
> <michael.collison@linaro.org> wrote:
> >
> > I have that attached to the bug report at the URL provided. I will work on a
> > testcase if you think it is warranted.
> 
> Yes it is almost always warranted.
> 
> https://gcc.gnu.org/contribute.html#patches
> 
> Testcases   If you cannot follow the recommendations of the GCC coding
> conventions about testcases, you should include a justification for
> why adequate testcases cannot be added.
> 
> See the last part of that sentence.  You don't have any justification
> on why you are not including testcases.

It is very hard to make a reliable testcase for such problems, because
they only happen when register allocation is under pressure.

The problem is not that "n" allows more than your predicate does.  The
predicate allows registers too, so the compiler happily made a register
contain some big const.  Now RA comes along, is out of registers but hey,
there is this "n", let's just put the big constant there!  Carnage.

So this is hard to test for; you can add some (big) code that exposed the
problem, but in a few months time that won't trigger the problem anymore
because earlier stages in the compiler will have generated slightly
different code.

It also does nothing to catch similar problems in other patterns.


Segher

  reply	other threads:[~2014-09-25 10:12 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-25  3:45 Michael Collison
2014-09-25  4:01 ` Andrew Pinski
2014-09-25  4:08   ` Michael Collison
2014-09-25  4:10     ` Andrew Pinski
2014-09-25  4:13       ` Michael Collison
2014-09-25  4:17         ` Andrew Pinski
2014-09-25 10:12           ` Segher Boessenkool [this message]
2014-09-25 17:33             ` Michael Collison
2014-09-25 19:30               ` Segher Boessenkool
2014-10-09 12:28                 ` Christophe Lyon
2014-11-04 10:44 ` Marcus Shawcroft
2015-05-08 10:42   ` Richard Biener
2015-06-15 12:20     ` Christophe Lyon
2015-06-16  8:36       ` Christophe Lyon
2015-06-16 10:31         ` Christophe Lyon

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=20140925101234.GA14286@gate.crashing.org \
    --to=segher@kernel.crashing.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=michael.collison@linaro.org \
    --cc=pinskia@gmail.com \
    /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).