[PATCH x86_64] Optimize access to globals in "-fpie -pie" builds with copy relocations
Mon Sep 29 17:57:00 GMT 2014
On Fri, Sep 19, 2014 at 2:11 PM, Sriraman Tallam <email@example.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?
> On Mon, Sep 8, 2014 at 3:19 PM, Sriraman Tallam <firstname.lastname@example.org> wrote:
>> On Tue, Sep 2, 2014 at 1:40 PM, Richard Henderson <email@example.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;
>>> This is the wrong place to patch.
>>> You ought to be adjusting SYMBOL_REF_LOCAL_P, by providing a modified
>> 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
More information about the Gcc-patches