diff mbox

Ping: Re: [PATCH 1/2] gcc: Remove unneeded global flag.

Message ID 20161119215926.GX5975@embecosm.com
State New
Headers show

Commit Message

Andrew Burgess Nov. 19, 2016, 9:59 p.m. UTC
* Christophe Lyon <christophe.lyon@linaro.org> [2016-11-18 13:21:50 +0100]:

> On 16 November 2016 at 23:12, Andrew Burgess
> <andrew.burgess@embecosm.com> wrote:
> > * Mike Stump <mikestump@comcast.net> [2016-11-16 12:59:53 -0800]:
> >
> >> On Nov 16, 2016, at 12:09 PM, Andrew Burgess <andrew.burgess@embecosm.com> wrote:
> >> > My only remaining concern is the new tests, I've tried to restrict
> >> > them to targets that I suspect they'll pass on with:
> >> >
> >> >    /* { dg-final-use { scan-assembler "\.section\[\t \]*\.text\.unlikely\[\\n\\r\]+\[\t \]*\.size\[\t \]*foo\.cold\.0" { target *-*-linux* *-*-gnu* } } } */
> >> >
> >> > but I'm still nervous that I'm going to introduce test failures.  Is
> >> > there any advice / guidance I should follow before I commit, or are
> >> > folk pretty relaxed so long as I've made a reasonable effort?
> >>
> >> So, if you are worried about the way the line is constructed, I usually test it by misspelling the *-*-linux* *-*-gnu* part as *-*-linNOTux* *-*-gnNOTu* and see if the test then doesn't run on your machine.  If it doesn't then you can be pretty confident that only machines that match the target triplet can be impacted.  I usually do this type of testing by running the test case in isolation (not the full tests suite).  Anyway, do the best you can, and don't worry about t it too much, learn from the experience, even if it goes wrong in some way.  If it did go wrong, just be responsive (don't check it in just before a 6 week vacation) about fixing it, if you can.
> >>
> >
> > Thanks for the feedback.
> >
> > Change committed as revision 242519.  If anyone sees any issues just
> > let me know.
> >
> 
> Hi Andrew,
> 
> Sorry for the delay, there are so many commits these days, with so
> many regression
> reports to check manually before reporting here....
> 
> So, your new test fails on arm* targets:

After a little digging I think the problem might be that
-freorder-blocks-and-partition is not supported on arm.

This should be detected as the new tests include:

    /* { dg-require-effective-target freorder } */

however this test passed on arm as -freorder-blocks-and-partition does
not issue any warning unless -fprofile-use is also passed.

The patch below extends check_effective_target_freorder to check using
-fprofile-use.  With this change in place the tests are skipped on
arm.

All feedback welcome,

Thanks,
Andrew

---

arm/gcc: Tighten checks in check_effective_target_freorder

In check_effective_target_freorder we check to see if the target
 supports -freorder-blocks-and-partition.  However we disable
 -freorder-blocks-and-partition when -fprofile-use is not supplied so
 for some targets we'll not see any message about lack of support for
 -freorder-blocks-and-partition unless -fprofile-use is also passed.

This commit extends check_effective_target_freorder to first try
-freorder-blocks-and-partition on its own, then try -fprofile-use and
-freorder-blocks-and-partition.

gcc/testsuite/ChangeLog:

	* lib/target-supports.exp (check_effective_target_freorder): Check
	additional case.
---
 gcc/testsuite/ChangeLog               | 5 +++++
 gcc/testsuite/lib/target-supports.exp | 8 +++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

Mike Stump Nov. 20, 2016, 5:27 p.m. UTC | #1
On Nov 19, 2016, at 1:59 PM, Andrew Burgess <andrew.burgess@embecosm.com> wrote:
>> So, your new test fails on arm* targets:
> 
> After a little digging I think the problem might be that
> -freorder-blocks-and-partition is not supported on arm.
> 
> This should be detected as the new tests include:
> 
>    /* { dg-require-effective-target freorder } */
> 
> however this test passed on arm as -freorder-blocks-and-partition does
> not issue any warning unless -fprofile-use is also passed.
> 
> The patch below extends check_effective_target_freorder to check using
> -fprofile-use.  With this change in place the tests are skipped on
> arm.

