From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 40461 invoked by alias); 23 Jul 2015 10:17:28 -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 40439 invoked by uid 89); 23 Jul 2015 10:17:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.4 required=5.0 tests=AWL,BAYES_50,KAM_STOCKGEN,SPF_PASS autolearn=no version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 23 Jul 2015 10:17:25 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-29-m2vKNyjUStCRQruXJb10SA-1; Thu, 23 Jul 2015 11:17:21 +0100 Received: from [10.2.207.50] ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Thu, 23 Jul 2015 11:17:20 +0100 Message-ID: <55B0BF30.2030007@arm.com> Date: Thu, 23 Jul 2015 10:27:00 -0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: James Greenhalgh CC: GCC Patches , Marcus Shawcroft , Richard Earnshaw Subject: Re: [PATCH][AArch64][9/14] Implement TARGET_CAN_INLINE_P References: <55A7CBDE.2090609@arm.com> <20150721160757.GA14953@arm.com> In-Reply-To: <20150721160757.GA14953@arm.com> X-MC-Unique: m2vKNyjUStCRQruXJb10SA-1 Content-Type: multipart/mixed; boundary="------------000804020602000608090503" X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg01913.txt.bz2 This is a multi-part message in MIME format. --------------000804020602000608090503 Content-Type: text/plain; charset=WINDOWS-1252; format=flowed Content-Transfer-Encoding: quoted-printable Content-length: 8336 On 21/07/15 17:07, James Greenhalgh wrote: > On Thu, Jul 16, 2015 at 04:21:02PM +0100, Kyrill Tkachov wrote: >> Hi all, >> >> This patch implements the target-specific inlining rules. >> The basic philosophy is that we want to definitely reject inlining if th= e callee's architecture >> is not a subset, feature-wise, of the caller's. >> >> Beyond that, we want to allow inlining if the callee is always_inline. >> If it's not, we reject inlining if the TargetSave options don't match up >> in a way that's described in the comments in the patch. >> >> Generally, we try to allow as much inlining as possible for the benefit = of LTO. >> However, if the architectural features of the callee are not a subset of= the features >> of the caller, then we must reject inlining. For example, inlining a fun= ction with 'simd' >> into a function without 'simd' is not allowed. >> >> Also, inlining a non-strict-align function into a strict-align function = is not allowed. >> These two restrictions apply even when the callee is tagged with always_= inline because they >> can affect the correctness of the program. >> >> Beyond that, we reject inlining only if the user has explicitly specifie= d attributes/options for >> both the caller and the callee and they don't match up. >> >> An exception to that are the tuning CPUs. We want to allow inlining even= when the tuning CPUs don't match. >> >> Bootstrapped and tested on aarch64. >> >> Ok for trunk? > Comments below. > >> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c >> index 0a6ed70..34cd986 100644 >> --- a/gcc/config/aarch64/aarch64.c >> +++ b/gcc/config/aarch64/aarch64.c >> @@ -8486,6 +8486,113 @@ aarch64_option_valid_attribute_p (tree fndecl, t= ree, tree args, int) >> return ret; >> } >>=20=20=20 >> +/* Helper for aarch64_can_inline_p. In the case where CALLER and CALLE= E are >> + tri-bool options (yes, no, don't care) and the default value is >> + DEF, determine whether to reject inlining. */ >> + >> +static bool >> +aarch64_reject_inlining (int caller, int callee, int dont_care, int def) >> +{ >> + /* If both caller and callee care about the value then reject inlining >> + if they don't match up. */ >> + if (caller !=3D dont_care && callee !=3D dont_care && caller !=3D cal= lee) >> + return true; >> + >> + /* If caller doesn't care then make sure that the default agrees >> + with the callee. */ >> + if (caller =3D=3D dont_care && callee !=3D dont_care && callee !=3D d= ef) >> + return true; > I don't like boolean functions which return the negative of how they > will be used, so how about flipping the sense of this function to be > positive and cleaning up the logical cases to be more explicit: > > static bool > aarch64_tribool_values_ok_for_inlining_p (int caller, int callee, > int dont_care, int def) > { > /* If the callee doesn't care, always allow inlining. */ > if (callee =3D=3D dont_care) > return true; > > /* If the caller doesn't care, always allow inlining. */ > if (caller =3D=3D dont_care) > return true; > > /* Otherwise, allow inlining if either the callee and caller values > agree, or if the callee is using the default value. > return (callee =3D=3D caller || callee =3D=3D def) > } > >> +/* Implement TARGET_CAN_INLINE_P. Decide whether it is valid >> + to inline CALLE into CALLER based on target-specific info. > s/CALLE/CALLEE/ > >> + Make sure that the caller and callee have compatible architectural >> + features. Then go through the other possible target attributes >> + and. Try not to reject always_inline callees unless they are > Typo: and... what? > >> + incompatible architecturally. */ >> + >> +static bool >> +aarch64_can_inline_p (tree caller, tree callee) >> +{ >> + bool ret =3D false; >> + tree caller_tree =3D DECL_FUNCTION_SPECIFIC_TARGET (caller); >> + tree callee_tree =3D DECL_FUNCTION_SPECIFIC_TARGET (callee); >> + >> + /* If callee has no option attributes, then it is ok to inline. */ >> + if (!callee_tree) >> + ret =3D true; > Just return true, drop the else and clean up the control flow, particular= ly > as the rest of the code below is going to fall through to each condition > in turn. > >> + else >> + { >> + struct cl_target_option *caller_opts >> + =3D TREE_TARGET_OPTION (caller_tree ? caller_tree >> + : target_option_default_node); >> + struct cl_target_option *callee_opts =3D TREE_TARGET_OPTION (call= ee_tree); >> + >> + >> + /* Callee's ISA flags should be a subset of the caller's. */ >> + if ((caller_opts->x_aarch64_isa_flags & callee_opts->x_aarch64_is= a_flags) >> + =3D=3D callee_opts->x_aarch64_isa_flags) >> + ret =3D true; > This is the only place that ret can become true. Why not switch the sense > of the comparison (!=3D callee_opts...) and simply return false here. > > Then each of the cases below can just return false as they go and the > fallthrough case can return true. > > That seems much more readable to me! Thanks, I've implemented the suggestions. Re-bootstrapped and tested on aarch64. How's this? Thanks, Kyrill 2015-07-23 Kyrylo Tkachov * config/aarch64/aarch64.c (aarch64_tribools_ok_for_inlining_p): New function. (aarch64_can_inline_p): Likewise. (TARGET_CAN_INLINE_P): Define. > > Thanks, > James > > >> + >> + /* Allow non-strict aligned functions inlining into strict >> + aligned ones. */ >> + if ((TARGET_STRICT_ALIGN_P (caller_opts->x_target_flags) >> + !=3D TARGET_STRICT_ALIGN_P (callee_opts->x_target_flags)) >> + && !(!TARGET_STRICT_ALIGN_P (callee_opts->x_target_flags) >> + && TARGET_STRICT_ALIGN_P (caller_opts->x_target_flags))) >> + ret =3D false; >> + >> + bool always_inline =3D lookup_attribute ("always_inline", >> + DECL_ATTRIBUTES (callee)); >> + >> + /* If the architectural features match up and the callee is alway= s_inline >> + then the other attributes don't matter. */ >> + if (always_inline) >> + return ret; >> + >> + if (caller_opts->x_aarch64_cmodel_var >> + !=3D callee_opts->x_aarch64_cmodel_var) >> + ret =3D false; >> + >> + if (caller_opts->x_aarch64_tls_dialect >> + !=3D callee_opts->x_aarch64_tls_dialect) >> + ret =3D false; >> + >> + >> + /* Honour explicit requests to workaround errata. */ >> + if (aarch64_reject_inlining (caller_opts->x_aarch64_fix_a53_err83= 5769, >> + callee_opts->x_aarch64_fix_a53_err835769, >> + 2, TARGET_FIX_ERR_A53_835769_DEFAULT)) >> + ret =3D false; >> + >> + /* If the user explicitly specified -momit-leaf-frame-pointer for= the >> + caller and calle and they don't match up, reject inlining. */ >> + if (aarch64_reject_inlining (caller_opts->x_flag_omit_leaf_frame_= pointer, >> + callee_opts->x_flag_omit_leaf_frame_pointer, >> + 2, 1)) >> + ret =3D false; >> + >> + /* If the callee has specific tuning overrides, respect them. */ >> + if (callee_opts->x_aarch64_override_tune_string !=3D NULL >> + && caller_opts->x_aarch64_override_tune_string =3D=3D NULL) >> + ret =3D false; >> + >> + /* If the user specified tuning override strings for the >> + caller and callee and they don't match up, reject inlining. >> + We just do a string compare here, we don't analyze the meaning >> + of the string, as it would be too costly for little gain. */ >> + if (callee_opts->x_aarch64_override_tune_string >> + && caller_opts->x_aarch64_override_tune_string >> + && (strcmp (callee_opts->x_aarch64_override_tune_string, >> + caller_opts->x_aarch64_override_tune_string) !=3D 0)) >> + ret =3D false; >> + } >> + >> + return ret; >> +} >> + >> /* Return true if SYMBOL_REF X binds locally. */ >>=20=20=20 >> static bool >> @@ -12963,6 +13070,9 @@ aarch64_unspec_may_trap_p (const_rtx x, unsigned= flags) >> #undef TARGET_OPTION_VALID_ATTRIBUTE_P >> #define TARGET_OPTION_VALID_ATTRIBUTE_P aarch64_option_valid_attribute= _p >>=20=20=20 >> +#undef TARGET_CAN_INLINE_P >> +#define TARGET_CAN_INLINE_P aarch64_can_inline_p >> + >> #undef TARGET_PASS_BY_REFERENCE >> #define TARGET_PASS_BY_REFERENCE aarch64_pass_by_reference >>=20=20=20 --------------000804020602000608090503 Content-Type: text/x-patch; name=aarch64-attrs-9.patch Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="aarch64-attrs-9.patch" Content-length: 4945 commit cc49230c0ed153759e6f617c6251d48f9a1814b2 Author: Kyrylo Tkachov Date: Thu May 14 12:00:07 2015 +0100 [AArch64][9/N] Implement TARGET_CAN_INLINE_P diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index ba82ca9..ff584fe 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -8486,6 +8486,113 @@ aarch64_option_valid_attribute_p (tree fndecl, tree= , tree args, int) return ret; } =20 +/* Helper for aarch64_can_inline_p. In the case where CALLER and CALLEE a= re + tri-bool options (yes, no, don't care) and the default value is + DEF, determine whether to reject inlining. */ + +static bool +aarch64_tribools_ok_for_inlining_p (int caller, int callee, + int dont_care, int def) +{ + /* If the callee doesn't care, always allow inlining. */ + if (callee =3D=3D dont_care) + return true; + + /* If the caller doesn't care, always allow inlining. */ + if (caller =3D=3D dont_care) + return true; + + /* Otherwise, allow inlining if either the callee and caller values + agree, or if the callee is using the default value. */ + return (callee =3D=3D caller || callee =3D=3D def); +} + +/* Implement TARGET_CAN_INLINE_P. Decide whether it is valid + to inline CALLEE into CALLER based on target-specific info. + Make sure that the caller and callee have compatible architectural + features. Then go through the other possible target attributes + and see if they can block inlining. Try not to reject always_inline + callees unless they are incompatible architecturally. */ + +static bool +aarch64_can_inline_p (tree caller, tree callee) +{ + tree caller_tree =3D DECL_FUNCTION_SPECIFIC_TARGET (caller); + tree callee_tree =3D DECL_FUNCTION_SPECIFIC_TARGET (callee); + + /* If callee has no option attributes, then it is ok to inline. */ + if (!callee_tree) + return true; + + struct cl_target_option *caller_opts + =3D TREE_TARGET_OPTION (caller_tree ? caller_tree + : target_option_default_node); + + struct cl_target_option *callee_opts =3D TREE_TARGET_OPTION (callee_tree= ); + + + /* Callee's ISA flags should be a subset of the caller's. */ + if ((caller_opts->x_aarch64_isa_flags & callee_opts->x_aarch64_isa_flags) + !=3D callee_opts->x_aarch64_isa_flags) + return false; + + /* Allow non-strict aligned functions inlining into strict + aligned ones. */ + if ((TARGET_STRICT_ALIGN_P (caller_opts->x_target_flags) + !=3D TARGET_STRICT_ALIGN_P (callee_opts->x_target_flags)) + && !(!TARGET_STRICT_ALIGN_P (callee_opts->x_target_flags) + && TARGET_STRICT_ALIGN_P (caller_opts->x_target_flags))) + return false; + + bool always_inline =3D lookup_attribute ("always_inline", + DECL_ATTRIBUTES (callee)); + + /* If the architectural features match up and the callee is always_inline + then the other attributes don't matter. */ + if (always_inline) + return true; + + if (caller_opts->x_aarch64_cmodel_var + !=3D callee_opts->x_aarch64_cmodel_var) + return false; + + if (caller_opts->x_aarch64_tls_dialect + !=3D callee_opts->x_aarch64_tls_dialect) + return false; + + /* Honour explicit requests to workaround errata. */ + if (!aarch64_tribools_ok_for_inlining_p ( + caller_opts->x_aarch64_fix_a53_err835769, + callee_opts->x_aarch64_fix_a53_err835769, + 2, TARGET_FIX_ERR_A53_835769_DEFAULT)) + return false; + + /* If the user explicitly specified -momit-leaf-frame-pointer for the + caller and calle and they don't match up, reject inlining. */ + if (!aarch64_tribools_ok_for_inlining_p ( + caller_opts->x_flag_omit_leaf_frame_pointer, + callee_opts->x_flag_omit_leaf_frame_pointer, + 2, 1)) + return false; + + /* If the callee has specific tuning overrides, respect them. */ + if (callee_opts->x_aarch64_override_tune_string !=3D NULL + && caller_opts->x_aarch64_override_tune_string =3D=3D NULL) + return false; + + /* If the user specified tuning override strings for the + caller and callee and they don't match up, reject inlining. + We just do a string compare here, we don't analyze the meaning + of the string, as it would be too costly for little gain. */ + if (callee_opts->x_aarch64_override_tune_string + && caller_opts->x_aarch64_override_tune_string + && (strcmp (callee_opts->x_aarch64_override_tune_string, + caller_opts->x_aarch64_override_tune_string) !=3D 0)) + return false; + + return true; +} + /* Return true if SYMBOL_REF X binds locally. */ =20 static bool @@ -12850,6 +12957,9 @@ aarch64_unspec_may_trap_p (const_rtx x, unsigned fl= ags) #undef TARGET_CAN_ELIMINATE #define TARGET_CAN_ELIMINATE aarch64_can_eliminate =20 +#undef TARGET_CAN_INLINE_P +#define TARGET_CAN_INLINE_P aarch64_can_inline_p + #undef TARGET_CANNOT_FORCE_CONST_MEM #define TARGET_CANNOT_FORCE_CONST_MEM aarch64_cannot_force_const_mem =20 --------------000804020602000608090503--