This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [committed] Add fpic requirement to tests that use PIC options


Hans-Peter Nilsson <hp@bitrange.com> writes:
> I think the gcc.dg/20020426-2.c part was wrong.  It already
> passed on non-fpic targets that were not x86 (using "-O2"
> there); you should have added the fpic to the target list
> instead.

Argh, yes indeed.  I was supposedly trying to make sure that when the
default options were non-PIC, I'd patched the offending PIC dg-options
instead (as in 981006.c), but I fluffed it in this case.  Fixed with
the patch below, thanks.  Tested on x86_64-linux-gnu and applied as
obvious.

>> Index: gcc/testsuite/gcc.target/i386/20011119-1.c
>> ===================================================================
>> --- gcc/testsuite/gcc.target/i386/20011119-1.c	(revision 122834)
>> +++ gcc/testsuite/gcc.target/i386/20011119-1.c	(working copy)
>> @@ -1,6 +1,7 @@
>>  /* Test for reload failing to eliminate from argp to sp.  */
>>  /* { dg-do run { target i?86-*-* x86_64-*-* } } */
>>  /* { dg-require-effective-target ilp32 } */
>> +/* { dg-require-effective-target fpic } */
>>  /* { dg-skip-if "" { "*-*-*" } { "-fpic" "-fPIC" } { "" } } */
>>  /* { dg-skip-if "PIC default" { "*-*-darwin*" } { "*" } { "" } } */
>>  /* { dg-options "-O2 -fomit-frame-pointer" } */
>
> Likewise here and in many of the other tests, but that's more of
> a nit as it doesn't change what's tested where.  I guess you
> missed that that you don't have to use
> dg-require-effective-target, right?

No, I didn't miss it: it was actually a deliberate decision.  As you can
see from the example you quote, dg-effective-target is already used for
other such things, and I personally find:

/* { dg-do run { target { { i?86-*-* x86_64-*-* } && { ilp32 && fpic } } } } */

much less easy to parse at first glance than:

/* { dg-do run { target i?86-*-* x86_64-*-* } } */
/* { dg-require-effective-target ilp32 } */
/* { dg-require-effective-target fpic } */

I did start out doing it all with target selectors.  The one that
really convinced me not to was:

/* { dg-do compile { target { { i?86-*-* rs6000-*-* alpha*-*-* x86_64-*-* } || { powerpc*-*-* && ilp32 } } } } */

which would have had to become:

/* { dg-do compile { target { { { i?86-*-* rs6000-*-* alpha*-*-* x86_64-*-* } || { powerpc*-*-* && ilp32 } } && fpic } } } */

With all those { }s, and with it spanning more than one line, it's hard
to tell at first glance what "&& fpic" is being combined with.  On the
other hand, a separate:

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

isn't open to any doubt.

I think it's appropriate to use dg-require-effective-target for
conditions that apply to the test as a whole.  As I understand it, the
enhanced target selection was added to make things like the dg-options
above possible (and xfails, and ...).  I didn't think it was an edict
that everything should now use target selectors.

Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]