Re: [patch] testsuite enable PIE tests on FreeBSD

[Andreas Tobler <andreast-list@fgznet.ch>]

On 22.05.15 23:47, Jeff Law wrote:
> On 05/21/2015 02:01 PM, Andreas Tobler wrote:
>> On 21.05.15 20:14, Andreas Tobler wrote:
>>> On 20.05.15 22:30, Jeff Law wrote:
>>>> On 05/20/2015 11:04 AM, Andreas Tobler wrote:
>>>>> Hi,
>>>>>
>>>>> the attached patch enables some PIE tests on FreeBSD.
>>>>>
>>>>> Ok for trunk?
>>>>>
>>>>> Thanks,
>>>>> Andreas
>>>>>
>>>>> 2015-05-20  Andreas Tobler  <andreast@gcc.gnu.org>
>>>>>
>>>>>         * gcc.target/i386/pr32219-1.c: Enable test on FreeBSD.
>>>>>         * gcc.target/i386/pr32219-2.c: Likewise.
>>>>>         * gcc.target/i386/pr32219-3.c: Likewise.
>>>>>         * gcc.target/i386/pr32219-4.c: Likewise.
>>>>>         * gcc.target/i386/pr32219-5.c: Likewise.
>>>>>         * gcc.target/i386/pr32219-6.c: Likewise
>>>>>         * gcc.target/i386/pr32219-7.c: Likewise.
>>>>>         * gcc.target/i386/pr32219-8.c: Likewise.
>>>>>         * gcc.target/i386/pr39013-1.c: Likewise.
>>>>>         * gcc.target/i386/pr39013-2.c: Likewise.
>>>>>         * gcc.target/i386/pr64317.c: Likewise.
>>>> Wouldn't it be better to remove the target selector and instead add:
>>>>
>>>> /* { dg-require-effective-target pie } */
>>>>
>>>> In each of those tests?
>>>>
>>>> While the net effect is the same today, it means there's only one place
>>>> to change if another x86 target gains PIE support in the future.
>>>>
>>>> Pre-approved using that style.
>>>
>>> Thanks!
>>>
>>> Tested on amd64-freebsd and CentOS.
>>>
>>> Andreas
>>>
>>>
>>> This is what I committed:
>>>
>>> 2015-05-21  Andreas Tobler  <andreast@gcc.gnu.org>
>>>
>>>      * gcc.target/i386/pr32219-1.c: Use 'dg-require-effective-target pie'
>>>      instead of listing several targets on its own.
>>
>>>      * gcc.target/i386/pr64317.c: Likewise.
>>
>> Yes, I know. The comment and the content for this test case do not match.
>>
>> Is this ok:
>>
>> Index: gcc.target/i386/pr64317.c
>> ===================================================================
>> --- gcc.target/i386/pr64317.c    (revision 223498)
>> +++ gcc.target/i386/pr64317.c    (working copy)
>> @@ -1,4 +1,5 @@
>> -/* { dg-do compile { target { { *-*-freebsd* *-*-linux* } && ia32 } } } */
>> +/* { dg-do compile { target ia32 } } */
>> +/* { dg-require-effective-target pie } */
>>    /* { dg-options "-O2 -fpie" } */
>>    /* { dg-final { scan-assembler "addl\[
>> \\t\]+\[$\]_GLOBAL_OFFSET_TABLE_, %ebx" } } */
>>    /* { dg-final { scan-assembler "movl\[ \\t\]+c@GOTOFF\[(\]%ebx\[)\]" }
>> } */
>>
>>
>> or do you prefer instead of
>>
>> /* { dg-do compile { target ia32 } } */
>>
>> this one:
>>
>> /* { dg-do compile } */
>> /* { dg-require-effective-target ia32 } */
>>
>>
>> Btw, all three variants run on i386.
> It looks like the dg-require-effective-target is typically on a line by
> itself.  So let's stick with the last version.

So, finally committed. Thanks!
Andreas