From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7629 invoked by alias); 19 Sep 2014 21:11:18 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 7580 invoked by uid 89); 19 Sep 2014 21:11:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-qa0-f43.google.com Received: from mail-qa0-f43.google.com (HELO mail-qa0-f43.google.com) (209.85.216.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 19 Sep 2014 21:11:13 +0000 Received: by mail-qa0-f43.google.com with SMTP id x12so3179021qac.2 for ; Fri, 19 Sep 2014 14:11:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=HRONPNE0nHycMmuSZkRhjnv8Gu/d9vcNa5ZrxneSoN8=; b=dXx+PuSYgPv8NeIE5CF7Y8lL2zEmwnV70HNJV8x4ztWhZnQ/Q+CxkKuErMLynJ6UD0 I0Uu2yyiTrjTc6zBrbgYsI3qi1vuVVzIOUZ2u0nn61xErRiQ3/rKV4aRydnTK02ZUsTi Qlo94bRf5oGbk8sQg0ouYIcptf6AJxDzzP5H6wAlLm8nYNDX7StAmyrsceRV9wRLvYFv eqXa3EMya0slidRX1Cj/pCNzQAO4I3TlCyXU0v3zP2zUBBCAHS8gJ/8Aq45udIlELpxk rc97ActoH/1PQdJ+p0kSpqSVbrAIoJOGGCwXXCJlGCBPcmRorMMzlFn4xCmed3TrH4yQ /Ing== X-Gm-Message-State: ALoCoQlTYuddPtMdzWoqnKG9fC772SCxYk2n13Rfwgj9KG3d/Xvtk5OmdBBWT+jNLYIlyJIJhmau MIME-Version: 1.0 X-Received: by 10.224.28.133 with SMTP id m5mr4365251qac.16.1411161071516; Fri, 19 Sep 2014 14:11:11 -0700 (PDT) Received: by 10.229.127.8 with HTTP; Fri, 19 Sep 2014 14:11:11 -0700 (PDT) In-Reply-To: References: <54062B52.9040706@redhat.com> Date: Fri, 19 Sep 2014 21:11:00 -0000 Message-ID: Subject: Re: [PATCH x86_64] Optimize access to globals in "-fpie -pie" builds with copy relocations From: Sriraman Tallam To: Richard Henderson Cc: GCC Patches , David Li , Cary Coutant , Ian Lance Taylor , Paul Pluzhnikov Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg01681.txt.bz2 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 wrote: > On Tue, Sep 2, 2014 at 1:40 PM, Richard Henderson 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~