From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21976 invoked by alias); 27 Sep 2018 07:46:37 -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 19264 invoked by uid 89); 27 Sep 2018 07:46:04 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_PASS autolearn=ham version=3.3.2 spammy=dumpfile, spotting, H*c:HHHHH X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 27 Sep 2018 07:46:02 +0000 Received: from relay1.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 22A46ADA8; Thu, 27 Sep 2018 07:46:00 +0000 (UTC) Date: Thu, 27 Sep 2018 07:46:00 -0000 From: Richard Biener To: Qing Zhao cc: Alexander Monakov , gcc Patches , jeff Law , Jakub Jelinek , andrew Pinski , martin Sebor Subject: Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions In-Reply-To: Message-ID: References: <7f314cc6-8cd3-a6f2-27b1-df4ef637542e@gmail.com> User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 Content-Type: multipart/mixed; BOUNDARY="-1609908220-2064143425-1538034360=:16707" X-SW-Source: 2018-09/txt/msg01628.txt.bz2 This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. ---1609908220-2064143425-1538034360=:16707 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Content-length: 3293 On Wed, 26 Sep 2018, Qing Zhao wrote: > > > On Sep 26, 2018, at 6:07 PM, Alexander Monakov wrote: > > > > On Wed, 26 Sep 2018, Qing Zhao wrote: > >> The request is for application developers who want to use gcc's online > >> patching feature. > >> > >> Today, developers can turn off inlining and deliver just the patched routine. They > >> can also allow all inlining and deliver the patched routine and all the routines > >> that the patched routine was inlined into. > >> > >> completely turning off inlining will sacrifice too much run-time performance. completely > >> enable inlining, on the other hand, will have the potential issues with code size, complexity and > >> debuggability for the online patching. > >> > >> the proposed option provides a compromised solution for the above issues. and enable more > >> developers to utilize gcc’s online patching feature. > > > > From this explanation it sounds to me that what you really need is -Os-like > > behavior for IPA passes, without enabling -Os for gimple/rtl passes, as I > > mentioned in my previous email. Honza, how does that sound? > > I don’t think a -Os-like option will do the job. > > As Jeff already mentioned in a very previous email: > > “Presumably one of the cases where this capability is really helpful is > things like ksplice. If you have a function with global scope that has > been potentially inlined, then it's a lot harder track down those > inlining points at DTRT. > > We ran into this internally when looking at hot patching some of the > spinlock code in the kernel. It would have been real helpful if the > kernel had been compiled with this kind of option :-) > > So conceptually I can see value in this kind of option. > “ > > so, specially control inlining on static/global will be helpful to online patch. But as Honza said this gets you only sofar. IIRC for our kernel livepatching we turn off most IPA passes because while we can "easily" figure what and where things were inlined spotting the effects of IPA analysis and transform is almost impossible. So there's two parts of the knob - one is to make the live-patch size not explode (do less inlining where it doesn't hurt performance - that eventually also applies to static functions called once inlining!). The other is to make it possible to conservatively compute the set of functions you have to replace (the set of functions that are affected by a patch). Having an option to _that_ effect might indeed be useful (to avoid people chasing all the flags they need to disable). So shouldn't this be a -fease-live-patching option rather that -finline-only-static which doesn't really capture the intention nor the effect? That is, -fease-live-patching would guarantee that if you source-patch function X then, if you replace all functions which debuginfo tells you X was inlined to, the result will be semantically equivalent with replacing the whole program? We might even add sth like -fpatch-symbol-list=FOO,BAR that outputs a list of symbols into BAR that are affected this way when functions FOO are changed (you run that on unpatched code of course). Or we add sth to the cgraph dumpfile that for each function lists the set of symbols it was affected by. Thanks, Richard. ---1609908220-2064143425-1538034360=:16707--