> All feedback welcome,

Seems reasonable, unless a -freorder-blocks-and-partition/-fprofile-use person thinks this is the wrong solution.
Christophe Lyon Nov. 21, 2016, 12:47 p.m. UTC | #2
On 20 November 2016 at 18:27, Mike Stump <mikestump@comcast.net> wrote:
> On Nov 19, 2016, at 1:59 PM, Andrew Burgess <andrew.burgess@embecosm.com> wrote:
>>> So, your new test fails on arm* targets:
>>
>> After a little digging I think the problem might be that
>> -freorder-blocks-and-partition is not supported on arm.
>>
>> This should be detected as the new tests include:
>>
>>    /* { dg-require-effective-target freorder } */
>>
>> however this test passed on arm as -freorder-blocks-and-partition does
>> not issue any warning unless -fprofile-use is also passed.
>>
>> The patch below extends check_effective_target_freorder to check using
>> -fprofile-use.  With this change in place the tests are skipped on
>> arm.
>
>> All feedback welcome,
>
> Seems reasonable, unless a -freorder-blocks-and-partition/-fprofile-use person thinks this is the wrong solution.
>

Hi,

As promised, I tested this patch: it makes
gcc.dg/tree-prof/section-attr-[123].c
unsupported on arm*, and thus they are not failing anymore :-)

However, it also makes other tests unsupported, while they used to pass:

  gcc.dg/pr33648.c
  gcc.dg/pr46685.c
  gcc.dg/tree-prof/20041218-1.c
  gcc.dg/tree-prof/bb-reorg.c
  gcc.dg/tree-prof/cold_partition_label.c
  gcc.dg/tree-prof/comp-goto-1.c
  gcc.dg/tree-prof/pr34999.c
  gcc.dg/tree-prof/pr45354.c
  gcc.dg/tree-prof/pr50907.c
  gcc.dg/tree-prof/pr52027.c
  gcc.dg/tree-prof/va-arg-pack-1.c

and failures are now unsupported:
  gcc.dg/tree-prof/cold_partition_label.c
  gcc.dg/tree-prof/section-attr-1.c
  gcc.dg/tree-prof/section-attr-2.c
  gcc.dg/tree-prof/section-attr-3.c

So, maybe this patch is too strong?

Christophe
Andrew Burgess Nov. 24, 2016, 9:40 p.m. UTC | #3
* Christophe Lyon <christophe.lyon@linaro.org> [2016-11-21 13:47:09 +0100]:

> On 20 November 2016 at 18:27, Mike Stump <mikestump@comcast.net> wrote:
> > On Nov 19, 2016, at 1:59 PM, Andrew Burgess <andrew.burgess@embecosm.com> wrote:
> >>> So, your new test fails on arm* targets:
> >>
> >> After a little digging I think the problem might be that
> >> -freorder-blocks-and-partition is not supported on arm.
> >>
> >> This should be detected as the new tests include:
> >>
> >>    /* { dg-require-effective-target freorder } */
> >>
> >> however this test passed on arm as -freorder-blocks-and-partition does
> >> not issue any warning unless -fprofile-use is also passed.
> >>
> >> The patch below extends check_effective_target_freorder to check using
> >> -fprofile-use.  With this change in place the tests are skipped on
> >> arm.
> >
> >> All feedback welcome,
> >
> > Seems reasonable, unless a -freorder-blocks-and-partition/-fprofile-use person thinks this is the wrong solution.
> >
> 
> Hi,
> 
> As promised, I tested this patch: it makes
> gcc.dg/tree-prof/section-attr-[123].c
> unsupported on arm*, and thus they are not failing anymore :-)
> 
> However, it also makes other tests unsupported, while they used to pass:
> 
>   gcc.dg/pr33648.c
>   gcc.dg/pr46685.c
>   gcc.dg/tree-prof/20041218-1.c
>   gcc.dg/tree-prof/bb-reorg.c
>   gcc.dg/tree-prof/cold_partition_label.c
>   gcc.dg/tree-prof/comp-goto-1.c
>   gcc.dg/tree-prof/pr34999.c
>   gcc.dg/tree-prof/pr45354.c
>   gcc.dg/tree-prof/pr50907.c
>   gcc.dg/tree-prof/pr52027.c
>   gcc.dg/tree-prof/va-arg-pack-1.c
> 
> and failures are now unsupported:
>   gcc.dg/tree-prof/cold_partition_label.c
>   gcc.dg/tree-prof/section-attr-1.c
>   gcc.dg/tree-prof/section-attr-2.c
>   gcc.dg/tree-prof/section-attr-3.c
> 
> So, maybe this patch is too strong?

