This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH x86_64] Optimize access to globals in "-fpie -pie" builds with copy relocations
- From: Sriraman Tallam <tmsriram at google dot com>
- To: Richard Henderson <rth at redhat dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, David Li <davidxl at google dot com>, Cary Coutant <ccoutant at google dot com>, Ian Lance Taylor <iant at google dot com>, Paul Pluzhnikov <ppluzhnikov at google dot com>
- Date: Mon, 10 Nov 2014 15:22:42 -0800
- Subject: Re: [PATCH x86_64] Optimize access to globals in "-fpie -pie" builds with copy relocations
- Authentication-results: sourceware.org; auth=none
- References: <CAAs8Hmz8_=m1EoX=eQqX2gV+qwVOR_SO5tTCaz=MmCu3vwkpeQ at mail dot gmail dot com> <CAAs8Hmz8-tQ+BoHvSxpKErf9FG+-F7Mho9hv3FpwxLE0aBGcGQ at mail dot gmail dot com> <CAAs8HmxZJ0LdWjXb8u+yRnqjGp4y1C4GUTVQy_S7-tKaCMZVVA at mail dot gmail dot com> <CAAs8Hmw+fC9=1VDTGcOnJgPXFmMo3qtC=HqdNUvnyWrHbm51Mw at mail dot gmail dot com> <54062B52 dot 9040706 at redhat dot com> <CAAs8HmzLve+KEJjxMVHrZVEVF+ZxrtJ4rO3Ov1jmBiXb2Jg55Q at mail dot gmail dot com> <CAAs8HmxhcVBAkYTzrOTHhZnZuSzMLYJb=5QwBFzRg-AJYoMzQw at mail dot gmail dot com> <CAAs8Hmyuc20qTsPrfsa2Ct8rhvMAs_ZTC-LLbjD-tOk_BPoV3g at mail dot gmail dot com> <CAAs8HmzjrSiXRPa9Y4PMshxAQ6B5RG5JL0b7tzjJBeyo45Xv5Q at mail dot gmail dot com>
Ping.
On Mon, Oct 6, 2014 at 1:43 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> Ping.
>
> On Mon, Sep 29, 2014 at 10:57 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>> Ping.
>>
>> On Fri, Sep 19, 2014 at 2:11 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>> Hi Richard,
>>>
>>> I also ran the gcc testsuite with
>>> RUNTESTFLAGS="--tool_opts=-mcopyrelocs" to check for issues. The only
>>> test that failed was g++.dg/tsan/default_options.C. It uses -fpie
>>> -pie and BFD ld to link. Since BFD ld does not support copy
>>> relocations with -pie, it does not link. I linked with gold to make
>>> the test pass.
>>>
>>> Could you please take another look at this patch?
>>>
>>> Thanks
>>> Sri
>>>
>>> On Mon, Sep 8, 2014 at 3:19 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>>> On Tue, Sep 2, 2014 at 1:40 PM, Richard Henderson <rth@redhat.com> wrote:
>>>>> On 06/20/2014 05:17 PM, Sriraman Tallam wrote:
>>>>>> Index: config/i386/i386.c
>>>>>> ===================================================================
>>>>>> --- config/i386/i386.c (revision 211826)
>>>>>> +++ config/i386/i386.c (working copy)
>>>>>> @@ -12691,7 +12691,9 @@ legitimate_pic_address_disp_p (rtx disp)
>>>>>> return true;
>>>>>> }
>>>>>> else if (!SYMBOL_REF_FAR_ADDR_P (op0)
>>>>>> - && SYMBOL_REF_LOCAL_P (op0)
>>>>>> + && (SYMBOL_REF_LOCAL_P (op0)
>>>>>> + || (TARGET_64BIT && ix86_copyrelocs && flag_pie
>>>>>> + && !SYMBOL_REF_FUNCTION_P (op0)))
>>>>>> && ix86_cmodel != CM_LARGE_PIC)
>>>>>> return true;
>>>>>> break;
>>>>>
>>>>> This is the wrong place to patch.
>>>>>
>>>>> You ought to be adjusting SYMBOL_REF_LOCAL_P, by providing a modified
>>>>> TARGET_BINDS_LOCAL_P.
>>>>
>>>> I have done this in the new attached patch, I added a new function
>>>> i386_binds_local_p which will check for this and call
>>>> default_binds_local_p otherwise.
>>>>
>>>>>
>>>>> Note in particular that I believe that you are doing the wrong thing with weak
>>>>> and COMMON symbols, in that you probably ought not force a copy reloc there.
>>>>
>>>> I added an extra check to not do this for WEAK symbols. I also added a
>>>> check for DECL_EXTERNAL so I believe this will also not be called for
>>>> COMMON symbols.
>>>>
>>>>>
>>>>> Note the complexity of default_binds_local_p_1, and the fact that all you
>>>>> really want to modify is
>>>>>
>>>>> /* If PIC, then assume that any global name can be overridden by
>>>>> symbols resolved from other modules. */
>>>>> else if (shlib)
>>>>> local_p = false;
>>>>>
>>>>> near the bottom of that function.
>>>>
>>>> I did not understand what you mean here? Were you suggesting an
>>>> alternative way of doing this?
>>>>
>>>> Thanks for reviewing
>>>> Sri
>>>>
>>>>>
>>>>>
>>>>> r~