Ping.
On Mon, Oct 6, 2014 at 1:43 PM, Sriraman Tallam <tmsri...@google.com> wrote: > Ping. > > On Mon, Sep 29, 2014 at 10:57 AM, Sriraman Tallam <tmsri...@google.com> wrote: >> Ping. >> >> On Fri, Sep 19, 2014 at 2:11 PM, Sriraman Tallam <tmsri...@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 <tmsri...@google.com> wrote: >>>> On Tue, Sep 2, 2014 at 1:40 PM, Richard Henderson <r...@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~