Re: [PATH] PR/49139 fix always_inline failures diagnostics

[Christian Bruel <christian.bruel@st.com>]

Hello Richard,

On 06/06/2011 11:55 AM, Richard Guenther wrote:
> On Mon, Jun 6, 2011 at 10:58 AM, Christian Bruel<christian.bruel@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 ?

> 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
>>
>