From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 51334 invoked by alias); 3 Apr 2015 07:54:41 -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 51320 invoked by uid 89); 3 Apr 2015 07:54:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.2 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ob0-f175.google.com Received: from mail-ob0-f175.google.com (HELO mail-ob0-f175.google.com) (209.85.214.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 03 Apr 2015 07:54:39 +0000 Received: by obbgh1 with SMTP id gh1so155224847obb.1 for ; Fri, 03 Apr 2015 00:54:37 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.182.210.197 with SMTP id mw5mr1592347obc.26.1428047677320; Fri, 03 Apr 2015 00:54:37 -0700 (PDT) Received: by 10.202.229.72 with HTTP; Fri, 3 Apr 2015 00:54:37 -0700 (PDT) In-Reply-To: <20150402170115.GB21276@atrey.karlin.mff.cuni.cz> References: <20150312102706.GL27860@msticlxl57.ims.intel.com> <20150319083455.GD64546@msticlxl57.ims.intel.com> <20150402170115.GB21276@atrey.karlin.mff.cuni.cz> Date: Fri, 03 Apr 2015 07:54:00 -0000 Message-ID: Subject: Re: [CHKP, PATCH] Fix LTO cgraph merge for instrumented functions From: Ilya Enkovich To: Jan Hubicka Cc: gcc-patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-04/txt/msg00104.txt.bz2 2015-04-02 20:01 GMT+03:00 Jan Hubicka : >> Ping > It would help if you add hubicka@ucw.cz to CC for IPA related patches. >> >> 2015-03-19 11:34 GMT+03:00 Ilya Enkovich : >> > On 12 Mar 13:27, Ilya Enkovich wrote: >> >> Hi, >> >> >> >> Currently cgraph merge has several issues with instrumented code: >> >> - original function node may be removed => no assembler name conflict is detected between function and variable > > Why do you need to detect this one? Assembler name of instrumented function is a transparent alias of original function's name. Alias chains are not taken into account by analysis. Thus we see no conflict between instrumented function's name and a variable name but emit them with the same assembler name. To detect name conflict I keep original function node alive. >> >> - only orig_decl name is privatized for instrumented function => node still shares assembler name which causes infinite privatization loop >> >> - information about changed name is stored in file_data of instrumented node => original section name may be not found for original function >> >> - chkp reference is not fixed when nodes are merged >> >> >> >> This patch should fix theese problems by keeping instrumentation thunks reachable, privatizing both nodes and fixing chkp references. Bootstrapped and tested on x86_64-unknown-linux-gnu. OK for trunk? > > Next stage1 I definitly hope we will be able to reduce number of special cases > we need for chck nodes. I will try to read the code and your design document > and give it some tought. Original design didn't take into account several LTO aspect and additional patches appeared to fix various LTO issues. It would be nice to improve it with your help. I'll need to update a design document to reflect recent problems and used fixes. >> > 2015-03-19 Ilya Enkovich >> > >> > * ipa-chkp.h (chkp_maybe_fix_chkp_ref): New. >> > * ipa-chkp.c (chkp_maybe_fix_chkp_ref): New. >> > * ipa.c (symbol_table::remove_unreachable_nodes): Don't >> > remove instumentation thunks calling reachable functions. >> > * lto-cgraph.c: Include ipa-chkp.h. >> > (input_symtab): Fix chkp references for boundary nodes. >> > * lto/lto-partition.c (privatize_symbol_name_1): New. >> > (privatize_symbol_name): Privatize both decl and orig_decl >> > names for instrumented functions. >> > * lto/lto-symtab.c: Include ipa-chkp.h. >> > (lto_cgraph_replace_node): Fix chkp references for merged >> > function nodes. >> > >> > gcc/testsuite/ >> > >> > 2015-03-19 Ilya Enkovich >> > >> > * gcc.dg/lto/chkp-privatize-1_0.c: New. >> > * gcc.dg/lto/chkp-privatize-1_1.c: New. >> > * gcc.dg/lto/chkp-privatize-2_0.c: New. >> > * gcc.dg/lto/chkp-privatize-2_1.c: New. >> > >> > >> > diff --git a/gcc/ipa-chkp.c b/gcc/ipa-chkp.c >> > index 0b857ff..7a7fc3c 100644 >> > --- a/gcc/ipa-chkp.c >> > +++ b/gcc/ipa-chkp.c >> > @@ -414,6 +414,40 @@ chkp_instrumentable_p (tree fndecl) >> > && (!fn || !copy_forbidden (fn, fndecl))); >> > } >> > >> > +/* Check NODE has a correct IPA_REF_CHKP reference. >> > + Create a new reference if required. */ >> > + >> > +void >> > +chkp_maybe_fix_chkp_ref (cgraph_node *node) > > OK, so you have the chkp references that links to instrumented_version. > You do not stream them for boundary symbols but for some reason they are needed, > but when you can end up with wrong CHKP link? Wrong links may appear when cgraph nodes are merged. Thanks Ilya