Re: [PATCH x86_64] Optimize access to globals in "-fpie -pie" builds with copy relocations

[Sriraman Tallam <tmsriram at google dot com>]

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~

Attachment: gcc_pie_copyrelocs_patch.txt
Description: Text document