Re: [PATCH 1/2] Don't warn function alignment if warn_if_not_aligned_p is true

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Aug 25, 2017 at 1:14 PM, Martin Sebor <msebor@gmail.com> wrote:
> On 08/25/2017 01:35 PM, H.J. Lu wrote:
>>
>> On Fri, Aug 25, 2017 at 12:24 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>
>>> On 08/25/2017 11:31 AM, H.J. Lu wrote:
>>>>
>>>>
>>>> On Fri, Aug 25, 2017 at 10:15 AM, Martin Sebor <msebor@gmail.com> wrote:
>>>>>
>>>>>
>>>>> On 08/21/2017 06:21 AM, H.J. Lu wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> When warn_if_not_aligned_p is true, a warning will be issued on
>>>>>> function
>>>>>> declaration later.  There is no need to warn function alignment when
>>>>>> warn_if_not_aligned_p is true.
>>>>>>
>>>>>> OK for trunk?
>>>>>>
>>>>>> H.J.
>>>>>> --
>>>>>>         * c-attribs.c (common_handle_aligned_attribute): Don't warn
>>>>>>         function alignment if warn_if_not_aligned_p is true.
>>>>>> ---
>>>>>>  gcc/c-family/c-attribs.c | 5 ++++-
>>>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
>>>>>> index 5f79468407f..78969532543 100644
>>>>>> --- a/gcc/c-family/c-attribs.c
>>>>>> +++ b/gcc/c-family/c-attribs.c
>>>>>> @@ -1754,9 +1754,12 @@ common_handle_aligned_attribute (tree *node,
>>>>>> tree
>>>>>> args, int flags,
>>>>>>        This formally comes from the c++11 specification but we are
>>>>>>        doing it for the GNU attribute syntax as well.  */
>>>>>>      *no_add_attrs = true;
>>>>>> -  else if (TREE_CODE (decl) == FUNCTION_DECL
>>>>>> +  else if (!warn_if_not_aligned_p
>>>>>> +          && TREE_CODE (decl) == FUNCTION_DECL
>>>>>>            && DECL_ALIGN (decl) > (1U << i) * BITS_PER_UNIT)
>>>>>>      {
>>>>>> +      /* Don't warn function alignment here if warn_if_not_aligned_p
>>>>>> is
>>>>>> +        true.  It will be warned later.  */
>>>>>>        if (DECL_USER_ALIGN (decl))
>>>>>>         error ("alignment for %q+D was previously specified as %d "
>>>>>>                "and may not be decreased", decl,
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Your comment refers to warning but the code here uses error().
>>>>> That raises two questions for me: a) will the later diagnostic
>>>>> really be a warning or an error, and if a warning, under what
>>>>> option will it be issued? and b) why is an error appropriate
>>>>> here when a warning is appropriate elsewhere (most other
>>>>> attribute conflicts are at present diagnosed with -Wattributes).
>>>>
>>>>
>>>>
>>>> It can be changed to warning.
>>>
>>>
>>>
>>> Okay, that's goo to hear (it validates what I did in the patch
>>> I referenced).
>>>
>>> As for your patch, I don't quite see ho the block can ever be
>>> entered.  In fact, I had to make changes to the function in
>>> my patch below to let it detect and diagnose the condition
>>> (attempting to reduce the alignment of a function).  The
>>> details are in bug 81566.  Did I miss something?
>>
>>
>> Try this:
>>
>> __attribute__((warn_if_not_aligned(8)))
>> void
>> foo2 (void)
>> {
>> }
>
>
> For me the top of trunk prints the same error both with and
> without your patch:
>
>   error: ‘warn_if_not_aligned’ may not be specified for ‘foo2’

Please try it with Linux/alpha cross compiler.

> The problem I was referring to above (bug 81566), and the reason
> why I said the block in the if statement modified by in your patch
> is unreachable is that its controlling expression is a subset of
> the test above it.  See comment 2 in bug 81566.
>
> Martin
>



-- 
H.J.