In all of the cases that used to pass the tests are compile only tests
(except for cold_partition_label, which I discuss below).

On ARM passing -fprofile-use and -freorder-blocks-and-partition
results in a warning, and the -freorder-blocks-and-partition flag is
ignored.  However, disabling -freorder-blocks-and-partition doesn't
stop any of the tests compiling, hence the passes.

All the tests include:

  /* { dg-require-effective-target freorder } */

which I understand to mean, the tests requires the 'freorder' feature
to be supported (which corresponds to -freorder-blocks-and-partition).

For cold_partition_label and my new tests it's seems clear that the
lack of support for -freorder-blocks-and-partition on ARM is the cause
of the test failures.

So, is it reasonable to give up the other tests as "unsupported"?  I'd
be inclined to say yes, but I happy to rework the patch if anyone has
a suggestion for an alternative approach.

One possibility would be to split the 'freorder' feature into two, one
being 'Can -freorder-blocks-and-partition be used without causing an
error (so ARM would say yes to this)', and a new feature would be
'Does -freorder-blocks-and-partition actually _work_ on the target?'
we'd then use this in cold_partition_label and in my new tests.
Though this doesn't feel a great solution....

Thanks,
Andrew
Jeff Law Nov. 28, 2016, 10:08 p.m. UTC | #4
On 11/24/2016 02:40 PM, Andrew Burgess wrote:
> * Christophe Lyon <christophe.lyon@linaro.org> [2016-11-21 13:47:09 +0100]:
>
>> On 20 November 2016 at 18:27, Mike Stump <mikestump@comcast.net> wrote:
>>> On Nov 19, 2016, at 1:59 PM, Andrew Burgess <andrew.burgess@embecosm.com> wrote:
>>>>> So, your new test fails on arm* targets:
>>>>
>>>> After a little digging I think the problem might be that
>>>> -freorder-blocks-and-partition is not supported on arm.
>>>>
>>>> This should be detected as the new tests include:
>>>>
>>>>    /* { dg-require-effective-target freorder } */
>>>>
>>>> however this test passed on arm as -freorder-blocks-and-partition does
>>>> not issue any warning unless -fprofile-use is also passed.
>>>>
>>>> The patch below extends check_effective_target_freorder to check using
>>>> -fprofile-use.  With this change in place the tests are skipped on
>>>> arm.
>>>
>>>> All feedback welcome,
>>>
>>> Seems reasonable, unless a -freorder-blocks-and-partition/-fprofile-use person thinks this is the wrong solution.
>>>
>>
>> Hi,
>>
>> As promised, I tested this patch: it makes
>> gcc.dg/tree-prof/section-attr-[123].c
>> unsupported on arm*, and thus they are not failing anymore :-)
>>
>> However, it also makes other tests unsupported, while they used to pass:
>>
>>   gcc.dg/pr33648.c
>>   gcc.dg/pr46685.c
>>   gcc.dg/tree-prof/20041218-1.c
>>   gcc.dg/tree-prof/bb-reorg.c
>>   gcc.dg/tree-prof/cold_partition_label.c
>>   gcc.dg/tree-prof/comp-goto-1.c
>>   gcc.dg/tree-prof/pr34999.c
>>   gcc.dg/tree-prof/pr45354.c
>>   gcc.dg/tree-prof/pr50907.c
>>   gcc.dg/tree-prof/pr52027.c
>>   gcc.dg/tree-prof/va-arg-pack-1.c
>>
>> and failures are now unsupported:
>>   gcc.dg/tree-prof/cold_partition_label.c
>>   gcc.dg/tree-prof/section-attr-1.c
>>   gcc.dg/tree-prof/section-attr-2.c
>>   gcc.dg/tree-prof/section-attr-3.c
>>
>> So, maybe this patch is too strong?
>
> In all of the cases that used to pass the tests are compile only tests
> (except for cold_partition_label, which I discuss below).
>
> On ARM passing -fprofile-use and -freorder-blocks-and-partition
> results in a warning, and the -freorder-blocks-and-partition flag is
> ignored.  However, disabling -freorder-blocks-and-partition doesn't
> stop any of the tests compiling, hence the passes.
>
> All the tests include:
>
>   /* { dg-require-effective-target freorder } */
>
> which I understand to mean, the tests requires the 'freorder' feature
> to be supported (which corresponds to -freorder-blocks-and-partition).
>
> For cold_partition_label and my new tests it's seems clear that the
> lack of support for -freorder-blocks-and-partition on ARM is the cause
> of the test failures.
>
> So, is it reasonable to give up the other tests as "unsupported"?  I'd
> be inclined to say yes, but I happy to rework the patch if anyone has
> a suggestion for an alternative approach.
It is reasonable.  It's not uncommon to have to drop various tests to 
UNSUPPORTED, particularly things which depend on assembler/linker 
capabilities, the target runtime system, etc.

