This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
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