public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Patrick Palka <patrick@parcs.ath.cx>
To: Jeff Law <law@redhat.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] Turn some compile-time tests into run-time tests
Date: Thu, 28 Apr 2016 14:03:00 -0000	[thread overview]
Message-ID: <CA+C-WL9u0EZ7S6qHN_QEiWKc1G7gOXu=HWCyOrrmifK9AF39EA@mail.gmail.com> (raw)
In-Reply-To: <75b51453-cac0-0d8f-2bfa-9a0cabca6e9b@redhat.com>

On Wed, Apr 27, 2016 at 5:36 PM, Jeff Law <law@redhat.com> wrote:
> On 03/10/2016 04:38 PM, Patrick Palka wrote:
>>
>> I ran the command
>>
>>   git grep -l "dg-do compile" | xargs grep -l __builtin_abort | xargs grep
>> -lw main
>>
>> to find tests marked as compile-time tests that likely ought to instead
>> be marked as run-time tests, by the rationale that they use
>> __builtin_abort and they also define main().  (I also then confirmed that
>> they
>> compile, link and run cleanly on my machine.)
>>
>> After this patch, the remaining test files reported by the above command
>> are:
>>
>>   These do not define all the functions they use:
>>     gcc/testsuite/g++.dg/ipa/devirt-41.C
>>     gcc/testsuite/g++.dg/ipa/devirt-44.C
>>     gcc/testsuite/g++.dg/ipa/devirt-45.C
>>     gcc/testsuite/gcc.target/i386/pr55672.c
>>
>>   These are non-x86 tests so I can't confirm that they run cleanly:
>>     gcc/testsuite/gcc.target/arm/pr58041.c
>>     gcc/testsuite/gcc.target/powerpc/pr35907.c
>>     gcc/testsuite/gcc.target/s390/dwarfregtable-1.c
>>     gcc/testsuite/gcc.target/s390/dwarfregtable-2.c
>>     gcc/testsuite/gcc.target/s390/dwarfregtable-3.c
>>
>>   These use dg-error:
>>     libstdc++-v3/testsuite/20_util/forward/c_neg.cc
>>     libstdc++-v3/testsuite/20_util/forward/f_neg.cc
>>
>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to
>> commit?  Does anyone have another heuristic one can use to help find
>> these kinds of typos?
>>
>> gcc/testsuite/ChangeLog:
>>
>>         * g++.dg/cpp0x/constexpr-aggr2.C: Make it a run-time test.
>>         * g++.dg/cpp0x/nullptr32.C: Likewise.
>>         * g++.dg/cpp1y/digit-sep-cxx11-neg.C: Likewise.
>>         * g++.dg/cpp1y/digit-sep.C: Likewise.
>>         * g++.dg/ext/flexary13.C: Likewise.
>>         * gcc.dg/alias-14.c: Likewise.
>>         * gcc.dg/ipa/PR65282.c: Likewise.
>>         * gcc.dg/pr69644.c: Likewise.
>>         * gcc.dg/tree-ssa/pr38533.c: Likewise.
>>         * gcc.dg/tree-ssa/pr61385.c: Likewise.
>
> My worry with the 38533 test is that while the ASM defines "f" from the
> standpoint of dataflow, it does not actually emit any code to ensure "f" is
> actually defined.  This could lead to spurious aborts due to use of an
> uninitialized value at runtime.  Similarly for alias-14.c
>
> I'd be worried that we don't necessarily have sync_bool_compare_and_swap on
> all targets for 69644.

Ah yeah, good points..

>
> flexary13.C probably won't link on a cross target unless the cross libraries
> are available.  But that's probably OK.
>
> The rest seem OK to me.  Note that I'm not convinced all these tests were
> designed to be execution tests, even though they use __builtin_abort and
> friends.  Though it's a good marker of something that can/should be looked
> at.

True..  What made me look into this in the first place is that I
caught myself making a similar mistake, i.e. marking an execution test
case as dg-do compile instead of dg-do run out of habit.  But I
suppose it's worth looking at the context of each of these tests to
see if they were not actually intended to be execution tests.  I'll
double check this and report back; in the meantime I also found some
more tests that ought to be looked at.

>
>
> jeff
>

  reply	other threads:[~2016-04-28 14:03 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-10 23:39 Patrick Palka
2016-03-10 23:54 ` Patrick Palka
2016-03-11 16:39 ` Patrick Palka
2016-04-27 21:26   ` Jeff Law
2016-04-27 21:37 ` Jeff Law
2016-04-28 14:03   ` Patrick Palka [this message]
2016-04-28 14:59     ` Jeff Law

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='CA+C-WL9u0EZ7S6qHN_QEiWKc1G7gOXu=HWCyOrrmifK9AF39EA@mail.gmail.com' \
    --to=patrick@parcs.ath.cx \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=law@redhat.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).