jeff
Andrew Burgess Nov. 29, 2016, 2:02 p.m. UTC | #5
* Jeff Law <law@redhat.com> [2016-11-28 15:08:46 -0700]:

> On 11/24/2016 02:40 PM, Andrew Burgess wrote:
> > * Christophe Lyon <christophe.lyon@linaro.org> [2016-11-21 13:47:09 +0100]:
> > 
> > > On 20 November 2016 at 18:27, Mike Stump <mikestump@comcast.net> wrote:
> > > > On Nov 19, 2016, at 1:59 PM, Andrew Burgess <andrew.burgess@embecosm.com> wrote:
> > > > > > So, your new test fails on arm* targets:
> > > > > 
> > > > > After a little digging I think the problem might be that
> > > > > -freorder-blocks-and-partition is not supported on arm.
> > > > > 
> > > > > This should be detected as the new tests include:
> > > > > 
> > > > >    /* { dg-require-effective-target freorder } */
> > > > > 
> > > > > however this test passed on arm as -freorder-blocks-and-partition does
> > > > > not issue any warning unless -fprofile-use is also passed.
> > > > > 
> > > > > The patch below extends check_effective_target_freorder to check using
> > > > > -fprofile-use.  With this change in place the tests are skipped on
> > > > > arm.
> > > > 
> > > > > All feedback welcome,
> > > > 
> > > > Seems reasonable, unless a -freorder-blocks-and-partition/-fprofile-use person thinks this is the wrong solution.
> > > > 
> > > 
> > > Hi,
> > > 
> > > As promised, I tested this patch: it makes
> > > gcc.dg/tree-prof/section-attr-[123].c
> > > unsupported on arm*, and thus they are not failing anymore :-)
> > > 
> > > However, it also makes other tests unsupported, while they used to pass:
> > > 
> > >   gcc.dg/pr33648.c
> > >   gcc.dg/pr46685.c
> > >   gcc.dg/tree-prof/20041218-1.c
> > >   gcc.dg/tree-prof/bb-reorg.c
> > >   gcc.dg/tree-prof/cold_partition_label.c
> > >   gcc.dg/tree-prof/comp-goto-1.c
> > >   gcc.dg/tree-prof/pr34999.c
> > >   gcc.dg/tree-prof/pr45354.c
> > >   gcc.dg/tree-prof/pr50907.c
> > >   gcc.dg/tree-prof/pr52027.c
> > >   gcc.dg/tree-prof/va-arg-pack-1.c
> > > 
> > > and failures are now unsupported:
> > >   gcc.dg/tree-prof/cold_partition_label.c
> > >   gcc.dg/tree-prof/section-attr-1.c
> > >   gcc.dg/tree-prof/section-attr-2.c
> > >   gcc.dg/tree-prof/section-attr-3.c
> > > 
> > > So, maybe this patch is too strong?
> > 
> > In all of the cases that used to pass the tests are compile only tests
> > (except for cold_partition_label, which I discuss below).
> > 
> > On ARM passing -fprofile-use and -freorder-blocks-and-partition
> > results in a warning, and the -freorder-blocks-and-partition flag is
> > ignored.  However, disabling -freorder-blocks-and-partition doesn't
> > stop any of the tests compiling, hence the passes.
> > 
> > All the tests include:
> > 
> >   /* { dg-require-effective-target freorder } */
> > 
> > which I understand to mean, the tests requires the 'freorder' feature
> > to be supported (which corresponds to -freorder-blocks-and-partition).
> > 
> > For cold_partition_label and my new tests it's seems clear that the
> > lack of support for -freorder-blocks-and-partition on ARM is the cause
> > of the test failures.
> > 
> > So, is it reasonable to give up the other tests as "unsupported"?  I'd
> > be inclined to say yes, but I happy to rework the patch if anyone has
> > a suggestion for an alternative approach.
> It is reasonable.  It's not uncommon to have to drop various tests to
> UNSUPPORTED, particularly things which depend on assembler/linker
> capabilities, the target runtime system, etc.

