From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 33815 invoked by alias); 14 Nov 2018 17:54:57 -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 33796 invoked by uid 89); 14 Nov 2018 17:54:56 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,HTML_MESSAGE,SPF_HELO_PASS,SPF_PASS,UNPARSEABLE_RELAY autolearn=ham version=3.3.2 spammy= X-HELO: userp2120.oracle.com Received: from userp2120.oracle.com (HELO userp2120.oracle.com) (156.151.31.85) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 14 Nov 2018 17:54:53 +0000 Received: from pps.filterd (userp2120.oracle.com [127.0.0.1]) by userp2120.oracle.com (8.16.0.22/8.16.0.22) with SMTP id wAEHi5Vh163516; Wed, 14 Nov 2018 17:54:51 GMT Received: from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233]) by userp2120.oracle.com with ESMTP id 2nr7cs5300-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 14 Nov 2018 17:54:51 +0000 Received: from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72]) by aserv0021.oracle.com (8.14.4/8.14.4) with ESMTP id wAEHsnHN008415 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 14 Nov 2018 17:54:49 GMT Received: from abhmp0013.oracle.com (abhmp0013.oracle.com [141.146.116.19]) by userv0121.oracle.com (8.14.4/8.13.8) with ESMTP id wAEHsm7f001457; Wed, 14 Nov 2018 17:54:48 GMT Received: from dhcp-10-159-247-249.vpn.oracle.com (/10.159.247.249) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 14 Nov 2018 09:54:47 -0800 From: Qing Zhao Message-Id: <8F7AEFF2-5DCE-49B6-BC9E-34FC0DEF2A55@oracle.com> Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: Re: [PATCH][RFC] Come up with -flive-patching master option. Date: Wed, 14 Nov 2018 17:54:00 -0000 In-Reply-To: Cc: Miroslav Benes , Jan Hubicka , Martin Jambor , live-patching@vger.kernel.org, gcc Patches To: =?utf-8?Q?Martin_Li=C5=A1ka?= References: <1a023bdc-28a6-eb41-b449-4d096f12064f@suse.cz> <048D9997-B7AF-444A-BF7E-79944DE8F174@oracle.com> <3E37D3A8-2D19-41C2-BA8A-8F0EFA1B4D5C@oracle.com> <10a54034-279b-a406-8466-55558effbf24@suse.cz> <20181003090457.GJ57692@kam.mff.cuni.cz> <54a75932-201b-671c-0a63-d1a5d8d7b562@suse.cz> <90c91045-cb9d-0bd2-fad3-d16426ceede6@suse.cz> <20181105095135.j3mnzox6rkktkoto@kam.mff.cuni.cz> <629b20d2-7b0b-9342-a64a-d12eb6e6a6b7@suse.cz> <20181108145909.5eh5ccen7a7elspe@kam.mff.cuni.cz> <173116c6-d51c-3bef-5382-38593e032f82@suse.cz> <8f467934-c088-9b60-c4d3-c19c2e03defd@suse.cz> <2F74DD05-E807-4FB4-80B1-3BBC6FEDF6E8@oracle.com> <7FA7E4B3-BF39-4E60-84AD-91161CC4867F@oracle.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg01310.txt.bz2 Hi,=20 > On Nov 14, 2018, at 9:03 AM, Martin Li=C5=A1ka wrote: >=20 >>>=20 >> Yes, you are right. I added this into my patch. >>=20 >> I am attaching the new patch here. >=20 > Hello. >=20 > Please use > git diff HEAD~ > /tmp/patch && ~/Programming/gcc/contrib/check_GNU_style.= py /tmp/patch >=20 > in order to address many formatting issues of the patch (skip the ones re= ported in common.opt). will do and fix the style issues. >=20 >>=20 >>=20 >> +flive-patching >> +Common RejectNegative Alias(flive-patching=3D,inline-clone) Optimization >> + >> +flive-patching=3D >> +Common Report Joined RejectNegative Enum(live_patching_level) Var(flag_= live_patching) Init(LIVE_NONE) Optimization >> +-flive-patching=3D[inline-only-static|inline-clone] Control ipa optimiz= ations to provide a=20 >=20 > Please use 'IPA' instead of 'ipa', similarly in documentation. Okay. >=20 >> --- a/gcc/doc/invoke.texi >> +++ b/gcc/doc/invoke.texi >> @@ -411,10 +411,11 @@ Objective-C and Objective-C++ Dialects}. >> -fgcse-sm -fhoist-adjacent-loads -fif-conversion @gol >> -fif-conversion2 -findirect-inlining @gol >> -finline-functions -finline-functions-called-once -finline-limit=3D@va= r{n} @gol >> --finline-small-functions -fipa-cp -fipa-cp-clone @gol >> +-finline-small-functions -fipa-cp -fipa-cp-clone @gol >=20 > This changes is probably not intended. No. will delete it. >=20 >>=20 >> diff --git a/gcc/flag-types.h b/gcc/flag-types.h >> index 500f663..72e0f0f 100644 >> --- a/gcc/flag-types.h >> +++ b/gcc/flag-types.h >> @@ -123,6 +123,14 @@ enum stack_reuse_level >> SR_ALL >> }; >>=20 >> +/* The live patching level. */ >> +enum live_patching_level >> +{ >> + LIVE_NONE =3D 0, >> + LIVE_INLINE_ONLY_STATIC, >> + LIVE_INLINE_CLONE >=20 > Maybe better LIVE_PATCHING_INLINE_CLONE, without the 'PATCHING' the enum > values are bit unclear. Okay. >=20 >>=20 >> + /* visibility change should be excluded by !flag_whole_program=20 >> + && !in_lto_p && !flag_ipa_cp_clone && !flag_ipa_sra=20 >=20 > You added sorry about LTO, maybe then !in_lto_p would be always true? Yes, since live-patching does not support LTO currently, !in_lto_p is alwa= ys TRUE. that=E2=80=99s the reason no need for a new flag to disable visibi= lity change.=20 >=20 >> + && !flag_partial_inlining. */ >> + if (!opts_set->x_flag_ipa_pta) >> + opts->x_flag_ipa_pta =3D 0; >> + if (!opts_set->x_flag_ipa_reference) >> + opts->x_flag_ipa_reference =3D 0; >> + if (!opts_set->x_flag_ipa_ra) >> + opts->x_flag_ipa_ra =3D 0; >> + if (!opts_set->x_flag_ipa_icf) >> + opts->x_flag_ipa_icf =3D 0; >> + if (!opts_set->x_flag_ipa_icf_functions) >> + opts->x_flag_ipa_icf_functions =3D 0; >> + if (!opts_set->x_flag_ipa_icf_variables) >> + opts->x_flag_ipa_icf_variables =3D 0; >> + if (!opts_set->x_flag_ipa_bit_cp) >> + opts->x_flag_ipa_bit_cp =3D 0; >> + if (!opts_set->x_flag_ipa_vrp) >> + opts->x_flag_ipa_vrp =3D 0; >> + if (!opts_set->x_flag_ipa_pure_const) >=20 > Can you please explain why you included: > if (!opts_set->x_flag_ipa_bit_cp) > opts->x_flag_ipa_bit_cp =3D 0; > if (!opts_set->x_flag_ipa_vrp) > opts->x_flag_ipa_vrp =3D 0; interprocedural bitwise constant propagation and interprocedural propagatio= n of value ranges does not involve creating clones, and the bitwise constant and value ranges info extracted during ipa-cp phas= e are used later by other optimizations. their effect on=20 impact functions are not clear at this moment. that=E2=80=99s the reason I = think we need to disable these two.=20 Martin Jambor raised this issue during our previous discussion on 10/03/201= 8: =E2=80=9C I was thinking a bit more about this and recalled that not all stuff that IPA-CP nowadays does involves creating clones, so we have to add also: - -fno-ipa-bit-cp, and - -fno-ipa-vrp. These two just record info in the parameters of *callees* of functions from which it extracted info, without any cloning involved. Both were introduced in GCC 7. Thanks, Martin =E2=80=9C and I think he is right. > ? >=20 >> + opts->x_flag_ipa_pure_const =3D 0; >> + /* unreachable code removal. */ >> + /* discovery of functions/variables with no address taken. */ >=20 > ^^^ these 2 comments looks misaligned. will fix them.=20 >=20 >>=20 >> +++ b/gcc/testsuite/gcc.dg/live-patching-1.c >> @@ -0,0 +1,22 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-O2 -flive-patching=3Dinline-only-static -fdump-ipa-in= line" } */ >> + >> +extern int sum, n, m; >> + >> +int foo (int a) >> +{ >> + return a + n; >> +} >> + >> +static int bar (int b) >> +{ >> + return b * m; >> +} >> + >> +int main() >> +{ >> + sum =3D foo (m) + bar (n);=20 >> + return 0; >> +} >> + >> +/* { dg-final { scan-ipa-dump "foo/0 function has external linkage when= the user requests only inlining static for live patching" "inline" } } */ >> -- 1.9.1 >=20 > It would be also handy to test the newly added option. not sure how to test it? any suggestion? > Please add ChangeLog entry for the patch. Okay. > Have you bootstrapped the patch and run test-suite? did on aarch64. I will do it on x86_64 as well. thanks. Qing