On 06/08/2011 11:11 AM, Richard Guenther wrote:
On Wed, Jun 8, 2011 at 9:46 AM, Christian Bruel<christian.br...@st.com>  wrote:
Hello Richard,

On 06/06/2011 11:55 AM, Richard Guenther wrote:

On Mon, Jun 6, 2011 at 10:58 AM, Christian Bruel<christian.br...@st.com>
  wrote:


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  DECL_COMDAT (decl)&&      !DECL_DECLARED_INLINE_P
(decl))
even
if not preemptible, so conservatively we don't want to duplicate the
availability check.

Hm, I'm confused.  Do all DECL_COMDAT functions have the
always_inline attribute set?  I would have expected

+      if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (decl)))
+       {
+         if (!DECL_DECLARED_INLINE_P (decl))
+           warning (OPT_Wattributes,
+                    "always_inline not declared inline might not be
inlinable");
+       }


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 (__error)
that doesn't propagate the inline keyword.

For instance, a reduced use extracted from the glibc

extern __inline __attribute__ ((__always_inline__))  void
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 the
function would be emitted. So I'm testing:

if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (decl))
          &&  !DECL_DECLARED_INLINE_P (decl)
          &&  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 :/


ooops, yes, TREE_ADDRESSABLE was in 4.5 :
   "In a FUNCTION_DECL, nonzero means its address is needed.
   So it must be compiled even if it is an inline function."
but indeed in 4.6 and above is is:
   "In a FUNCTION_DECL it has no meaning."

so can't use TREE_ADDRESSABLE, (I'm also patching the 4.5 locally). I'll check if redefined_extern_value does the job.

thanks

Christian

Richard.

Ok.  The patch is ok with the !DECL_DECLARED_INLINE_P check
if it still passes bootstrap&    regtest.


thanks, for now I postpone until glibc is ok and more legacy checks.

Cheers

Christian

Thanks,
Richard.

Cheers

Christian




Reply via email to