OK, I'm going to take that as approval for my patch[1].  I'll wait a
couple of days to give people a chance to correct me, then I'll push
the change.  This should resolve the test regressions I introduced for
ARM.

Thanks,
Andrew

[1] https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02050.html
Jeff Law Nov. 29, 2016, 5:35 p.m. UTC | #6
On 11/29/2016 07:02 AM, Andrew Burgess wrote:
> * Jeff Law <law@redhat.com> [2016-11-28 15:08:46 -0700]:
>
>> On 11/24/2016 02:40 PM, Andrew Burgess wrote:
>>> * Christophe Lyon <christophe.lyon@linaro.org> [2016-11-21 13:47:09 +0100]:
>>>
>>>> On 20 November 2016 at 18:27, Mike Stump <mikestump@comcast.net> wrote:
>>>>> On Nov 19, 2016, at 1:59 PM, Andrew Burgess <andrew.burgess@embecosm.com> wrote:
>>>>>>> So, your new test fails on arm* targets:
>>>>>>
>>>>>> After a little digging I think the problem might be that
>>>>>> -freorder-blocks-and-partition is not supported on arm.
>>>>>>
>>>>>> This should be detected as the new tests include:
>>>>>>
>>>>>>    /* { dg-require-effective-target freorder } */
>>>>>>
>>>>>> however this test passed on arm as -freorder-blocks-and-partition does
>>>>>> not issue any warning unless -fprofile-use is also passed.
>>>>>>
>>>>>> The patch below extends check_effective_target_freorder to check using
>>>>>> -fprofile-use.  With this change in place the tests are skipped on
>>>>>> arm.
>>>>>
>>>>>> All feedback welcome,
>>>>>
>>>>> Seems reasonable, unless a -freorder-blocks-and-partition/-fprofile-use person thinks this is the wrong solution.
>>>>>
>>>>
>>>> Hi,
>>>>
>>>> As promised, I tested this patch: it makes
>>>> gcc.dg/tree-prof/section-attr-[123].c
>>>> unsupported on arm*, and thus they are not failing anymore :-)
>>>>
>>>> However, it also makes other tests unsupported, while they used to pass:
>>>>
>>>>   gcc.dg/pr33648.c
>>>>   gcc.dg/pr46685.c
>>>>   gcc.dg/tree-prof/20041218-1.c
>>>>   gcc.dg/tree-prof/bb-reorg.c
>>>>   gcc.dg/tree-prof/cold_partition_label.c
>>>>   gcc.dg/tree-prof/comp-goto-1.c
>>>>   gcc.dg/tree-prof/pr34999.c
>>>>   gcc.dg/tree-prof/pr45354.c
>>>>   gcc.dg/tree-prof/pr50907.c
>>>>   gcc.dg/tree-prof/pr52027.c
>>>>   gcc.dg/tree-prof/va-arg-pack-1.c
>>>>
>>>> and failures are now unsupported:
>>>>   gcc.dg/tree-prof/cold_partition_label.c
>>>>   gcc.dg/tree-prof/section-attr-1.c
>>>>   gcc.dg/tree-prof/section-attr-2.c
>>>>   gcc.dg/tree-prof/section-attr-3.c
>>>>
>>>> So, maybe this patch is too strong?
>>>
>>> In all of the cases that used to pass the tests are compile only tests
>>> (except for cold_partition_label, which I discuss below).
>>>
>>> On ARM passing -fprofile-use and -freorder-blocks-and-partition
>>> results in a warning, and the -freorder-blocks-and-partition flag is
>>> ignored.  However, disabling -freorder-blocks-and-partition doesn't
>>> stop any of the tests compiling, hence the passes.
>>>
>>> All the tests include:
>>>
>>>   /* { dg-require-effective-target freorder } */
>>>
>>> which I understand to mean, the tests requires the 'freorder' feature
>>> to be supported (which corresponds to -freorder-blocks-and-partition).
>>>
>>> For cold_partition_label and my new tests it's seems clear that the
>>> lack of support for -freorder-blocks-and-partition on ARM is the cause
>>> of the test failures.
>>>
>>> So, is it reasonable to give up the other tests as "unsupported"?  I'd
>>> be inclined to say yes, but I happy to rework the patch if anyone has
>>> a suggestion for an alternative approach.
>> It is reasonable.  It's not uncommon to have to drop various tests to
>> UNSUPPORTED, particularly things which depend on assembler/linker
>> capabilities, the target runtime system, etc.
>
> OK, I'm going to take that as approval for my patch[1].  I'll wait a
> couple of days to give people a chance to correct me, then I'll push
> the change.  This should resolve the test regressions I introduced for
> ARM.
I'll just go ahead and explicitly ACK this.

