From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8191 invoked by alias); 8 Jun 2011 09:11:43 -0000 Received: (qmail 8123 invoked by uid 22791); 8 Jun 2011 09:11:41 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RFC_ABUSE_POST X-Spam-Check-By: sourceware.org Received: from mail-ww0-f51.google.com (HELO mail-ww0-f51.google.com) (74.125.82.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 08 Jun 2011 09:11:22 +0000 Received: by wwf26 with SMTP id 26so248039wwf.8 for ; Wed, 08 Jun 2011 02:11:21 -0700 (PDT) MIME-Version: 1.0 Received: by 10.227.197.83 with SMTP id ej19mr7148869wbb.105.1307524281431; Wed, 08 Jun 2011 02:11:21 -0700 (PDT) Received: by 10.227.37.152 with HTTP; Wed, 8 Jun 2011 02:11:21 -0700 (PDT) In-Reply-To: <4DEF28F0.6080300@st.com> References: <4DE49EBE.30804@st.com> <4DE5115B.4070409@st.com> <4DE63846.7060405@st.com> <4DEC9699.4010102@st.com> <4DEF28F0.6080300@st.com> Date: Wed, 08 Jun 2011 09:42:00 -0000 Message-ID: Subject: Re: [PATH] PR/49139 fix always_inline failures diagnostics From: Richard Guenther To: Christian Bruel Cc: GCC Patches , Jan Hubicka Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes 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 X-SW-Source: 2011-06/txt/msg00624.txt.bz2 On Wed, Jun 8, 2011 at 9:46 AM, Christian Bruel wr= ote: > Hello Richard, > > On 06/06/2011 11:55 AM, Richard Guenther wrote: >> >> On Mon, Jun 6, 2011 at 10:58 AM, Christian Bruel >> =A0wrote: >>> >>> >>>>> OK, the only difference is that we don't have the node analyzed here, >>>>> so >>>>> externally_visible, etc are not set. With the initial proposal the >>>>> warning >>>>> was emitted only if the function could not be inlined. Now it will be >>>>> emitted for each =A0DECL_COMDAT (decl)&& =A0 =A0!DECL_DECLARED_INLINE= _P >>>>> (decl)) >>>>> even >>>>> if not preemptible, so conservatively we don't want to duplicate the >>>>> availability check. >>>> >>>> Hm, I'm confused. =A0Do all DECL_COMDAT functions have the >>>> always_inline attribute set? =A0I would have expected >>>> >>>> + =A0 =A0 =A0if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (d= ecl))) >>>> + =A0 =A0 =A0 { >>>> + =A0 =A0 =A0 =A0 if (!DECL_DECLARED_INLINE_P (decl)) >>>> + =A0 =A0 =A0 =A0 =A0 warning (OPT_Wattributes, >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"always_inline not declared i= nline might not be >>>> inlinable"); >>>> + =A0 =A0 =A0 } >>>> >>> >>> I meant !DECL_COMDAT || !DECL_DECLARED_INLINE_P. but I was >>> overprecautious. >>> Didn't realize that member functions was already marked with >>> DECLARED_INLINED_P even if not explicitly set. So OK one check is enough >>> >>>> do you get excessive warnings with this? >>>> >>> >>> No I don't. That's enough to catch the original issue >> > > Unfortunately still not satisfactory, I've been testing it against a few > packages, and I notice excessive warnings with the use of __typeof (__err= or) > that doesn't propagate the inline keyword. > > For instance, a reduced use extracted from the glibc > > extern __inline __attribute__ ((__always_inline__)) =A0void > error () > { > > } > > extern void > __error () > { > } > > extern __typeof (__error) error __attribute__ ((weak, alias ("__error"))); > > emits an annoying warning on the error redefinition. > > So, a check in addition of the DECL_DECLARED_INLINED_P is needed, > TREE_ADDRESSABLE seems appropriate, since in the case of missing inline t= he > function would be emitted. So I'm testing: > > if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (decl)) > =A0 =A0 =A0 =A0 =A0&& !DECL_DECLARED_INLINE_P (decl) > =A0 =A0 =A0 =A0 =A0&& TREE_ADDRESSABLE (decl)) > > other idea ? or should be just drop this warning ? Hmm. Honza, any idea on the above? Christian, I suppose you could check if the cgraph node for that decl has the redefined_extern_inline flag set (checking TREE_ADDRESSABLE looks bogus to me). I'm not sure how the frontend merges those two decls - I suppose it will have a weak always-inline function with body :/ Richard. >> Ok. =A0The patch is ok with the !DECL_DECLARED_INLINE_P check >> if it still passes bootstrap& =A0regtest. >> > > thanks, for now I postpone until glibc is ok and more legacy checks. > > Cheers > > Christian > >> Thanks, >> Richard. >> >>> Cheers >>> >>> Christian >>> >> >