Thanks,
jeff
Andrew Burgess Nov. 30, 2016, 11:39 a.m. UTC | #7
* Jeff Law <law@redhat.com> [2016-11-29 10:35:50 -0700]:

> On 11/29/2016 07:02 AM, Andrew Burgess wrote:
> > * Jeff Law <law@redhat.com> [2016-11-28 15:08:46 -0700]:
> > 
> > > On 11/24/2016 02:40 PM, Andrew Burgess wrote:
> > > > * Christophe Lyon <christophe.lyon@linaro.org> [2016-11-21 13:47:09 +0100]:
> > > > 
> > > > > On 20 November 2016 at 18:27, Mike Stump <mikestump@comcast.net> wrote:
> > > > > > On Nov 19, 2016, at 1:59 PM, Andrew Burgess <andrew.burgess@embecosm.com> wrote:
> > > > > > > > So, your new test fails on arm* targets:
> > > > > > > 
> > > > > > > After a little digging I think the problem might be that
> > > > > > > -freorder-blocks-and-partition is not supported on arm.
> > > > > > > 
> > > > > > > This should be detected as the new tests include:
> > > > > > > 
> > > > > > >    /* { dg-require-effective-target freorder } */
> > > > > > > 
> > > > > > > however this test passed on arm as -freorder-blocks-and-partition does
> > > > > > > not issue any warning unless -fprofile-use is also passed.
> > > > > > > 
> > > > > > > The patch below extends check_effective_target_freorder to check using
> > > > > > > -fprofile-use.  With this change in place the tests are skipped on
> > > > > > > arm.
> > > > > > 
> > > > > > > All feedback welcome,
> > > > > > 
> > > > > > Seems reasonable, unless a -freorder-blocks-and-partition/-fprofile-use person thinks this is the wrong solution.
> > > > > > 
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > As promised, I tested this patch: it makes
> > > > > gcc.dg/tree-prof/section-attr-[123].c
> > > > > unsupported on arm*, and thus they are not failing anymore :-)
> > > > > 
> > > > > However, it also makes other tests unsupported, while they used to pass:
> > > > > 
> > > > >   gcc.dg/pr33648.c
> > > > >   gcc.dg/pr46685.c
> > > > >   gcc.dg/tree-prof/20041218-1.c
> > > > >   gcc.dg/tree-prof/bb-reorg.c
> > > > >   gcc.dg/tree-prof/cold_partition_label.c
> > > > >   gcc.dg/tree-prof/comp-goto-1.c
> > > > >   gcc.dg/tree-prof/pr34999.c
> > > > >   gcc.dg/tree-prof/pr45354.c
> > > > >   gcc.dg/tree-prof/pr50907.c
> > > > >   gcc.dg/tree-prof/pr52027.c
> > > > >   gcc.dg/tree-prof/va-arg-pack-1.c
> > > > > 
> > > > > and failures are now unsupported:
> > > > >   gcc.dg/tree-prof/cold_partition_label.c
> > > > >   gcc.dg/tree-prof/section-attr-1.c
> > > > >   gcc.dg/tree-prof/section-attr-2.c
> > > > >   gcc.dg/tree-prof/section-attr-3.c
> > > > > 
> > > > > So, maybe this patch is too strong?
> > > > 
> > > > In all of the cases that used to pass the tests are compile only tests
> > > > (except for cold_partition_label, which I discuss below).
> > > > 
> > > > On ARM passing -fprofile-use and -freorder-blocks-and-partition
> > > > results in a warning, and the -freorder-blocks-and-partition flag is
> > > > ignored.  However, disabling -freorder-blocks-and-partition doesn't
> > > > stop any of the tests compiling, hence the passes.
> > > > 
> > > > All the tests include:
> > > > 
> > > >   /* { dg-require-effective-target freorder } */
> > > > 
> > > > which I understand to mean, the tests requires the 'freorder' feature
> > > > to be supported (which corresponds to -freorder-blocks-and-partition).
> > > > 
> > > > For cold_partition_label and my new tests it's seems clear that the
> > > > lack of support for -freorder-blocks-and-partition on ARM is the cause
> > > > of the test failures.
> > > > 
> > > > So, is it reasonable to give up the other tests as "unsupported"?  I'd
> > > > be inclined to say yes, but I happy to rework the patch if anyone has
> > > > a suggestion for an alternative approach.
> > > It is reasonable.  It's not uncommon to have to drop various tests to
> > > UNSUPPORTED, particularly things which depend on assembler/linker
> > > capabilities, the target runtime system, etc.
> > 
> > OK, I'm going to take that as approval for my patch[1].  I'll wait a
> > couple of days to give people a chance to correct me, then I'll push
> > the change.  This should resolve the test regressions I introduced for
> > ARM.
> I'll just go ahead and explicitly ACK this.

Committed as r243009.

Thanks,
Andrew
diff mbox

Patch

diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 8a2abd2..0716792 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -1014,9 +1014,15 @@  proc check_effective_target_fstack_protector {} {
 # for trivial code, 0 otherwise.
 
 proc check_effective_target_freorder {} {
-    return [check_no_compiler_messages freorder object {
+    if { [check_no_compiler_messages freorder object {
 	void foo (void) { }
     } "-freorder-blocks-and-partition"]
+    && [check_no_compiler_messages fprofile_use_freorder object {
+	void foo (void) { }
+    } "-fprofile-use -freorder-blocks-and-partition"] } {
+	return 1
+    }
+    return 0
 }
 
 # Return 1 if -fpic and -fPIC are supported, as in no warnings or errors